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 129 to Chromium 130 #25340

Draft
wants to merge 140 commits into
base: master
Choose a base branch
from
Draft

Upgrade from Chromium 129 to Chromium 130 #25340

wants to merge 140 commits into from

Conversation

emerick
Copy link
Contributor

@emerick emerick commented Aug 27, 2024

Resolves brave/brave-browser#40694

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • 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 wiki
    • npm run presubmit wiki, 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:

cdesouza-chromium and others added 24 commits September 19, 2024 16:36
`WebUIBubbleManager::MaybeInitPersistentRenderer` was being stubbed in
brave, but this function is not more, so we can do away with the stub.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/28ebfbc03cb7d7036868510f4dddb564e694a367

commit 28ebfbc03cb7d7036868510f4dddb564e694a367
Author: Thomas Lukaszewicz <tluk@chromium.org>
Date:   Wed Sep 11 00:54:41 2024 +0000

    [tab search] Remove kWebUIBubblePerProfilePersistence

    kWebUIBubblePerProfilePersistence is a feature that allows sharing
    a single Tab Search instance across many different bubble hosts
    for a given profile.

    This feature experiment has concluded and the team is no longer
    pursuing this approach. This CL cleans up any feature-associated
    code.

    Bug: None
This function was taking a `SidePanelEntry` instance, but now both
variants only take a `SidePanelEntry::Key`.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/f96901b4389d051c9c1d520f82a60ae92a34add2

commit f96901b4389d051c9c1d520f82a60ae92a34add2
Author: Erik Chen <erikchen@chromium.org>
Date:   Tue Sep 10 23:00:49 2024 +0000

    Reduce fragility in SidePanelCoordinator (part 4 / N)

    This CL is primarily a refactor with no intended behavior change. It
    removes redundant logic from SidePanelCoordinator.

    This CL fixes a minor bug from part 2 where we were not correctly
    checking whether a closing extension was using the tab-scoped or
    window-scoped registry. This is not expected to have any user-visible
    effect, but is good to fix anyways.

    Bug: 363743081
This is a result of upstream spanification. For our case it seems to be
very simple replacement.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/c5f3a25855b895bb587af5dbd8ff3ebfb7be76e1

commit c5f3a25855b895bb587af5dbd8ff3ebfb7be76e1
Author: Fredrik Söderquist <fs@opera.com>
Date:   Tue Sep 10 19:26:09 2024 +0000

    spanification: InspectorNetworkAgent and related bits

    Pass SpanOrSize<> to DidReceiveData() and a span<> to
    DidSendWebSocketMessage(). Propagate up and down the relevant call
    chains as needed. Use span<>s in NetworkResourcesData.

    Bug: 351564777
This only affects how certain instatiations were being done, as the
data itself was only getting passed along.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/eecda5d4d6f20fe6baba7f7c0eb0e02284a349f6

commit eecda5d4d6f20fe6baba7f7c0eb0e02284a349f6
Author: Joshua Pawlicki <waffles@chromium.org>
Date:   Wed Sep 11 13:48:38 2024 +0000

    update_client: Simplify ping_manager.

    Also remove a dependency from util on component.

    Also improve memory safety in request_sender.

    Bug: 353249967
`ScriptValue` can be constructed from `V8Promise` with an isolate.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/60d8423bb34a146cfb1a2d1ed8a818176ae410e9

commit 60d8423bb34a146cfb1a2d1ed8a818176ae410e9
Author: Andrey Kosyakov <caseq@chromium.org>
Date:   Wed Sep 11 18:03:42 2024 +0000

    Remove ScriptPromise::AsScriptValue(), let call sites create it themselves

    Change-Id: I606a7b388bf90a1a283b474b242a14866d2f92c6
