-
Notifications
You must be signed in to change notification settings - Fork 904
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 70.0.3538.67 to 71.0.3578.20 #744
Conversation
329cb5c
to
8e76d3b
Compare
There were some cookie failures so I added a network delegate handler for reading and writing cookies to fix it. |
This is most easily reviewed per commit. All unit and browser tests fully pass. |
@@ -120,7 +120,6 @@ void BraveMainDelegate::PreSandboxStartup() { | |||
bool BraveMainDelegate::BasicStartupComplete(int* exit_code) { | |||
base::CommandLine& command_line = | |||
*base::CommandLine::ForCurrentProcess(); | |||
command_line.AppendSwitch(switches::kEnableTabAudioMuting); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't exist anymore.
@@ -28,8 +28,6 @@ source_set("extensions") { | |||
"brave_extension_service.h", | |||
"brave_tor_client_updater.cc", | |||
"brave_tor_client_updater.h", | |||
"brave_webstore_inline_installer.cc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline installer no longer exists.
@@ -20,7 +20,7 @@ BravePrompt::~BravePrompt() { | |||
base::string16 BravePrompt::GetDialogTitle() const { | |||
if (!extensions::BraveExtensionProvider::IsVetted(extension())) { | |||
if (type_ == ExtensionInstallPrompt::INSTALL_PROMPT || | |||
type_ == ExtensionInstallPrompt::INLINE_INSTALL_PROMPT) { | |||
type_ == ExtensionInstallPrompt::WEBSTORE_WIDGET_PROMPT) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INLINE_INSTALL_PROMPT doesn't exist, searching history log, their refs change to WEBSOTRE_WIDGET_PROMPT.
@@ -8,7 +8,7 @@ | |||
#include "extensions/browser/extension_function_registry.h" | |||
|
|||
// TOOD: I don't know why this isn't automatically linked in | |||
#include "gen/brave/common/extensions/api/generated_api_registration.cc" | |||
#include "../gen/brave/common/extensions/api/generated_api_registration.cc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a compiling error with #include <version>
because of a file in the root called version by having the root out path in the gen config, so I had to remove it and tweak includes like this
@@ -1,42 +0,0 @@ | |||
/* This Source Code Form is subject to the terms of the Mozilla Public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned inline install no longer possible, so code is removed.
base::Bind(&BraveNetworkDelegateBase::RunNextCallback, | ||
base::Unretained(this), request, ctx)); | ||
return net::ERR_IO_PENDING; | ||
} | ||
|
||
bool BraveNetworkDelegateBase::OnCanGetCookies(const URLRequest& request, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it turns out we weren't blocking all 3p cookies from the network service (even with C70). This fixes it.
#include "brave/browser/ui/content_settings/brave_autoplay_blocked_image_model.h" | ||
#include "third_party/widevine/cdm/buildflags.h" | ||
|
||
#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New build flag introduced in C71 which was causing build problems only for Linux. It's better we don't show this UI anyway on Linux since we don't have Widevine available yet there.
@@ -3,7 +3,6 @@ import("//brave/build/config.gni") | |||
# Changing these will cause a full rebuild | |||
brave_include_dirs_ = [ | |||
"//brave/chromium_src", | |||
root_out_dir, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment about #include <version>
|
||
// This chormium_src override allows us to skip a lot of patching to chrome/BUILD.gn | ||
#include "brave/app/brave_main_delegate.cc" | ||
#include "../../../../chrome/app/chrome_main_delegate.cc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided I was tired of rebasing that BUILD.gn file for all the brave_main_delegate.cc
references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When removing them(brave_main_delegate.[cc|h]
from chrome/BUILD.gn
, modifying them doesn't trigger rebuild.
If those files aren't changed frequently, how about adding some comment to those files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or how about adding them to proper BUILD.gn
in brave
folder?
Ah, chrome_main_delegate.cc
should be touched when they are modifed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should solve this in a general way of touching the changed files. Problem is there is a reference of the file in 10 different places otherwise.
base::FeatureList::IsEnabled(features::kEnableEphemeralFlashPermission) | ||
? ContentSettingsInfo::EPHEMERAL | ||
: ContentSettingsInfo::PERSISTENT); | ||
ContentSettingsInfo::EPHEMERAL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kEnableEphemeralFlashPermission doesn't exist now and EPHEMERAL is always assumed.
return false; | ||
} else if (resource_identifier == "trackers") { | ||
} else if (resource_identifier == brave_shields::kTrackers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why we didn't use these constants, but we should have been, so updated it.
Browser distribution string now uses IDS_ABOUT_VERSION_COMPANY_NAME See also Chromium commit: commit 31e1a2dbccc34a129ce779fea8d793b53f3ed945 Author: Greg Thompson <grt@chromium.org> Date: Wed Sep 5 11:15:44 2018 +0000 Move GetPublisherName from BrowserDistribution into InstallUtil. Chromium will now report "The Chromium Authors" rather than "Chromium" as its publisher name (as it should have all along). BUG=879568 --- Remove app_menu_controller.mm patch... Because: IsSecondaryUiMd() should always be true ShowAllDialogsWithViewsToolkit() should always be true And it doesn't exist anymore upstream. See also: commit a738bf5a7f9bb73fdcb4fff030f8b4069406c3fe Author: Robert Sesek <rsesek@chromium.org> Date: Mon Sep 24 01:28:08 2018 +0000 mac: Delete the Cocoa app menu. Bug: 832676
... and remove test reference
See in Chromium: commit 479cc17585a64910853e9949b053499ecbeca9a5 Author: Peter Kasting <pkasting@chromium.org> Date: Fri Sep 28 22:34:37 2018 +0000 Remove --enable-tab-audio-muting Per UX leads, this is dead. Bug: 752226
Per https://blog.chromium.org/2018/06/improving-extension-transparency-for.html See also: commit c4fb02304cc8ee7c6896f860f52fef72db1f069a Author: Benjamin Ackerman <ackermanb@chromium.org> Date: Wed Oct 3 19:55:41 2018 +0000 [Extensions] Obliterating the inline install API. Removing everything associated with inline installation as it is deprecated per the announcement in https://blog.chromium.org/2018/06/improving-extension-transparency-for.html
...with std <version> vs generated file version
TODO for Pete
See also: commit 8652dcd5d8f35d68f105136b3857e9b54148c31b Author: Eric Seckler <eseckler@chromium.org> Date: Thu Sep 20 10:42:28 2018 +0000 content: Replace uses of BrowserThread task posting with post_task.h API This patch updates callsites of BrowserThread task posting methods to use the post_task.h API instead. Background: We're changing the way tasks are posted to a BrowserThread, see PSA [1] and design doc [2]. This unifies the way tasks are posted and paves the way for annotating tasks with task types and other attributes that can be used to prioritize tasks in the future browser UI thread scheduler (design doc [3]). This CL changes callsites of the following forms: (a) BrowserThread::Post*Task(BrowserThread::UI/IO, ..) to base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI/IO}, ..), (b) BrowserThread::GetTaskRunnerForThread(BrowserThread::UI/IO) to base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI/IO}). It also adds necessary includes. These mechanical changes were applied by a script. Tasks posted with the same BrowserThread::ID trait (via PostTaskWithTraits or TaskRunners obtained from Create*TaskRunnerWithTraits) will still execute in the order they were posted, see [4].
To avoid new linking error by introduced build flag
--- commit 64df410d0ca79a8522d329e75ce8c21a9db77038 Author: Eric Roman <eroman@chromium.org> Date: Thu Oct 4 21:29:27 2018 +0000 Remove unused ProxyResolutionService::TryResolveProxySynchronously(). Bug: 721403 ---
Re-implements Brave's style for the New Tab Button against C71. Previously implemented against C70 in 966b6d5
Tests started to fail for cookie blockin C71, this makes it work again
Perhaps this simpler 4 line patch methodology will actually be simpler than the previous copy-and-patch method of the C70-targeted implementation de337bc#diff-d76fac9e942abff28ffedf5c3230d12d
Upgrade Chromium from 70.0.3538.67 to 71.0.3578.20
Fix brave/brave-browser#1753
Fix brave/brave-browser#1287
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests
) ongit rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist: