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.

Sign up for GitHub

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

Update wrong GOOGLE_CHROME_BUILD usage #3718

Merged
merged 2 commits into from
Oct 22, 2019
Merged

Update wrong GOOGLE_CHROME_BUILD usage #3718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf9cd47
Fixes our use of GOOGLE_CHROME_BUILD
bridiver Oct 21, 2019
f041122
lint fix
bsclifton Oct 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions browser/component_updater/brave_component_updater_configurator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,7 @@ BraveConfigurator::GetProtocolHandlerFactory() const {

update_client::RecoveryCRXElevator BraveConfigurator::GetRecoveryCRXElevator()
const {
#if defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems like this was wrong from the start and should have be IS_OFFICIAL_BUILD for us?

Copy link
Collaborator

@mkarolin mkarolin Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

GOOGLE_CHROME_BUILD was never defined for this file, as far as I can see, so the code inside was never used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bbondy is going to look into this for a potential follow-up

return base::BindOnce(&RunRecoveryCRXElevated);
#else
return {};
#endif
}

} // namespace component_updater
4 changes: 0 additions & 4 deletions browser/extensions/updater/brave_update_client_config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,7 @@ BraveUpdateClientConfig::GetProtocolHandlerFactory() const {

update_client::RecoveryCRXElevator
BraveUpdateClientConfig::GetRecoveryCRXElevator() const {
#if defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
return base::BindOnce(&RunRecoveryCRXElevated);
#else
return {};
#endif
}

BraveUpdateClientConfig::~BraveUpdateClientConfig() {}
Expand Down
6 changes: 3 additions & 3 deletions chromium_src/chrome/common/media/cdm_host_file_path.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@
#include "base/logging.h"
#include "base/path_service.h"
#include "base/stl_util.h"
#include "build/branding_buildflags.h"
#include "build/build_config.h"
#include "chrome/common/chrome_version.h"

#if defined(OS_MACOSX)
#include "base/mac/bundle_locations.h"
#include "chrome/common/buildflags.h"
#include "chrome/common/chrome_constants.h"
#endif

// All above headers copied from original cdm_host_file_path.cc are included
// to prevent below GOOGLE_CHROEM_BUILD affect them.

#define GOOGLE_CHROME_BUILD
mkarolin marked this conversation as resolved.
Show resolved Hide resolved
#undef BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING
#define BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING() (1)
#include "../../../../../chrome/common/media/cdm_host_file_path.cc" // NOLINT
#undef GOOGLE_CHROME_BUILD
21 changes: 13 additions & 8 deletions chromium_src/components/metrics/metrics_service_accessor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#if defined(BRAVE_CHROMIUM_BUILD)
#define GOOGLE_CHROME_BUILD
#endif
#include "components/metrics/metrics_service_accessor.h"

#include "../../../../components/metrics/metrics_service_accessor.cc"
#include "base/base_switches.h"
#include "base/command_line.h"
#include "build/branding_buildflags.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h"
#include "components/prefs/pref_service.h"
#include "components/variations/hashing.h"

#if defined(BRAVE_CHROMIUM_BUILD)
#undef GOOGLE_CHROME_BUILD
#endif
#undef BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING
#define BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING() (1)
#include "../../../../components/metrics/metrics_service_accessor.cc"
17 changes: 4 additions & 13 deletions patches/chrome-installer-util-google_update_settings.cc.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 441ed6d50d5cb4edcd665665fca7887f034ff6a0..709c7e5e24518902ae86ab1ef819dd9d86239d1a 100644
index 441ed6d50d5cb4edcd665665fca7887f034ff6a0..54a22d11c380ee31e12440900cf2a365781fe8d9 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -29,7 +29,11 @@ using base::win::RegKey;
Expand All @@ -14,23 +14,14 @@ index 441ed6d50d5cb4edcd665665fca7887f034ff6a0..709c7e5e24518902ae86ab1ef819dd9d
const wchar_t GoogleUpdateSettings::kUpdatePolicyValue[] = L"UpdateDefault";
const wchar_t GoogleUpdateSettings::kDownloadPreferencePolicyValue[] =
L"DownloadPreference";
@@ -41,6 +45,11 @@ const wchar_t GoogleUpdateSettings::kCheckPeriodOverrideMinutes[] =
@@ -41,6 +45,10 @@ const wchar_t GoogleUpdateSettings::kCheckPeriodOverrideMinutes[] =
const int GoogleUpdateSettings::kCheckPeriodOverrideMinutesMax =
60 * 24 * 7 * 6;

+
+#if defined(BRAVE_CHROMIUM_BUILD) && defined(OFFICIAL_BUILD)
+#define GOOGLE_CHROME_BUILD
+#undef BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING
+#define BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING() (1)
+#endif
+
const GoogleUpdateSettings::UpdatePolicy
GoogleUpdateSettings::kDefaultUpdatePolicy =
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
@@ -836,3 +845,7 @@ bool GoogleUpdateSettings::ReadExperimentLabels(

return result == ERROR_SUCCESS;
}
+
+#if defined(BRAVE_CHROMIUM_BUILD) && defined(OFFICIAL_BUILD)
+#undef GOOGLE_CHROME_BUILD
+#endif