Related Chromium change:
https://source.chromium.org/chromium/chromium/src/+/333ffb2d4a93d2ce9e9a5fabdf1e593d6d062e9d

	Remove and clean up old Translate Infobar code. Leaving translate_infobar_delegate because there are still iOS dependencies attached.

	Fixed: 353735844
	Bug: 40213244
	Change-Id: Ief6abe1485ef372f1ac44426b1edeb3b69b89208
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5727132
"RetainOmniboxOnFocus" feature is used instead

Related Chromium change:
https://source.chromium.org/chromium/chromium/src/+/46147680fe334aeec900ec6d76c8b9e443d07452

	Adds flag-guarded support for retaining omnibox on focus.

	This CL adds flag-guarded support for retaining omnibox contents on
	focus (as opposed to clearing it). When enabled, omnibox contents will
	also be fully selected (to allow for quick replace) and zero-prefixed
	suggestions are still shown (to allow for quick search). When enabled,
	only devices with large screens, attached keyboards, and attached
	precision pointers are affected.

	Design doc: http://shortn/_g7JfqshAX0

	Before:
	* google.com: http://shortn/_QQDvhI44dH
	* NTP w/o paste: http://shortn/_RowwKkjjGM
	* NTP w/ paste: http://shortn/_9Im63Fv4Ep

	After:
	* google.com: http://shortn/_29TqnQpVQE
	* NTP w/o paste: http://shortn/_whJqKzcDnv
	* NTP w/ paste: http://shortn/_ZnBOkVUvrU

	Bug: 347632178
	Change-Id: I8c8ceffa766ff7c960f122241ae3fb2efe3654c6
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5804846
This crate is used in brave, and it has now been introduced in upstream
chromium as well. This change deletes the local copy for this crate, as
this was causing duplicated id errors for the two crates, and makes sure
we use the crate checked out on upstream. However, some feature patching
was required for the use we do in brave.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/6cc428454a87621f6cefbc08050ca53ded4cfca7

commit 6cc428454a87621f6cefbc08050ca53ded4cfca7
Author: danakj <danakj@chromium.org>
Date:   Fri Sep 13 20:46:38 2024 +0000

    Import zerocopy and byteorder crates

    The zerocopy crate is used by rand now (via ppv-lite86) so we pull it in
    to be able to roll our 3p dependencies to their latest versions. As it
    is developed by and used internally at Google, we'll see future crates
    make use of it as well.

    The zerocopy crate provides safe abstractions to handle data in a
    zero-copy manner, which we would grately prefer over writing unsafe Rust
    code, so we allow the crate for 1p use while importing it.

    Unsafe review for byteorder is done in this CL.

    Change-Id: I077b0596bd3738ecd26da8ad2aab168626f23d2f
This is another case where we are required to add one of the headers to
our inclusions to avoid unwanted substitution.
The return value for `GetInstalledFile` has changed, which has broken
the override, as it relies on the substitution declaration to have the
same return type.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/4bc4563fa88f842b1fa05ee6ec28f80c26a48f6f

commit 4bc4563fa88f842b1fa05ee6ec28f80c26a48f6f
Author: Joshua Pawlicki <waffles@chromium.org>
Date:   Thu Sep 12 19:14:48 2024 +0000

    update_client: Simplify ActionRunner.

    Also eliminate the implementation of GetInstalledFile for the extension
    updater. The diff system no longer calls it and the only remaining
    caller is ActionRunner, which we don't need to support for extensions.

    Also add some checks against parent and absolute paths in the run
    command. (We don't need these and maybe it's unwise to have the client
    accept them from the server for insider risk reasons.)

    The original motivation of the CL was to continue to eliminate refs to
    Component (ActionRunner held a raw_ref.)

    Bug: 353249967
Upstream has enabled this warning, which has failed in some cases in our
codebase.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/8104564104c2c2e5f3961623707ee5fc55e2706c

