Remove SidePanelCoordinator final keyword substitution
#20765
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`SidePanelCoordinator` is tagged upstream as `final`, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keyword `final` with naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral. This change replaces the substitution with a patch. The substitution was removing the `final` keyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure.
mherrmann
pushed a commit
that referenced
this pull request
Nov 9, 2023
`SidePanelCoordinator` is tagged upstream as `final`, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keyword `final` with naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral. This change replaces the substitution with a patch. The substitution was removing the `final` keyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure.
25 tasks
mkarolin
pushed a commit
that referenced
this pull request
Nov 28, 2023
`SidePanelCoordinator` is tagged upstream as `final`, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keyword `final` with naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral. This change replaces the substitution with a patch. The substitution was removing the `final` keyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure.
mkarolin
pushed a commit
that referenced
this pull request
Nov 30, 2023
`SidePanelCoordinator` is tagged upstream as `final`, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keyword `final` with naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral. This change replaces the substitution with a patch. The substitution was removing the `final` keyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure.
kjozwiak
pushed a commit
that referenced
this pull request
Dec 1, 2023
* Merge pull request #20434 from brave/cr120 Upgrade from Chromium 119 to Chromium 120. * Updated chrome-VERSION.patch from Chromium 119.0.6045.163 to Chromium 120.0.6099.35. * Additional assert_ts.js -> assert.js for 1.61.x. Chromium change: https://chromium.googlesource.com/chromium/src/+/f7a2ea671005530e7a891bba411a34bfd1b3b4cd commit f7a2ea671005530e7a891bba411a34bfd1b3b4cd Author: dpapad <dpapad@chromium.org> Date: Mon Oct 16 21:13:04 2023 +0000 WebUI: Rename assert_ts.ts to assert.ts, part 7 (last) In this part updating one remaining reference in ui/file_manager/ and removing assert_ts.ts. assert_ts.ts used the "_ts" suffix to differentiate from the previously existing assert.js file. The latter no longer exists and the "_ts" suffix can be dropped. Fixed: 1291526 * Remove `SidePanelCoordinator` `final` keyword substitution (#20765) `SidePanelCoordinator` is tagged upstream as `final`, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keyword `final` with naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral. This change replaces the substitution with a patch. The substitution was removing the `final` keyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure. * `Label::CalculatePreferredSize()` marked as `final` (#20767) This function is now marked as `final`, however there is another entry in the overload set that can be used in its place for the override in `BraveVPNStatusLabel`. Chromium change: https://chromium.googlesource.com/chromium/src/+/bf2f84a8c4f27e399e05bf4730a29152f6a8b0b6 commit bf2f84a8c4f27e399e05bf4730a29152f6a8b0b6 Author: weidongliu <liuwd8@gmail.com> Date: Mon Oct 30 15:59:03 2023 +0000 views: Mark CalculatePreferredSize() of views::Label as final Views::Label now has two functions to calculate the preferred size. This can cause some unexpected errors. CalculatePreferredSize() is now expected to be deprecated. Therefore, mark it as final to prevent overrided by subclasses. Bug: 1346889 * Revert "Disable kPowerBookmarksSidePanel feature flag" This reverts commit 237341e. * Java files formatting. * Fix `BraveBrowsingDataRemoverDelegate` substitution (#21034) This override has been missing a matching header since it was introduced, and this has been causing linking to fail in every upstream occurrence of `ChromeBrowsingDataRemoverDelegateFactory::GetForProfile` as the return type in the header was not being replaced accordingly. This changes adds the changes to the header as well, and provides an inclusion `BraveBrowsingDataRemoverDelegate` in the shadow file, to allow the compiler to understand that `BraveBrowsingDataRemoverDelegate*` is pefectly convertible to `ChromeBrowsingDataRemoverDelegate`. * [Android] Move JNI classes to jni_zero namespace * Disabled failing upstream browser tests. * Merge pull request #21152 from brave/maxk-fix-page-info-cookies-separator [cr120 follow-up] Removes an extra separator in cookies page info view. * Merge pull request #21153 from brave/maxk-redirect-on-device-data-to-cookies [cr120 follow-up] Redirect On-device data link to cookies page. * Merge pull request #21166 from brave/maxk-hide-hover-card-image-toggle [cr120 follow-up] Hide Settings->Appearance hover card toggle. * Fixed crash accessing 'send to devices' in share menu via Sync (#21168) Fixes brave/brave-browser#34636 Related Chromium commit: https://source.chromium.org/chromium/chromium/src/+/84ed1f62ecd64bbfc3d2c4775a6e80a6b0e9defb Clean up SendTabToSelfSigninPromo flag Enabled by default in M119 (https://crrev.com/c/4905024), can be cleaned up in M120. Fixed: 1479693, 1295204 Change-Id: Ic778e9c886a5d9391fb5da7510d166952481fe21 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905343 Reviewed-by: Boris Sazonov <bsazonov@chromium.org> Reviewed-by: Wenyu Fu <wenyufu@chromium.org> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Commit-Queue: Victor Vianna <victorvianna@google.com> Cr-Commit-Position: refs/heads/main@{#1204655} * Merge pull request #21169 from brave/120.0.6099.56_master Upgrade from Chromium 120.0.6099.35 to Chromium 120.0.6099.56. * Updated chrome-VERSION.patch from Chromium 120.0.6099.35 to Chromium 120.0.6099.56. --------- Co-authored-by: cdesouza-chromium <cdesouza@brave.com> Co-authored-by: Emerick Rogul <erogul@brave.com> Co-authored-by: Artem Samoilenko <artem@brave.com> Co-authored-by: AlexeyBarabash <AlexeyBarabash@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SidePanelCoordinatoris tagged upstream asfinal, and as a workaround to be able to derive from it, an override had been introduced at some point to replace the keywordfinalwith naught. This type of redefinition is unreliable, as it is UB, and it also becomes highly viral.This change replaces the substitution with a patch. The substitution was removing the
finalkeyword from the signature of other functions in different headers, and leaving it with no token to indicate the function is an override, causing a build failure.Resolves
Submitter Checklist:
QA/YesorQA/No;release-notes/includeorrelease-notes/exclude;OS/...) to the associated issuenpm run test -- brave_browser_tests,npm run test -- brave_unit_testswikinpm run lint,npm run presubmitwiki,npm run gn_check,npm run tslintgit rebase master(if needed)Reviewer Checklist:
gnAfter-merge Checklist:
changes has landed on
Test Plan: