Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade from Chromium 96 to Chromium 97 #10664

Merged
merged 86 commits into from
Dec 8, 2021
Merged

Upgrade from Chromium 96 to Chromium 97 #10664

merged 86 commits into from
Dec 8, 2021

Conversation

mkarolin
Copy link
Collaborator

@mkarolin mkarolin commented Oct 23, 2021

Fixes brave/brave-browser#18954

Also:

Fixes brave/brave-browser#17568
Fixes brave/brave-browser#19907

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

samartnik and others added 22 commits December 7, 2021 14:45
Chromium change:
chromium/chromium@f7e3a41

Adds ability to include both v8 snapshot types
And adds feature as to which one is used at runtime. Specific
parts:
. adds gn arg: include_both_v8_snapshots. At this time this is
  only supported on android. I will likely make it work on chromeos
  next.
. Adds feature kUseContextSnapshot. This feature is available if
  include_both_v8_snapshots is set.
. Adds switch kUseContextSnapshotSwitch. This is passed from browser
  to renderer if kUseContextSnapshot is enabled. A separate switch is
  used as at the time the v8 snapshot is loaded features have not been
  loaded.
And this updates a bunch of build targets accordingly.

BUG=1257321
TEST=none

Change-Id: I225a9235ac00e1c25ceaf0423587a00a1aafc740
With new R8 library there is a warning related to our bytecode patching:
Warning: Missing method void Lg2.d() (referenced from: void Vf2.run())

Without obfuscation it looks like this:
Warning: Missing method void org.chromium.chrome.browser.toolbar.ToolbarManager.$invoke$special$updateButtonStatus() (referenced from: void org.chromium.chrome.browser.toolbar.ToolbarManager$$ExternalSyntheticLambda3.run())

We do change invoke special to invoke virtual for this method and this is expected.

Chromium change:
chromium/chromium@e3db48a

Android: Stop disabling R8's allowaccessmodification -keep semantics
Causes a 5.5kb regression, but brings us closer to stock R8, and will
ensure visibility of test @rule fields are not changed, thus avoiding
bugs like bug 1265431.

Bug: 1265431
This reverts previous commit:

[Android] Filter warning after bytecode pacthing
With new R8 library there is a warning related to our bytecode patching:
Warning: Missing method void Lg2.d() (referenced from: void Vf2.run())

Without obfuscation it looks like this:
Warning: Missing method void org.chromium.chrome.browser.toolbar.ToolbarManager.$invoke$special$updateButtonStatus() (referenced from: void org.chromium.chrome.browser.toolbar.ToolbarManager$$ExternalSyntheticLambda3.run())

We do change invoke special to invoke virtual for this method and this is expected.

Chromium change:
chromium/chromium@e3db48a

Android: Stop disabling R8's allowaccessmodification -keep semantics
Causes a 5.5kb regression, but brings us closer to stock R8, and will
ensure visibility of test @rule fields are not changed, thus avoiding
bugs like bug 1265431.

Bug: 1265431
- Added BUILD.gn files to exclusions
- Added *test.mm files to exclusions
- Alphabetized exclusions
Removed obsolete check
The flag is no longer needed: the tests run normally on CI without it.
When building tests the following error happens:

17:40:57  gen/chrome/test/data/webui/settings/preprocessed/test_lifetime_browser_proxy.ts:11:14 - error TS2420: Class 'TestLifetimeBrowserProxy' incorrectly implements interface 'LifetimeBrowserProxy'.
17:40:57    Property 'relaunchOnMac' is missing in type 'TestLifetimeBrowserProxy' but required in type 'LifetimeBrowserProxy'.
17:40:57
17:40:57  11 export class TestLifetimeBrowserProxy extends TestBrowserProxy implements
17:40:57                  ~~~~~~~~~~~~~~~~~~~~~~~~
17:40:57
17:40:57    gen/chrome/browser/resources/settings/tsc/lifetime_browser_proxy.d.ts:4:5
17:40:57      4     relaunchOnMac(): void;
17:40:57            ~~~~~~~~~~~~~~~~~~~~~~
17:40:57      'relaunchOnMac' is declared here.

which means we'd have to add a patch to test_lifetime_proxy.ts to add our relaunchOnMac functionality.

Instead, this change removes all relaunchOnMac functionality from WebUI and moves it
into the BrowserLifetimeHandler.
Chromium change:

https://chromium.googlesource.com/chromium/src.git/+/fdf5d8eb7fb14333f433299f3acb0a98e979a6dd

commit fdf5d8eb7fb14333f433299f3acb0a98e979a6dd
Author: Olesia Marukhno <[email protected]>
Date:   Tue Nov 2 15:43:55 2021 +0000

    [page info] Clean up PageInfoV2Desktop flag

    After launching page info, remove the flag and code that supports old
    bubble view.

    Bug: 1188101
From cr97 on, having this feature enabled means that the browser will
attempt to reach the Optimization Guide Server every now and then at
the https://optimizationguide-pa.googleapis.com endpoint, as it was
detected by the npm run network-audit command:

  [:ERROR:brave_network_audit_browsertest.cc(151)] NETWORK AUDIT FAIL:https://optimizationguide-pa.googleapis.com/v1:GetModels?key=dummytoken
  [:ERROR:brave_network_audit_browsertest.cc(151)] NETWORK AUDIT FAIL:https://optimizationguide-pa.googleapis.com/v1:GetModels?key=dummytoken

    [...]

  [:ERROR:brave_network_audit_browsertest.cc(151)] NETWORK AUDIT FAIL:https://optimizationguide-pa.googleapis.com/v1:GetModels?key=dummytoken
  ../../brave/browser/net/brave_network_audit_browsertest.cc:249: Failure
  Value of: PerformNetworkAuditProcess(events)
    Actual: false
  Expected: true
  network-audit FAILED. Import /home/mario/work/brave-rebases/brave-browser/src/network_log.json in chrome://net-internals for more details.
  Stack trace:
  #0 0x55f213f6cc10 brave::(anonymous namespace)::BraveNetworkAuditTest::TearDownInProcessBrowserTestFixture()
  #1 0x55f215bda3be content::BrowserTestBase::SetUp()
  #2 0x55f215bd741e InProcessBrowserTest::SetUp()

This patch disables the kkOptimizationHints feature. TBD if this is
needed.

This patch disables the kRemoteOptimizationGuideFetching features that
are otherwise enabled by default, avoiding this URL requests.

PS: I couldn't pinpoint yet where exactly in Chromium 97 this started
happening, but I can consistently reproduce it on my local cr97 branch
setting kMaxTimeoutPerLoadedURL in brave_network_audit_browsertest.cc
to at least 20 seconds, while this doesn't happen on cr96 even when
using the default timeout of 5 minutes, so it's cr97-related.
Otherwise it falls back to MaterialButton and causes styling issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/run-network-audit Run network-audit
Projects
None yet