commit 8104564104c2c2e5f3961623707ee5fc55e2706c
Author: Devon Loehr <dloehr@google.com>
Date:   Fri Sep 13 19:20:46 2024 +0000

    Enable deprecated-this-capture warning

    We've finally eliminated all lambdas which implicitly capture the `this` pointer by value, so time to enable the warning globally.

    Includes a fix for one last file which snuck in since my last fix.

    Change-Id: Ib601567205f0aab2992bac0c60fe0a2b4e63465b
    Fixed: 351004963
`BravePrivacySandboxSettings` stubs out everything to false, as we
disable this feature.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/b67df9e045d89274e7641b8a077c5bfafe762886

commit b67df9e045d89274e7641b8a077c5bfafe762886
Author: Xiaochen Zhou <xiaochenzh@chromium.org>
Date:   Fri Sep 13 18:53:36 2024 +0000

    [Privacy Sandbox] Implement IsLocalUnpartitionedDataAccessAllowed
    attestation check

    Implement the PrivacySandboxSettings layer attestation check for the
    new attestation API: LOCAL_UNPARTITIONED_DATA_ACCESS

    The change in this CL does not have any effect until the upstream
    attestation check in ChromeContentBrowserClient is implemented and
    plug into the API call site.

    The logics of IsLocalUnpartitionedDataAccessAllowed mimics
    IsSharedStorageAllowed. Except it does not implement the debug
    functionality. Also there is a TODO crbug.com/365788691 for gating
    this feature on 3PC setting.

    This CL also adds the histogram metric for this new attestation API.

    See https://protobuf.dev/programming-guides/proto3/#updating:~:text=If%20you%20add,section%20for%20details.

    See https://github.com/WICG/fenced-frame/blob/master/explainer/fenced_frames_with_local_unpartitioned_data_access.md.

    Bug: 361375807
This change is of no consequence to the only override in the codebase
for this function, because the argument was not being used.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/4a0ed02d15c72920ae48b2a1987e867d096ed1f9

commit 4a0ed02d15c72920ae48b2a1987e867d096ed1f9
Author: Kevin McNee <mcnee@chromium.org>
Date:   Thu Sep 12 20:26:29 2024 +0000

    Remove unused inner WebContents attach params

    Following the removal of the portals implementation, the remaining
    callers of AttachInnerWebContents don't have remote frame interfaces to
    bind.

    None of the observers of InnerWebContentsAttached use `is_full_page`.

    Bug: 40287334
Chromium change:
https://chromium.googlesource.com/chromium/src/+/8c8b25bb5e964207b68cc982d718f161ef601f29

commit 8c8b25bb5e964207b68cc982d718f161ef601f29
Author: rbpotter <rbpotter@chromium.org>
Date:   Wed Sep 11 17:28:44 2024 +0000

    Downloads: Cleanup Polymer leftovers

    - Check in .html.ts file for downloads item
    - Migrate icons to cr-iconset
    - Remove Polymer ts_dep, except in branded builds where it is still
      needed for the branded iconset.

    Bug: 40943652
This was introduced some time ago, however it has been reverted on
upstream. This change readjusts our own overrides for this type.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/e861d646ca7fa8d8e5dbeee54e9e88d50433da54

commit e861d646ca7fa8d8e5dbeee54e9e88d50433da54
Author: Alex Ilin <alexilin@chromium.org>
Date:   Mon Sep 16 09:51:44 2024 +0000

    Revert "Split API key caches to load them separately."

    This reverts commit 1f30fba88b01443407adaf2a6e1852b8beee8744.

    Reason for revert: splitting the cache didn't help and we'd like to
    initialize all values at the same time again.

    This is not a clean revert because of merge conflicts + keeping changes
    to tests.

    Original change's description:
    > Split API key caches to load them separately.
    >
    > This CL splits the API caches into 4 different objects: the goal is to attempt to load them as lazily as possible, so that the main API key is loaded after the FeatureList is initialized on ChromeOS.
    >
    > This CL assumes that there one of the other keys stored in the old
    > `g_api_key_cache` is used early during the initialization process on
    > ChromeOS and this is why the entire cache of API Keys is loaded earlier
    > than the feature flags are loaded. This CL is an attempt to load the
    > main APIKey lazily (e.g. decouple its load from `metrics_key` which
    > seems to be used early during the initialization sequence).
    >
    > To validate this approach, we expect there to be 0 entries in histogram
    > Signin.APIKeyMatchesFeatureOnStartup for bucket "false" when the
    > experiment is enabled.
    >
    > Bug: TBD
    > Change-Id: I9da5cc5024bb397e2158b584da33d8519e9388b1
    > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5830788
    > Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
    > Reviewed-by: Alex Ilin <alexilin@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#1349928}

    Bug: TBD
