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 Chromium from 73.0.3683.75 to Chromium 74.0.3729.22 #2013

Merged
merged 33 commits into from
Mar 21, 2019

Conversation

mkarolin
Copy link
Collaborator

@mkarolin mkarolin commented Mar 19, 2019

Fix brave/brave-browser#3502
Related PR: brave/brave-browser#3790

It's easiest to review this per commit.
Each commit explains where the change came from in Chromium's history.

Special notes to QA:

  • Retest importer fully
  • Retest ads fully
  • Retest ledger fully
  • Retest Tor fully
  • Retest shields fully

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

See brave/brave-browser#3502.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

{{declare_initial(property.name.to_upper_camel_case() if not header)|indent(spaces, true)}} {
{{caller(property)|indent(spaces, true)-}}
-{{'}'|indent(spaces, true)}}
+{{'\n}'|indent(spaces, true)}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial patch was:

commit d1410d069d22188896cdc13a526b3732cc21c90b
Author: Brian R. Bondy
Date:   Fri Feb 15 00:03:08 2019 -0500

Add newline on line ending for } for generated file

This was causing an error on some computers when building (Anthony and me)

In C74 the code has been changed to:

    {% if property.style_builder_inline == header and property.style_builder_generate_initial %}
{{declare_initial(property.name.to_upper_camel_case() if not header)|indent(spaces, true)}} {
{{(caller(property) ~ '}')|indent(spaces, true)}}

Chromium's Service Manager no longer supports JSON manifests.
All JSON manifests and overlays need to be converted to C++.
See: https://bugs.chromium.org/p/chromium/issues/detail?id=895616

This change affects the following services:
- bat_ads
- bat_ledger
- tor_launcher
base::JSONReader::ReadAndReturnError.

base::JSONReader changed signatue of Read to return Optional<Value>
instead of std::unique_ptr<Value>. ReadAndReturnError has been
deprecated and replaced with ReadAndReturnValueWithError that
returns a struct with error instead of having multiple out parameters.

Chromium chages:

commit 025edc25d8c8649d5a8dfd062e952855b3dfa13e
Author: Lei Zhang <thestig@chromium.org>
Date:   Sat Feb 16 05:02:25 2019 +0000

    Change base::JSONReader::Read()/ReadAndReturnError() to return Optional.
    BUG=925165

commit 015397daa81409c43afbcd11ac41978cd7175cd0
Author: Lei Zhang <thestig@chromium.org>
Date:   Wed Feb 20 04:26:59 2019 +0000

    Change base::JSONReader::ReadAndReturnError() to not use out params.

    Instead of returning Optional<Value> and putting the error status in 4
    out parameters, return struct ValueWithError which contains the value
    and the error status.
ContextLifecycleStateObserver.

Chromium change:

commit 6bbacc4529f0ed67a55e49128b927ea1c3e5d33a
Author: Dave Tapuska <dtapuska@chromium.org>
Date:   Mon Feb 11 20:00:17 2019 +0000

    Rename PausableObject to ContextLifecycleStateObserver

    Get rid of PauseState enum and use the FrameLifecycleState

    BUG=907125
Tranport socket is now a meber of the base class and doesn't need to be
passed around. Destination is now a host/port pair.

Chromium changes:

commit 4b412da433b4b3e4cb70b00210395bbc73fbdfaa
Author: Matt Menke <mmenke@chromium.org>
Date:   Fri Jan 25 19:31:12 2019 +0000

    Socket Pools Refactor 8: Merge SOCKS socket pool into transport pool.

    This CL merges the SOCKSClientSocketPool with the
    TransportClientSocketPool it used to sit above. In doing this, the
    SOCKSClientSocketPool class is removed, in favor of a
    TransportClientSocketPool. SOCKSConnectJobs now directly create
    TransportConnectJobs, instead of going through a socket pool.

    This doesn't affect any other socket pools - there's still an SSL socket
    pool on top of the socket pool for a SOCKS proxy, for instance.

    This is part of an effort to flatten the socket pools.
    https://docs.google.com/document/d/1g0EA4iDqaDhNXA_mq-YK3SlSX-xRkoKvZetAQqdRrxM/edit

    Bug: 472729

commit 6b85fafc752ff33b9e92318f8577032897c9e40b
Author: Eric Orth <ericorth@chromium.org>
Date:   Wed Feb 6 18:53:35 2019 +0000

    Modernize DNS in SOCKS[5]ClientSocket

    Convert all |destination| storage to use HostPortPair instead of
    HostResolver::ResolveInfo, and convert Resolve() request to
    CreateRequest().

    Removed HangingHostResolverWithCancel from test as it didn't support
    CreateRequest(), and a working equivalent already exists in
    mock_host_resolver.h.

    Bug: 922699
URLLoaderFactory method is replaced with GetNetworkFetcherFactory.

Chromium changes:

commit 75e6bf2135e7c42a24bed2343f63f5a07e17fb13
Author: Sorin Jianu <sorin@chromium.org>
Date:   Tue Feb 12 16:07:12 2019 +0000

    Introduce update_client::NetworkFetcher as a way to abstract the network.

    This is a mechanical change and work in progress.

    The goal here is to decouple update client from its Chrome network
    dependencies such as //net and //services/network.

    This change avoids inejction of a direct dependency of the servicified
    network stack in favor of injecting a NetworkFetcherFactory, which
    in turns can create instances of NetworkFetcher, where they are needed.

    The next change will further isolate the implementation dependencies on
    the Chrome network, and allow for selecting different implementations
    of the NetworkFetcher.

    Bug: 929167

commit 4e0b41d09eeb9cf79def67b567fa2cf396d7f339
Author: Sorin Jianu <sorin@chromium.org>
Date:   Wed Feb 20 17:21:58 2019 +0000

    Decouple update_client::NetworkFetcher interface from Chromium network.

    This change is mechanical.

    It enables NetworkFetcherFactory, which is injected by the embedder, and
    which creates instances of NetworkFetcher.

    Bug: 929167
Debug/DCHECK compilation now uses [-Werror,-Wextra-semi] which causes
semicolons after function definitions or namespaces to be treated as
errors. For example, void foo() {}; would produce an error due to the
trailing semicolon.

Chromium change:

commit a02a8f4b72fe5abad3c123a9c52c6ef78940de45
Author: Nico Weber <thakis@chromium.org>
Date:   Sat Feb 23 04:13:21 2019 +0000

    Reland "Enable -Wextra-semi in non-cros non-chromecast non-fuzzer linux builds that have DCHECKs enabled."

    This is a reland of 5b911be859f216fe70bad5c41cc91a8684d773f6

    Original change's description:
    > Enable -Wextra-semi in non-cros non-chromecast non-fuzzer linux builds that have DCHECKs enabled.
    >
    > Only enables the warning for chromium_code.n
    >
    > Also fix the last few -Wextra-semi instances in sanitizer and linux/ozone builds.
    >
    > Getting to this state required fixing > 3000 unique warnings in 15+ different
    > repositories.  If this breaks some internal-only build somewhere, either consider
    > using no_chromium_code for your internal code, or fix the warnings -- it's
    > pretty easy. (Build with treat_warnings_as_errors=false, then open the error log
    > in vim, run `:%g/warning:/t$` to copy the warnings to the end of the buffer,
    > remove all but just those lines, run `:sort u` to get uniques, then save as
    > `tmp.txt` and run `:cf %` and then use something like `:map m :w<cr>:cn<cr>`
    > to go through them quickly.)
    >
    > TBR=rsesek
    >
    > Bug: 926235
    > Change-Id: Ica629737523ff6bbd756edc44d182f35ff6cf8ac
    > Reviewed-on: https://chromium-review.googlesource.com/c/1483396
    > Reviewed-by: Scott Violet <sky@chromium.org>
    > Auto-Submit: Nico Weber <thakis@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#634826}

    TBR=rsesek

    Bug: 926235, 935033
The functionality was called from override of
ChromeBrowserMainParts::PreShutdown, which was removed along with the
call to it from BrowserMainLoop::PreShutdown. This fix subclasses
BrowserMainLoop and calls BraveBrowserMainParts::PreShutdown from it.
It also patches BrowserMainParts to add virtual PreShutdown method
and BrowserMainRunnerImpl to instantiate BraveBrowserMainLoop instead
of BrowserMainLoop.

Chromium change:

commit 0f15e7a22545884fd259b873366a511686f4e5d9
Author: Alexei Filippov <alph@chromium.org>
Date:   Thu Feb 14 21:34:28 2019 +0000

    [heap profiler] Migrate UMA heap profiler off LegacyCallStackProfileBuilder

    ... and remove the latter as it was the last client.

    BUG=916303,913570
BraveBrowserCommandController::ExecuteCommandWithDisposition.

Chromium change:

commit 6f6fc1e333f968d4caddb52f11b6ff9f3ef1c0f9
Author: Edwin Joe <ejoe@google.com>
Date:   Wed Feb 27 20:00:37 2019 +0000

    Added a new UMA to track tab switching latency time

    Created a new TabSwitchEventLatencyRecorder class that will track
    the latency between input event timestamp and the time when tab
    switching begin processing, and upload the metric to a UMA.

    Changed the TabStripModel::ActivateTabAt method interface to take
    enough input event context from the Tab to the TabStripModel,
    and changed all TabStripModel::ActivateTabAt callsites accordingly.

    Bug: 921120
class constructor.

Also, browser_ pointer was moved to the parent class of
ProfileChooserView and now needs to be accessed via a getter.

Chromium change:

commit 6c85b61a18bad83016864a9a7b54c85d465aea90
Author: Ramin Halavati <rhalavati@chromium.org>
Date:   Thu Feb 28 09:28:03 2019 +0000

    Separate ProfileChooserView into two classes.

    ProfileChooserView prepares several menus, including different flavors
    of profile menu, guest menu, and incognito menu.

    The class is now divided into two classes, ProfileMenuViewBase which is
    in charge of the UI implementation and styling, and ProfileChooserView
    which decides about the elements in the menu based on the selected
    menu mode.

    Bug: 934689
Chromium change:

commit bf2ef6b1f33e8c1653c50a2614e90be61c5bcc2e
Author: Lucas Furukawa Gadani <lfg@chromium.org>
Date:   Wed Feb 20 18:38:39 2019 +0000

    Remove const interfaces from content::ResourceRequestInfo.

    Bug: 908139
secure/insecure.

Chromium change:

commit 44525ced669fe2c54d914d3df86c44453afd3402
Author: Maks Orlovich <morlovich@chromium.org>
Date:   Mon Feb 25 14:17:58 2019 +0000

    Reland "Take scheme in CookieStore::SetCanonicalCookieAsync, not just whether it's secure."

    This reverts commit 90c86f0ce73498a900dbb601f1de95dc519de18a.

    Justification for revert of revert: Fixed build/merged with a trunk change
    Bug:850044
A bool for user gesture or not has been replaces with a struct.
Opted to use GestureType::kOther to maintain the previous value of 'true' that
we were passing in.

Chromium change:

commit 6f6fc1e333f968d4caddb52f11b6ff9f3ef1c0f9
Author: Edwin Joe <ejoe@google.com>
Date:   Wed Feb 27 20:00:37 2019 +0000

    Added a new UMA to track tab switching latency time

    Created a new TabSwitchEventLatencyRecorder class that will track
    the latency between input event timestamp and the time when tab
    switching begin processing, and upload the metric to a UMA.

    Changed the TabStripModel::ActivateTabAt method interface to take
    enough input event context from the Tab to the TabStripModel,
    and changed all TabStripModel::ActivateTabAt callsites accordingly.

    Bug: 921120
Chromium change:

commit 9da196a5beb8fd4739939937953f9e8ba6f50770
Author: Lei Zhang <thestig@chromium.org>
Date:   Fri Mar 1 17:01:27 2019 +0000

    Make ShowConstrainedWebDialog() parameters unique_ptrs.

    Do the same for ConstrainedDialogWebView. Fix lint errors along the way.
For base::Bind
    base::DoNothing
    base::ThreadTaskRunnerHandle
Chromium change:

commit 708977c9da301d08676f218a2163f382e3607276
Author: Robert Liao <robliao@chromium.org>
Date:   Fri Mar 1 02:13:50 2019 +0000

    Rename view_properties.* to view_class_properties.*

    This is a mechanical execution of tools/git/mass-rename.py.

    BUG=937018
ChromeComponentExtensionResourceManager::AddComponentResourceEntries now takes
GzippedGritResourceMap, so all the maps we pass in
BraveComponentExtensionResourceManager need to be gzipped.

Updated .grd files map/header directives to gzipped_resource...
Updated BasicUI to consume GzippedGritResourceMap.

Chromium change:

commit 20b9be0f2dcbece3929c1a7fa8da6b4a3d925f73
Author: Sam McNally <sammc@chromium.org>
Date:   Wed Jan 30 02:29:00 2019 +0000

    Add support for gzipped component app/extension resources.

    Applied to the ink wasm module, this saves 3.9MB.

    If applied to all Files app, Gallery etc. html/css/js resources this
    could save a further 3.2MB.

    Bug: 923204
…num.

Chromium change:

commit 193c09db4eb3b886512b3c43dfa3432f0fcef719
Author: Yao Xiao <yaoxia@chromium.org>
Date:   Wed Feb 13 17:30:19 2019 +0000

    Change allow_download in ResourceRequestInfoImpl to an enum

    Motivation: Currently the |allow_download| flag in
    |ResourceRequestInfoImpl| can be set to false in the circumstances of
    Viewsource, Interstitial, etc. In all these cases, it's safe to
    skip intercepting the resource before plugins handler is set up
    (see MimeSniffingResourceHandler::MaybeStartInterception()). However,
    this may not always be the case if we want to exploit this flag to
    prevent download in other circumstances, e.g. in adframe. In this case,
    bypassing the setup of plugin handler will also prevent a PDF to
    display in adframe, although the restriction was to prevent just
    download in adframe.

    Therefore, I'm changing the boolean |allow_download| inside
    |ResourceRequestInfoImpl| to 3 state enum, which allows us to differentiate
    between a safe immediate prevention (Viewsource, Interstitial, etc.) that we
    have always been doing, and a prevention that should happen only after the
    plugin handler is checked and possibly set up (AdFrame).

    The following CL gives an idea of how the new enum will be used:
    https://chromium-review.googlesource.com/c/chromium/src/+/1367912

    Bug: 929911
Updated consequent calls to GetList to GetAsList (soon to go away).
GetList now returns a vector<base::Value> and more code will need to be
refactored once GetAsList goes away complitely.

Chromium change:

commit 9dd8968271bf31642e97511c0b3cca00534a1ee0
Author: Lei Zhang <thestig@chromium.org>
Date:   Tue Feb 19 20:14:53 2019 +0000

    Change base::JSONReader::ReadToValue() to return Optional.

    Update most unit tests to reflect this change, or change the
    JSONReader().ReadToValue() pattern to JSONReader::Read().
    Convert existing callers that did not get updated to use the deprecated
    version of the API.

    BUG=925165
… errors.

Debug/DCHECK compilation now uses [-Werror,-Wextra-semi] which causes
semicolons after function definitions or namespaces to be treated as
errors. For example, void foo() {}; would produce an error due to the
trailing semicolon.

Chromium change:

    commit a02a8f4b72fe5abad3c123a9c52c6ef78940de45
    Author: Nico Weber <thakis@chromium.org>
    Date:   Sat Feb 23 04:13:21 2019 +0000

        Reland "Enable -Wextra-semi in non-cros non-chromecast non-fuzzer linux builds that have DCHECKs enabled."

        This is a reland of 5b911be859f216fe70bad5c41cc91a8684d773f6

        Bug: 926235, 935033
Chromium change:

commit 572755bab76f7d4aa697fffebbdcb03ac92a44cd
Author: Alexandre Frechette <frechette@chromium.org>
Date:   Wed Feb 13 22:30:20 2019 +0000

    Moving the core language prefs (kAcceptLanguages and kPreferredLanguages) to components/language.

    Bug: 902354, 872096
This feature was disabled by brave/brave-browser#1051 to mitigate
Spectre. Spectre was later addressed by Chromium via site isolation and
Chromium re-enabled SharedArrayBuffer, but Brave didn't. Now in C74,
WebAssemblyThreads feature is being enabled by default. Since
WebAssemblyThreads is built with and auto-enables SharedArrayBuffer,
there is no reason for us to continue keeping SharedArrayBuffer
disabled.

Aslo, removed a browser test for checking that SharedArrayBuffer is
disabled.

Chromium change making WebAssemblyThreads enabled by default:

commit f9cc6b912e601070728b449892097cec0d59cbfa
Author: Ben Smith <binji@chromium.org>
Date:   Wed Feb 27 02:09:43 2019 +0000

    [wasm] Enable wasm threads by default

    See intent to ship blink-dev thread:
    https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/tD6np-OG2PU

    Bug: chromium:754910
When enabled this feature does not instantiate ChromeNetworkDelegate and
by its virtue BraveNetworkDelegateBase which is where we add hooks into
network stack.

Related: brave/brave-browser#2351

Chromium change to not instantiate ChromeNetworkDelegate:

commit  4b7d02f22b70c27f32fa5508f785ba310d08aad3
Author: John Abd-El-Malek < jam@chromium.org  >
Date: Thu May 03 17:46:35 2018

Don't instantiate ChromeNetworkDelegate when network service is enabled.

Bug:  598073

Chromium change in C74 to enable network service by default:

commit 708db9bc46bec1d0037f44f8fdaff7b7d72724e0
Author: John Abd-El-Malek <jam@chromium.org>
Date:   Mon Feb 25 18:10:35 2019 +0000

    Enable network service on Win/Mac/Linux by default as it launched on stable.

    Removing running integration tests on these bots (waterfall + CQ).

    Keep running the layout tests with network service disabled on on debug Linux Tests bots as we're still supporting ChromeOS/Android/Cast which haven't switched over yet.

    Also remove the disabled-service-worker-servicification virtual test.

    Bug: 933880,926114
@mkarolin mkarolin marked this pull request as ready for review March 20, 2019 20:54
@mkarolin mkarolin changed the title WIP: Upgrade Chromium from 73.0.3683.75 to Chromium 74.0.3729.22 Upgrade Chromium from 73.0.3683.75 to Chromium 74.0.3729.22 Mar 20, 2019
@@ -0,0 +1,12 @@
diff --git a/chrome/browser/resources/history/history_toolbar.html b/chrome/browser/resources/history/history_toolbar.html
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved from patches/chrome-browser-resources-md_history-history_toolbar.html.patch (deleted below).

@@ -10,7 +10,7 @@
namespace net {

int SOCKS5ClientSocket::DoAuth(int rv) {
rv = Authenticate(rv, *transport_, net_log_, io_callback_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @yrliou pls review this commit: "Updated SOCKS5 client socket overrides."

@@ -22,10 +22,12 @@
#include "components/prefs/pref_registry_simple.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jumde please review this commit: "Change to classes inheriting from update_client::Configurator."

@@ -145,6 +146,7 @@ bool BraveMainDelegate::BasicStartupComplete(int* exit_code) {
autofill::features::kAutofillServerCommunication.name,
features::kAudioServiceOutOfProcess.name,
features::kDefaultEnableOopRasterization.name,
network::features::kNetworkService.name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted an issue here to revert this and support the network service:
brave/brave-browser#3819

@bbondy
Copy link
Member

bbondy commented Mar 20, 2019

I edited Comment 0 to add a section "Special notes to QA:" with some things I wanted them to pay attention to based on the code changes.

@bbondy
Copy link
Member

bbondy commented Mar 20, 2019

Just noting that this will need an uplift to 0.63.x as well after it is merged to master. We should wait for Nightly builds for a couple days before merging in the Dev uplift though.

Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mkarolin mkarolin merged commit af85ea6 into master Mar 21, 2019
@mkarolin mkarolin deleted the 74.0.3729.22 branch March 21, 2019 14:23
mkarolin added a commit that referenced this pull request Mar 21, 2019
Upgrade Chromium from 73.0.3683.75 to Chromium 74.0.3729.22
@bsclifton bsclifton mentioned this pull request Mar 26, 2019
19 tasks
bsclifton added a commit that referenced this pull request Mar 27, 2019
(originally implemented with #2013)

Fixes brave/brave-browser#3898
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade from Chromium 73.0.3683.75 to Chromium 74.0.3729.40
4 participants