-
Notifications
You must be signed in to change notification settings - Fork 892
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
Remove usage of BRAVE_CHROMIUM_BUILD and brave_chromium_build #7969
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,13 @@ | ||
import("//brave/build/config.gni") | ||
|
||
# Changing these will cause a full rebuild | ||
brave_include_dirs_ = [ | ||
"//brave/chromium_src", | ||
] | ||
brave_include_dirs_ = [ "//brave/chromium_src" ] | ||
|
||
if (brave_chromium_build) { | ||
config("compiler") { | ||
include_dirs = brave_include_dirs_ | ||
defines = [ "BRAVE_CHROMIUM_BUILD" ] | ||
} | ||
} else { | ||
config("compiler") {} | ||
config("compiler") { | ||
include_dirs = brave_include_dirs_ | ||
} | ||
|
||
# do not expose this as a public config anywhere | ||
config("version") { | ||
defines = [ | ||
"BRAVE_CHROMIUM_VERSION=\"$chrome_version_string\"", | ||
] | ||
defines = [ "BRAVE_CHROMIUM_VERSION=\"$chrome_version_string\"" ] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,4 @@ | ||
declare_args() { | ||
brave_chromium_build = true | ||
|
||
brave_services_key = "" | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,14 +6,10 @@ | |
#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_WEBUI_SETTINGS_METRICS_REPORTING_HANDLER_H_ | ||
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_WEBUI_SETTINGS_METRICS_REPORTING_HANDLER_H_ | ||
|
||
#if defined(BRAVE_CHROMIUM_BUILD) | ||
#define GOOGLE_CHROME_BUILD | ||
#endif | ||
#include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h" | ||
|
||
#define GOOGLE_CHROME_BUILD | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, unrelated to this PR, but we should probably include all headers from the original file here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto (i.e. will do on a separate PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As clarified above, the change is small enough (and only affects these files) that I'm including it in this PR in the end. |
||
#include "../../../../../../../chrome/browser/ui/webui/settings/metrics_reporting_handler.h" | ||
|
||
#if defined(BRAVE_CHROMIUM_BUILD) | ||
#undef GOOGLE_CHROME_BUILD | ||
#endif | ||
|
||
#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_WEBUI_SETTINGS_METRICS_REPORTING_HANDLER_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
diff --git a/chrome/app/chrome_crash_reporter_client.cc b/chrome/app/chrome_crash_reporter_client.cc | ||
index b3a7aebffa224c360e3760f816887cb979999c57..cd8cf6856ae1fa1c28f1274ab2db1299d576e9f1 100644 | ||
index b3a7aebffa224c360e3760f816887cb979999c57..e56f309c199b87d63b32c33effb5e08017224f7f 100644 | ||
--- a/chrome/app/chrome_crash_reporter_client.cc | ||
+++ b/chrome/app/chrome_crash_reporter_client.cc | ||
@@ -154,7 +154,7 @@ bool ChromeCrashReporterClient::IsRunningUnattended() { | ||
} | ||
|
||
bool ChromeCrashReporterClient::GetCollectStatsConsent() { | ||
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING) | ||
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING) || (defined(BRAVE_CHROMIUM_BUILD) && defined(OFFICIAL_BUILD)) | ||
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING) || defined(OFFICIAL_BUILD) | ||
bool is_official_chrome_build = true; | ||
#else | ||
bool is_official_chrome_build = false; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,20 @@ | ||
diff --git a/chrome/app/theme/BUILD.gn b/chrome/app/theme/BUILD.gn | ||
index 57e8917e359a2fc59f567bd33f9340e400e55b1f..b127c994019d8ba1e5750b33c300c2ec9dcf3463 100644 | ||
index 57e8917e359a2fc59f567bd33f9340e400e55b1f..a4489e2c34d1a02138afb05ccfc72d0129572f3e 100644 | ||
--- a/chrome/app/theme/BUILD.gn | ||
+++ b/chrome/app/theme/BUILD.gn | ||
@@ -21,6 +21,7 @@ grit("theme_resources") { | ||
":chrome_unscaled_resources", | ||
"//ui/resources", | ||
] | ||
+ if (brave_chromium_build) { deps += ["//brave/app/theme:brave_theme_resources"]} | ||
+ deps += ["//brave/app/theme:brave_theme_resources"] | ||
|
||
output_dir = "$root_gen_dir/chrome" | ||
} | ||
@@ -32,6 +33,7 @@ grit("chrome_unscaled_resources") { | ||
"grit/chrome_unscaled_resources.h", | ||
"chrome_unscaled_resources.pak", | ||
] | ||
+ if (brave_chromium_build) { deps = ["//brave/app/theme:brave_unscaled_resources"]} | ||
+ deps = ["//brave/app/theme:brave_unscaled_resources"] | ||
|
||
output_dir = "$root_gen_dir/chrome" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,12 @@ | ||
diff --git a/chrome/browser/net/net_error_tab_helper.cc b/chrome/browser/net/net_error_tab_helper.cc | ||
index 313758372e0fcb842ee6a5b5cd69a64b8a95b2ac..cc920fb9459462075a1a9c123ae7af34ea5c8ac8 100644 | ||
index 313758372e0fcb842ee6a5b5cd69a64b8a95b2ac..bc8c3c243dd2f05b5fa9708d1d72bfa9f6dffcc0 100644 | ||
--- a/chrome/browser/net/net_error_tab_helper.cc | ||
+++ b/chrome/browser/net/net_error_tab_helper.cc | ||
@@ -200,6 +200,9 @@ void NetErrorTabHelper::InitializePref(WebContents* contents) { | ||
@@ -200,6 +200,7 @@ void NetErrorTabHelper::InitializePref(WebContents* contents) { | ||
} | ||
|
||
bool NetErrorTabHelper::ProbesAllowed() const { | ||
+#if defined(BRAVE_CHROMIUM_BUILD) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we could develop a way to avoid creation of certain tabhelpers, because it seems the whole thing is not needed for us There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I lack context to properly understand what you mean here. Could you please elaborate a bit more? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears to me that the whole point of this class is sending some sort of "probes" which we disable in this method. So instead of patching we could maybe avoid instantiating the helper (it's unclear to me how exactly, just a raw idea). And probably there are more unneeded helpers (or keyed services) like this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see what you mean now, thanks for the clarification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handled in brave/brave-browser#14219 |
||
+ return false; | ||
+#endif | ||
if (testing_state_ != TESTING_DEFAULT) | ||
return testing_state_ == TESTING_FORCE_ENABLED; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,12 @@ | ||
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc | ||
index 2f5ed4b15589ffa5bae18d7f06e6776f705f5229..62b9d86de52131eac490aeb41b55a057f4d9373e 100644 | ||
index 2f5ed4b15589ffa5bae18d7f06e6776f705f5229..a4bd6ab7ef01d4740211eff1de086fe1493f2129 100644 | ||
--- a/chrome/browser/profiles/profile_io_data.cc | ||
+++ b/chrome/browser/profiles/profile_io_data.cc | ||
@@ -248,6 +248,9 @@ bool ProfileIOData::IsHandledProtocol(const std::string& scheme) { | ||
@@ -248,6 +248,7 @@ bool ProfileIOData::IsHandledProtocol(const std::string& scheme) { | ||
#endif | ||
content::kChromeUIScheme, | ||
content::kChromeUIUntrustedScheme, | ||
+#if defined(BRAVE_CHROMIUM_BUILD) | ||
+ kBraveUIScheme, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to this PR, this can probably be done via a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually looked into doing that yesterday, but it was not clear to me how we would do it besides creating a define in the chromium_src override and then patching this file to include that define. It would be 1 line vs 3 lines in the patch, though, so perhaps still worth it. @mkarolin Could you clarify what you mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I better understood what you meant and got a PR already proposing a change, see #7995 |
||
+#endif | ||
url::kDataScheme, | ||
#if BUILDFLAG(IS_CHROMEOS_ASH) | ||
content::kExternalFileScheme, |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,12 @@ | ||
diff --git a/chrome/browser/ui/startup/startup_tab_provider.cc b/chrome/browser/ui/startup/startup_tab_provider.cc | ||
index b16e6b9deb6c951007ab6932f028982ca7c74671..b453e5b069bd4721e1ee857e71843b0a5494b2db 100644 | ||
index b16e6b9deb6c951007ab6932f028982ca7c74671..833666f7469189cb319ea7f1a5fb00ce182972d0 100644 | ||
--- a/chrome/browser/ui/startup/startup_tab_provider.cc | ||
+++ b/chrome/browser/ui/startup/startup_tab_provider.cc | ||
@@ -144,7 +144,11 @@ StartupTabs StartupTabProviderImpl::GetExtensionCheckupTabs( | ||
@@ -144,6 +144,7 @@ StartupTabs StartupTabProviderImpl::GetExtensionCheckupTabs( | ||
bool StartupTabProviderImpl::CanShowWelcome(bool is_signin_allowed, | ||
bool is_supervised_user, | ||
bool is_force_signin_enabled) { | ||
+#if defined(BRAVE_CHROMIUM_BUILD) | ||
+ return true; | ||
+#else | ||
return is_signin_allowed && !is_supervised_user && !is_force_signin_enabled; | ||
+#endif | ||
} | ||
|
||
// static |
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.
Unrelated to this PR, but we should probably include all headers from the original file here.
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'll prepare a separate PR for these changes. Thanks
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.
On a second thought, I think the change is small enough we can include it here (I thought there would be many other cases defining
GOOGLE_CHROME_BUILD
in other chromium_src overrides, but turns out it's only these two files). Let me know if you prefer it on a separate PR, though, but otherwise I'm merging that into this PR.