Upstream has introduced a corresponding `GetForProfile` to
`GetForBrowserState` static implementations. Some of these functions are
shadowed with a different implementation on brave's build. This change
corrects our own shadowing to mach the upstream change.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/60e41b00436d57c7a7ff5c4671cb6c5e970ae9d4

commit 60e41b00436d57c7a7ff5c4671cb6c5e970ae9d4
Author: Mark Cogan <marq@google.com>
Date:   Wed Sep 11 15:53:28 2024 +0000

    Forward GetForBrowserState() getters in keyed services.

    For every keyed service with a GetForBrowserState() getter, this CL
    adds a GetForProfile() getter as the authoritative implementation and
    updates GetForBrowserState() to forward to this method.

    Also:

    * The GetForBrowserState params are changed to be "ProfileIOS* profile".
    * In many cases, the ordering of methods in the keyed service header
      are standardized and spurious comments are removed.
    * In many cases, comments referencing browser states are updated to
      reference profiles.
    * ReadingListModelFactory::GetAsDualReadingListModelForBrowserState is
      migrated (all call sites updated) to ...ModelForProfile().
    * Other small cleanups.

    Low-Coverage-Reason: LARGE_SCALE_REFACTOR
    Bug: 358335594
These tests were crashing on Android:
   PageMetricsUnitTest.DomainsLoadedCount
   PageMetricsUnitTest.BookmarkCount
   PageMetricsUnitTest.FirstPageLoadTimeTooLate
   PageMetricsUnitTest.FirstPageLoadTimeImmediate
   PageMetricsUnitTest.PagesLoadedCount
   PageMetricsUnitTest.FirstPageLoadTimeLater
   BraveSearchProviderTest.SearchDoesNotIncludeHistoryWhenHistoryDisabled
   BraveSearchProviderTest.SearchIncludesHistoryWhenHistoryEnabled

Tests were crashing because they cannot create SearchEngineCountryDelegateImpl object.

Crash didn't happen on cr129 because SearchEngineChoiceService::GetCountryIdInternal
returnd before Java_SearchEngineChoiceService_requestCountryFromPlayApi call.

Related Chromium change:
chromium/chromium@17b1d05

	[waffle]Remove search_engines::IsChoiceScreenFlagEnabled()
	Remove the function and related calls and params. This is because the
	kSearchEngineChoiceTrigger feature is now fully launched.

	Fixed: b:364815974
	Change-Id: I1b3c253c656c76246fae96486b2863632878f17a
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5837290
We must it duplicate at our brave/android/java/org/chromium/chrome/browser/ntp/IncognitoNewTabPageView.java

Related Chromium change:
https://source.chromium.org/chromium/chromium/src/+/a269abacce33fa1cdb136a4c4b0ef6b6d20d4d0b

	[E2E] Draw Incognito NTP edge to edge

	This change plumb through the EdgeToEdgeController to Incognito NTP (INTP). When drawing edge to edge, add the bottom inset padding to INTP's scroll view, and #setClipToPadding(false). This makes the view drawing into the nav bar region while ensure the controls in the page remains reachable when scrolled to the bottom.

	The change also introduces an interface for NativePage to check if it is compatible for edge to edge. Currently only INTP is supported, and change is guarded behind DrawKeyNativePageToEdge.

	The preview:
	https://screenshot.googleplex.com/BYiJ8TkNP4Q3pqa

	Bug: 339025702
	Change-Id: I86459682089adc9f8898382fb90278136444c705
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5847378
…dgeControllerSupplier

Related Chromium change:
https://source.chromium.org/chromium/chromium/src/+/a269abacce33fa1cdb136a4c4b0ef6b6d20d4d0b

	[E2E] Draw Incognito NTP edge to edge

	This change plumb through the EdgeToEdgeController to Incognito NTP (INTP). When drawing edge to edge, add the bottom inset padding to INTP's scroll view, and #setClipToPadding(false). This makes the view drawing into the nav bar region while ensure the controls in the page remains reachable when scrolled to the bottom.

	The change also introduces an interface for NativePage to check if it is compatible for edge to edge. Currently only INTP is supported, and change is guarded behind DrawKeyNativePageToEdge.

	The preview:
	https://screenshot.googleplex.com/BYiJ8TkNP4Q3pqa

	Bug: 339025702
	Change-Id: I86459682089adc9f8898382fb90278136444c705
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5847378
It is not static now and have a new tracker arg.

Related Chromium change:
https://source.chromium.org/chromium/chromium/src/+/828eb6d14543d75671991083fd162b3308bcadbe

	[DefaultBrowser] Add helper for the role manager restriction

	This CL adds a helper DefaultBrowserPromoUtils#shouldShowOtherPromo to
	decide if other default browser promos can be shown.
	The conditions mean to ensure that we only show other promo when the
	role manager promo can't be shown either because of reaching the max
	count or role manager not available in the current system.

	It also adds the a feature engagement event to ensure the new promo
	can only be shown if the role manager promo not shown in the 7 days
	window.

	Bug: 364906215
	Change-Id: If5684ee24ff87d193939dd9b8c6ae6d2a8af783c
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839024
Chromium change:
https://source.chromium.org/chromium/chromium/src/+/52962da478ed750038f966c386c4ee0ff2d73837

Always apply luminance calculation to navigation bar icons

Currently, the navigation bar icons only respond to the color of the navigation bar during animations. Since animations are being disabled for the navbar coloring experiment, it's important that the luminance calculation to properly color the navigation bar icons is applied all the time, not just during animations.

Bug: 364228106, 40925025
emerick and others added 4 commits September 19, 2024 16:46
This hasn't been working for several releases and, with the switch to Lit, it's
no longer compiling. After speaking with Jay, decided that the simplest way
forward is to remove the override and fix it in master once cr130 is merged.

Tracked via brave/brave-browser#41153
otherwise there are errors
```
ERROR at //brave/third_party/rust/sha2/v0_10/BUILD.gn:56:15 (//build/toolchain/android:android_clang_arm): Item not found
    deps -= [ "//brave/third_party/rust/cpufeatures/v0_2:lib" ]
              ^----------------------------------------------
You were trying to remove "//brave/third_party/rust/cpufeatures/v0_2:lib"
from the list but it wasn't there.
See //brave/third_party/rust/ed25519_dalek_bip32/v0_2/BUILD.gn:36:5: which caused the file to be included.
    "//brave/third_party/rust/sha2/v0_10:lib",
```
- at `src/brave/third_party/rust/aes/v0_8/BUILD.gn`
- at `src/brave/third_party/rust/keccak/v0_1/BUILD.gn`
- at `src/brave/third_party/rust/sha2/v0_10/BUILD.gn`
- at `src/brave/third_party/rust/sha2/v0_9/BUILD.gn`

Happens when current_cpu=arm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) CI/run-network-audit Run network-audit CI/run-upstream-tests Run upstream unit and browser tests on Linux and Windows (otherwise only on Linux) CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet/core feature/web3/wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade from Chromium 129 to Chromium 130
6 participants