From 902a241a764a05c10a3164c29b80a8bdda2df60d Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 22 Aug 2018 09:32:44 +0900 Subject: [PATCH 1/7] Make our theme color chosen at first --- ...e-browser-themes-theme_properties.cc.patch | 20 --------------- ...rome-browser-themes-theme_service.cc.patch | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+), 20 deletions(-) delete mode 100644 patches/chrome-browser-themes-theme_properties.cc.patch create mode 100644 patches/chrome-browser-themes-theme_service.cc.patch diff --git a/patches/chrome-browser-themes-theme_properties.cc.patch b/patches/chrome-browser-themes-theme_properties.cc.patch deleted file mode 100644 index a8e6c7f15ffb..000000000000 --- a/patches/chrome-browser-themes-theme_properties.cc.patch +++ /dev/null @@ -1,20 +0,0 @@ -diff --git a/chrome/browser/themes/theme_properties.cc b/chrome/browser/themes/theme_properties.cc -index 733d3521fb2ecb8dd47c68cc9ca451752bdec04e..e52c5cecb77044d2b33bb2917e850da1fbd4a058 100644 ---- a/chrome/browser/themes/theme_properties.cc -+++ b/chrome/browser/themes/theme_properties.cc -@@ -10,6 +10,7 @@ - #include "base/optional.h" - #include "base/strings/string_split.h" - #include "base/strings/string_util.h" -+#include "brave/browser/themes/theme_properties.h" - #include "build/build_config.h" - #include "chrome/browser/themes/browser_theme_pack.h" - #include "ui/base/material_design/material_design_controller.h" -@@ -290,6 +291,7 @@ color_utils::HSL ThemeProperties::GetDefaultTint(int id, bool incognito) { - - // static - SkColor ThemeProperties::GetDefaultColor(int id, bool incognito) { -+ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito) - const base::Optional color = - MaybeGetDefaultColorForNewerMaterialUi(id, incognito); - if (color) diff --git a/patches/chrome-browser-themes-theme_service.cc.patch b/patches/chrome-browser-themes-theme_service.cc.patch new file mode 100644 index 000000000000..b6197c7fbd23 --- /dev/null +++ b/patches/chrome-browser-themes-theme_service.cc.patch @@ -0,0 +1,25 @@ +diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc +index 655be29bffb9c3c6abc063c255fac06aaa69951d..4d311afc35287b71ee65a8032470828edfb17f09 100644 +--- a/chrome/browser/themes/theme_service.cc ++++ b/chrome/browser/themes/theme_service.cc +@@ -64,6 +64,10 @@ + #include "chrome/browser/supervised_user/supervised_user_theme.h" + #endif + ++#if defined(BRAVE_CHROMIUM_BUILD) ++#include "brave/browser/themes/theme_properties.h" ++#endif ++ + using base::UserMetricsAction; + using content::BrowserThread; + using extensions::Extension; +@@ -157,6 +161,9 @@ gfx::ImageSkia* ThemeService::BrowserThemeProvider::GetImageSkiaNamed( + } + + SkColor ThemeService::BrowserThemeProvider::GetColor(int id) const { ++#if defined(BRAVE_CHROMIUM_BUILD) ++ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito_) ++#endif + return theme_service_.GetColor(id, incognito_); + } + From 1abed3d1de23e082839ff8078b07d46d8ef64cd5 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 22 Aug 2018 13:16:52 +0900 Subject: [PATCH 2/7] Introduce brave theme type prefs With kBraveThemeType, we can switch dark/light theme. --- browser/brave_profile_prefs.cc | 3 ++ browser/themes/BUILD.gn | 3 ++ .../themes/brave_theme_type_browsertest.cc | 37 +++++++++++++++++++ browser/themes/theme_properties.cc | 29 ++++++++++----- browser/themes/theme_properties.h | 8 ++-- browser/themes/theme_util.cc | 34 +++++++++++++++++ browser/themes/theme_util.h | 26 +++++++++++++ common/pref_names.cc | 1 + common/pref_names.h | 3 +- ...rome-browser-themes-theme_service.cc.patch | 4 +- test/BUILD.gn | 1 + 11 files changed, 134 insertions(+), 15 deletions(-) create mode 100644 browser/themes/brave_theme_type_browsertest.cc create mode 100644 browser/themes/theme_util.cc create mode 100644 browser/themes/theme_util.h diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 54b56e16f245..f29989e48757 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -5,6 +5,7 @@ #include "brave/browser/brave_profile_prefs.h" #include "brave/browser/alternate_private_search_engine_util.h" +#include "brave/browser/themes/theme_util.h" #include "brave/common/pref_names.h" #include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h" #include "chrome/browser/net/prediction_options.h" @@ -24,6 +25,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { RegisterAlternatePrivateSearchEngineProfilePrefs(registry); + RegisterProfilePrefsForBraveThemeType(registry); + registry->RegisterBooleanPref(kWidevineOptedIn, false); // No sign into Brave functionality diff --git a/browser/themes/BUILD.gn b/browser/themes/BUILD.gn index 620522bb232f..b02d019194dd 100644 --- a/browser/themes/BUILD.gn +++ b/browser/themes/BUILD.gn @@ -2,8 +2,11 @@ source_set("themes") { sources = [ "theme_properties.cc", "theme_properties.h", + "theme_util.cc", + "theme_util.h", ] deps = [ + "//net", "//skia", ] } diff --git a/browser/themes/brave_theme_type_browsertest.cc b/browser/themes/brave_theme_type_browsertest.cc new file mode 100644 index 000000000000..3ded3f356b6f --- /dev/null +++ b/browser/themes/brave_theme_type_browsertest.cc @@ -0,0 +1,37 @@ +/* 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/. */ + +#include "brave/browser/themes/theme_util.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/themes/theme_properties.h" +#include "chrome/browser/themes/theme_service.h" +#include "chrome/browser/ui/browser.h" + +using BraveThemeTypeTest = InProcessBrowserTest; + +IN_PROC_BROWSER_TEST_F(BraveThemeTypeTest, BraveThemeChangeTest) { + Profile* profile = browser()->profile(); +#if defined(OFFICIAL_BUILD) + const SkColor light_frame_color = SkColorSetRGB(0xD8, 0xDE, 0xE1); +#endif + const SkColor dark_frame_color = SkColorSetRGB(0x58, 0x5B, 0x5E); + + // Check default type is set initially. + EXPECT_EQ(BRAVE_THEME_TYPE_DEFAULT, GetBraveThemeType(profile)); + + const ui::ThemeProvider& tp = ThemeService::GetThemeProviderForProfile(profile); + SetBraveThemeType(browser()->profile(), BRAVE_THEME_TYPE_LIGHT); + EXPECT_EQ(BRAVE_THEME_TYPE_LIGHT, GetBraveThemeType(profile)); +#if defined(OFFICIAL_BUILD) + EXPECT_EQ(light_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); +#else + // Non-official build always uses dark theme. + EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); +#endif + + SetBraveThemeType(browser()->profile(), BRAVE_THEME_TYPE_DARK); + EXPECT_EQ(BRAVE_THEME_TYPE_DARK, GetBraveThemeType(profile)); + EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); +} \ No newline at end of file diff --git a/browser/themes/theme_properties.cc b/browser/themes/theme_properties.cc index 4155a676c1f3..2e37a77cd54d 100644 --- a/browser/themes/theme_properties.cc +++ b/browser/themes/theme_properties.cc @@ -4,10 +4,11 @@ #include "brave/browser/themes/theme_properties.h" +#include "brave/browser/themes/theme_util.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/common/channel_info.h" #include "components/version_info/channel.h" -#include "ui/gfx/color_palette.h" namespace { @@ -70,22 +71,32 @@ base::Optional MaybeGetDefaultColorForBraveUiDevChannel(int id, bool in } // namespace // Returns a |nullopt| if the UI color is not handled by Brave. -base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito) { +base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile) { if (id == BRAVE_COLOR_FOR_TEST) { return SkColorSetRGB(11, 13, 17); } #if !defined(OFFICIAL_BUILD) return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); #else - switch (chrome::GetChannel()) { - case version_info::Channel::STABLE: - case version_info::Channel::BETA: + switch (GetBraveThemeType(profile)) { + case BRAVE_THEME_TYPE_DEFAULT: + switch (chrome::GetChannel()) { + case version_info::Channel::STABLE: + case version_info::Channel::BETA: + return MaybeGetDefaultColorForBraveUiReleaseChannel(id, incognito); + case version_info::Channel::DEV: + case version_info::Channel::CANARY: + case version_info::Channel::UNKNOWN: + default: + return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); + } + case BRAVE_THEME_TYPE_LIGHT: return MaybeGetDefaultColorForBraveUiReleaseChannel(id, incognito); - case version_info::Channel::DEV: - case version_info::Channel::CANARY: - case version_info::Channel::UNKNOWN: - default: + case BRAVE_THEME_TYPE_DARK: return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); + default: + NOTREACHED(); + } #endif return base::nullopt; diff --git a/browser/themes/theme_properties.h b/browser/themes/theme_properties.h index f8da9d48485c..f945166d23ea 100644 --- a/browser/themes/theme_properties.h +++ b/browser/themes/theme_properties.h @@ -8,12 +8,14 @@ #include "base/optional.h" #include "third_party/skia/include/core/SkColor.h" +class Profile; + #define BRAVE_COLOR_FOR_TEST 0x7FFFFFFF -base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito); +base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile); -#define MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito) \ - const base::Optional braveColor = MaybeGetDefaultColorForBraveUi(id, incognito); \ +#define MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito, profile) \ + const base::Optional braveColor = MaybeGetDefaultColorForBraveUi(id, incognito, profile); \ if (braveColor) return braveColor.value(); #endif // BRAVE_BROWSER_THEMES_THEME_PROPERTIES_H_ diff --git a/browser/themes/theme_util.cc b/browser/themes/theme_util.cc new file mode 100644 index 000000000000..e234659638f3 --- /dev/null +++ b/browser/themes/theme_util.cc @@ -0,0 +1,34 @@ +/* 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/. */ + +#include "brave/browser/themes/theme_util.h" + +#include "brave/common/pref_names.h" +#include "chrome/browser/chrome_notification_types.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/themes/theme_service.h" +#include "chrome/browser/themes/theme_service_factory.h" +#include "components/prefs/pref_service.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "content/public/browser/notification_service.h" + +int GetBraveThemeType(Profile* profile) { + return profile->GetPrefs()->GetInteger(kBraveThemeType); +} + +void SetBraveThemeType(Profile* profile, BraveThemeType type) { + profile->GetPrefs()->SetInteger(kBraveThemeType, type); + + content::NotificationService* service = + content::NotificationService::current(); + service->Notify(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, + content::Source( + ThemeServiceFactory::GetForProfile(profile)), + content::NotificationService::NoDetails()); +} + +void RegisterProfilePrefsForBraveThemeType( + user_prefs::PrefRegistrySyncable* registry) { + registry->RegisterIntegerPref(kBraveThemeType, BRAVE_THEME_TYPE_DEFAULT); +} diff --git a/browser/themes/theme_util.h b/browser/themes/theme_util.h new file mode 100644 index 000000000000..fc9332c864c1 --- /dev/null +++ b/browser/themes/theme_util.h @@ -0,0 +1,26 @@ +/* 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/. */ + +#ifndef BRAVE_BROWSER_THEMES_THEME_UTIL_H_ +#define BRAVE_BROWSER_THEMES_THEME_UTIL_H_ + +class Profile; + +namespace user_prefs { +class PrefRegistrySyncable; +} + +enum BraveThemeType { + BRAVE_THEME_TYPE_DEFAULT, // Choose theme by channel + BRAVE_THEME_TYPE_DARK, // Use dark theme regardless of channel + BRAVE_THEME_TYPE_LIGHT, // Use light theme regardless of channel +}; + +int GetBraveThemeType(Profile* profile); + +void SetBraveThemeType(Profile* profile, BraveThemeType type); + +void RegisterProfilePrefsForBraveThemeType(user_prefs::PrefRegistrySyncable* registry); + +#endif // BRAVE_BROWSER_THEMES_THEME_UTIL_H_ diff --git a/common/pref_names.cc b/common/pref_names.cc index d386f68ffdcc..9205048e4e24 100644 --- a/common/pref_names.cc +++ b/common/pref_names.cc @@ -18,3 +18,4 @@ const char kAdBlockCurrentRegion[] = "brave.ad_block.current_region"; const char kWidevineOptedIn[] = "brave.widevine_opted_in"; const char kUseAlternatePrivateSearchEngine[] = "brave.use_alternate_private_search_engine"; +const char kBraveThemeType[] = "brave.theme.type"; diff --git a/common/pref_names.h b/common/pref_names.h index 1740bb9ec239..98b1d7baa35c 100644 --- a/common/pref_names.h +++ b/common/pref_names.h @@ -18,5 +18,6 @@ extern const char kWeekOfInstallation[]; extern const char kAdBlockCurrentRegion[]; extern const char kWidevineOptedIn[]; extern const char kUseAlternatePrivateSearchEngine[]; +extern const char kBraveThemeType[]; -#endif +#endif // BRAVE_COMMON_PREF_NAMES_H_ diff --git a/patches/chrome-browser-themes-theme_service.cc.patch b/patches/chrome-browser-themes-theme_service.cc.patch index b6197c7fbd23..96879ee7273c 100644 --- a/patches/chrome-browser-themes-theme_service.cc.patch +++ b/patches/chrome-browser-themes-theme_service.cc.patch @@ -1,5 +1,5 @@ diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc -index 655be29bffb9c3c6abc063c255fac06aaa69951d..4d311afc35287b71ee65a8032470828edfb17f09 100644 +index 655be29bffb9c3c6abc063c255fac06aaa69951d..5a0366963f8c39cc32f65ce55533d38de8e38b74 100644 --- a/chrome/browser/themes/theme_service.cc +++ b/chrome/browser/themes/theme_service.cc @@ -64,6 +64,10 @@ @@ -18,7 +18,7 @@ index 655be29bffb9c3c6abc063c255fac06aaa69951d..4d311afc35287b71ee65a8032470828e SkColor ThemeService::BrowserThemeProvider::GetColor(int id) const { +#if defined(BRAVE_CHROMIUM_BUILD) -+ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito_) ++ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito_, theme_service_.profile()) +#endif return theme_service_.GetColor(id, incognito_); } diff --git a/test/BUILD.gn b/test/BUILD.gn index 2dbfe4e09340..fd3445199359 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -170,6 +170,7 @@ test("brave_browser_tests") { "//brave/browser/extensions/brave_extension_provider_browsertest.cc", "//brave/renderer/brave_content_settings_observer_browsertest.cc", "//brave/renderer/brave_content_settings_observer_flash_browsertest.cc", + "//brave/browser/themes/brave_theme_type_browsertest.cc", "//chrome/browser/extensions/browsertest_util.cc", "//chrome/browser/extensions/browsertest_util.h", "//chrome/browser/extensions/extension_browsertest.cc", From cffca8df4013fe72ab56d953f6b32d764422a7ab Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 22 Aug 2018 19:39:12 +0900 Subject: [PATCH 3/7] Call brave method for overring default theme color in GetColor With this, theme extension have a chance to override theme color. --- .../chrome-browser-themes-theme_service.cc.patch | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/patches/chrome-browser-themes-theme_service.cc.patch b/patches/chrome-browser-themes-theme_service.cc.patch index 96879ee7273c..581822f9d5ba 100644 --- a/patches/chrome-browser-themes-theme_service.cc.patch +++ b/patches/chrome-browser-themes-theme_service.cc.patch @@ -1,5 +1,5 @@ diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc -index 655be29bffb9c3c6abc063c255fac06aaa69951d..5a0366963f8c39cc32f65ce55533d38de8e38b74 100644 +index 655be29bffb9c3c6abc063c255fac06aaa69951d..850c8b93645763985d7dd8d6f4e11a4f92c636a1 100644 --- a/chrome/browser/themes/theme_service.cc +++ b/chrome/browser/themes/theme_service.cc @@ -64,6 +64,10 @@ @@ -13,13 +13,14 @@ index 655be29bffb9c3c6abc063c255fac06aaa69951d..5a0366963f8c39cc32f65ce55533d38d using base::UserMetricsAction; using content::BrowserThread; using extensions::Extension; -@@ -157,6 +161,9 @@ gfx::ImageSkia* ThemeService::BrowserThemeProvider::GetImageSkiaNamed( - } +@@ -771,6 +775,10 @@ SkColor ThemeService::GetColor(int id, + return color; + } - SkColor ThemeService::BrowserThemeProvider::GetColor(int id) const { +#if defined(BRAVE_CHROMIUM_BUILD) -+ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito_, theme_service_.profile()) ++ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito, profile()) +#endif - return theme_service_.GetColor(id, incognito_); ++ + return GetDefaultColor(id, incognito); } From f98349c33e7949fc6e8de97e4e8b238d0764cd25 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Thu, 23 Aug 2018 11:41:36 +0900 Subject: [PATCH 4/7] Subclassing ThemeService to override GetDefaultColor() BraveThemeService became subclass of ThemeServiceAuraX11 and ThemeServiceWin. Also apis in theme_util.h are moved to static methods of BraveThemeService. --- browser/brave_profile_prefs.cc | 4 +- browser/themes/BUILD.gn | 9 +++- browser/themes/brave_theme_service.cc | 41 +++++++++++++++++++ browser/themes/brave_theme_service.h | 37 +++++++++++++++++ ....cc => brave_theme_service_browsertest.cc} | 19 +++++---- browser/themes/brave_theme_service_win.cc | 16 ++++++++ browser/themes/brave_theme_service_win.h | 22 ++++++++++ browser/themes/theme_properties.cc | 10 ++--- browser/themes/theme_properties.h | 4 -- browser/themes/theme_util.cc | 34 --------------- browser/themes/theme_util.h | 26 ------------ .../browser/themes/theme_service_aurax11.h | 10 +++++ .../browser/themes/theme_service_factory.cc | 18 ++++++++ .../chrome/browser/themes/theme_service_win.h | 23 +++++++++++ ...rome-browser-themes-theme_service.cc.patch | 26 ------------ test/BUILD.gn | 2 +- 16 files changed, 192 insertions(+), 109 deletions(-) create mode 100644 browser/themes/brave_theme_service.cc create mode 100644 browser/themes/brave_theme_service.h rename browser/themes/{brave_theme_type_browsertest.cc => brave_theme_service_browsertest.cc} (65%) create mode 100644 browser/themes/brave_theme_service_win.cc create mode 100644 browser/themes/brave_theme_service_win.h delete mode 100644 browser/themes/theme_util.cc delete mode 100644 browser/themes/theme_util.h create mode 100644 chromium_src/chrome/browser/themes/theme_service_aurax11.h create mode 100644 chromium_src/chrome/browser/themes/theme_service_factory.cc create mode 100644 chromium_src/chrome/browser/themes/theme_service_win.h delete mode 100644 patches/chrome-browser-themes-theme_service.cc.patch diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index f29989e48757..53c0e72467e7 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -5,7 +5,7 @@ #include "brave/browser/brave_profile_prefs.h" #include "brave/browser/alternate_private_search_engine_util.h" -#include "brave/browser/themes/theme_util.h" +#include "brave/browser/themes/brave_theme_service.h" #include "brave/common/pref_names.h" #include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h" #include "chrome/browser/net/prediction_options.h" @@ -25,7 +25,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { RegisterAlternatePrivateSearchEngineProfilePrefs(registry); - RegisterProfilePrefsForBraveThemeType(registry); + BraveThemeService::RegisterProfilePrefs(registry); registry->RegisterBooleanPref(kWidevineOptedIn, false); diff --git a/browser/themes/BUILD.gn b/browser/themes/BUILD.gn index b02d019194dd..bd972c93de6b 100644 --- a/browser/themes/BUILD.gn +++ b/browser/themes/BUILD.gn @@ -1,11 +1,16 @@ source_set("themes") { sources = [ + "brave_theme_service.cc", + "brave_theme_service.h", + "brave_theme_service_win.cc", + "brave_theme_service_win.h", "theme_properties.cc", "theme_properties.h", - "theme_util.cc", - "theme_util.h", ] + deps = [ + "//base", + "//components/version_info", "//net", "//skia", ] diff --git a/browser/themes/brave_theme_service.cc b/browser/themes/brave_theme_service.cc new file mode 100644 index 000000000000..8f50ffe4d791 --- /dev/null +++ b/browser/themes/brave_theme_service.cc @@ -0,0 +1,41 @@ +/* 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/. */ + +#include "brave/browser/themes/brave_theme_service.h" + +#include "brave/browser/themes/theme_properties.h" +#include "brave/common/pref_names.h" +#include "chrome/browser/themes/theme_service_factory.h" +#include "chrome/browser/profiles/profile.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "components/prefs/pref_service.h" + +// static +void BraveThemeService::RegisterProfilePrefs( + user_prefs::PrefRegistrySyncable* registry) { + registry->RegisterIntegerPref(kBraveThemeType, BRAVE_THEME_TYPE_DEFAULT); +} + +// static +int BraveThemeService::GetBraveThemeType(Profile* profile) { + return profile->GetPrefs()->GetInteger(kBraveThemeType); +} + +// static +void BraveThemeService::SetBraveThemeType(Profile* profile, + BraveThemeType type) { + profile->GetPrefs()->SetInteger(kBraveThemeType, type); + + static_cast(ThemeServiceFactory::GetForProfile(profile)) + ->NotifyThemeChanged(); +} + +SkColor BraveThemeService::GetDefaultColor(int id, bool incognito) const { + const base::Optional braveColor = + MaybeGetDefaultColorForBraveUi(id, incognito, profile()); + if (braveColor) + return braveColor.value(); + + return ThemeService::GetDefaultColor(id, incognito); +} diff --git a/browser/themes/brave_theme_service.h b/browser/themes/brave_theme_service.h new file mode 100644 index 000000000000..4683a9ca7a76 --- /dev/null +++ b/browser/themes/brave_theme_service.h @@ -0,0 +1,37 @@ +/* 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/. */ + +#ifndef BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_H_ +#define BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_H_ + +#include "chrome/browser/themes/theme_service.h" + +namespace user_prefs { +class PrefRegistrySyncable; +} + +class BraveThemeService : public ThemeService { + public: + enum BraveThemeType { + BRAVE_THEME_TYPE_DEFAULT, // Choose theme by channel + BRAVE_THEME_TYPE_DARK, // Use dark theme regardless of channel + BRAVE_THEME_TYPE_LIGHT, // Use light theme regardless of channel + }; + + static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); + static int GetBraveThemeType(Profile* profile); + static void SetBraveThemeType(Profile* profile, BraveThemeType type); + + BraveThemeService() = default; + ~BraveThemeService() override = default; + + protected: + // ThemeService overrides: + SkColor GetDefaultColor(int id, bool incognito) const override; + + private: + DISALLOW_COPY_AND_ASSIGN(BraveThemeService); +}; + +#endif // BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_H_ diff --git a/browser/themes/brave_theme_type_browsertest.cc b/browser/themes/brave_theme_service_browsertest.cc similarity index 65% rename from browser/themes/brave_theme_type_browsertest.cc rename to browser/themes/brave_theme_service_browsertest.cc index 3ded3f356b6f..51861f44348d 100644 --- a/browser/themes/brave_theme_type_browsertest.cc +++ b/browser/themes/brave_theme_service_browsertest.cc @@ -2,16 +2,17 @@ * 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/. */ -#include "brave/browser/themes/theme_util.h" +#include "brave/browser/themes/brave_theme_service.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_service.h" #include "chrome/browser/ui/browser.h" -using BraveThemeTypeTest = InProcessBrowserTest; +using BraveThemeServiceTest = InProcessBrowserTest; +using BTS = BraveThemeService; -IN_PROC_BROWSER_TEST_F(BraveThemeTypeTest, BraveThemeChangeTest) { +IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, BraveThemeChangeTest) { Profile* profile = browser()->profile(); #if defined(OFFICIAL_BUILD) const SkColor light_frame_color = SkColorSetRGB(0xD8, 0xDE, 0xE1); @@ -19,11 +20,11 @@ IN_PROC_BROWSER_TEST_F(BraveThemeTypeTest, BraveThemeChangeTest) { const SkColor dark_frame_color = SkColorSetRGB(0x58, 0x5B, 0x5E); // Check default type is set initially. - EXPECT_EQ(BRAVE_THEME_TYPE_DEFAULT, GetBraveThemeType(profile)); + EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DEFAULT, BTS::GetBraveThemeType(profile)); const ui::ThemeProvider& tp = ThemeService::GetThemeProviderForProfile(profile); - SetBraveThemeType(browser()->profile(), BRAVE_THEME_TYPE_LIGHT); - EXPECT_EQ(BRAVE_THEME_TYPE_LIGHT, GetBraveThemeType(profile)); + BTS::SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_LIGHT); + EXPECT_EQ(BTS::BRAVE_THEME_TYPE_LIGHT, BTS::GetBraveThemeType(profile)); #if defined(OFFICIAL_BUILD) EXPECT_EQ(light_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); #else @@ -31,7 +32,7 @@ IN_PROC_BROWSER_TEST_F(BraveThemeTypeTest, BraveThemeChangeTest) { EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); #endif - SetBraveThemeType(browser()->profile(), BRAVE_THEME_TYPE_DARK); - EXPECT_EQ(BRAVE_THEME_TYPE_DARK, GetBraveThemeType(profile)); + BTS::SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_DARK); + EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DARK, BTS::GetBraveThemeType(profile)); EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); -} \ No newline at end of file +} diff --git a/browser/themes/brave_theme_service_win.cc b/browser/themes/brave_theme_service_win.cc new file mode 100644 index 000000000000..27e68f31807d --- /dev/null +++ b/browser/themes/brave_theme_service_win.cc @@ -0,0 +1,16 @@ +/* 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/. */ + +#include "brave/browser/themes/brave_theme_service_win.h" + +#include "chrome/browser/themes/theme_properties.h" + +SkColor BraveThemeServiceWin::GetDefaultColor(int id, bool incognito) const { + // Prevent dcheck in chrome/browser/themes/theme_properties.cc(384) + // It assumes this id handled in theme service. + if (DwmColorsAllowed() && id == ThemeProperties::COLOR_ACCENT_BORDER) + return dwm_accent_border_color_; + // Skip ThemeServiceWin::GetDefaultColor() to prevent using dwm frame color. + return BraveThemeService::GetDefaultColor(id, incognito); +} diff --git a/browser/themes/brave_theme_service_win.h b/browser/themes/brave_theme_service_win.h new file mode 100644 index 000000000000..c7489dc61835 --- /dev/null +++ b/browser/themes/brave_theme_service_win.h @@ -0,0 +1,22 @@ +/* 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/. */ + +#ifndef BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_WIN_H_ +#define BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_WIN_H_ + +#include "chrome/browser/themes/theme_service_win.h" + +class BraveThemeServiceWin : public ThemeServiceWin { + public: + BraveThemeServiceWin() = default; + ~BraveThemeServiceWin() override = default; + + private: + // ThemeServiceWin overrides: + SkColor GetDefaultColor(int id, bool incognito) const override; + + DISALLOW_COPY_AND_ASSIGN(BraveThemeServiceWin); +}; + +#endif // BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_WIN_H_ diff --git a/browser/themes/theme_properties.cc b/browser/themes/theme_properties.cc index 2e37a77cd54d..83983aeeb644 100644 --- a/browser/themes/theme_properties.cc +++ b/browser/themes/theme_properties.cc @@ -4,7 +4,7 @@ #include "brave/browser/themes/theme_properties.h" -#include "brave/browser/themes/theme_util.h" +#include "brave/browser/themes/brave_theme_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/common/channel_info.h" @@ -78,8 +78,8 @@ base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, P #if !defined(OFFICIAL_BUILD) return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); #else - switch (GetBraveThemeType(profile)) { - case BRAVE_THEME_TYPE_DEFAULT: + switch (BraveThemeService::GetBraveThemeType(profile)) { + case BraveThemeService::BRAVE_THEME_TYPE_DEFAULT: switch (chrome::GetChannel()) { case version_info::Channel::STABLE: case version_info::Channel::BETA: @@ -90,9 +90,9 @@ base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, P default: return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); } - case BRAVE_THEME_TYPE_LIGHT: + case BraveThemeService::BRAVE_THEME_TYPE_LIGHT: return MaybeGetDefaultColorForBraveUiReleaseChannel(id, incognito); - case BRAVE_THEME_TYPE_DARK: + case BraveThemeService::BRAVE_THEME_TYPE_DARK: return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); default: NOTREACHED(); diff --git a/browser/themes/theme_properties.h b/browser/themes/theme_properties.h index f945166d23ea..7dd7a7bbad7d 100644 --- a/browser/themes/theme_properties.h +++ b/browser/themes/theme_properties.h @@ -14,8 +14,4 @@ class Profile; base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile); -#define MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito, profile) \ - const base::Optional braveColor = MaybeGetDefaultColorForBraveUi(id, incognito, profile); \ - if (braveColor) return braveColor.value(); - #endif // BRAVE_BROWSER_THEMES_THEME_PROPERTIES_H_ diff --git a/browser/themes/theme_util.cc b/browser/themes/theme_util.cc deleted file mode 100644 index e234659638f3..000000000000 --- a/browser/themes/theme_util.cc +++ /dev/null @@ -1,34 +0,0 @@ -/* 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/. */ - -#include "brave/browser/themes/theme_util.h" - -#include "brave/common/pref_names.h" -#include "chrome/browser/chrome_notification_types.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/themes/theme_service.h" -#include "chrome/browser/themes/theme_service_factory.h" -#include "components/prefs/pref_service.h" -#include "components/pref_registry/pref_registry_syncable.h" -#include "content/public/browser/notification_service.h" - -int GetBraveThemeType(Profile* profile) { - return profile->GetPrefs()->GetInteger(kBraveThemeType); -} - -void SetBraveThemeType(Profile* profile, BraveThemeType type) { - profile->GetPrefs()->SetInteger(kBraveThemeType, type); - - content::NotificationService* service = - content::NotificationService::current(); - service->Notify(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, - content::Source( - ThemeServiceFactory::GetForProfile(profile)), - content::NotificationService::NoDetails()); -} - -void RegisterProfilePrefsForBraveThemeType( - user_prefs::PrefRegistrySyncable* registry) { - registry->RegisterIntegerPref(kBraveThemeType, BRAVE_THEME_TYPE_DEFAULT); -} diff --git a/browser/themes/theme_util.h b/browser/themes/theme_util.h deleted file mode 100644 index fc9332c864c1..000000000000 --- a/browser/themes/theme_util.h +++ /dev/null @@ -1,26 +0,0 @@ -/* 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/. */ - -#ifndef BRAVE_BROWSER_THEMES_THEME_UTIL_H_ -#define BRAVE_BROWSER_THEMES_THEME_UTIL_H_ - -class Profile; - -namespace user_prefs { -class PrefRegistrySyncable; -} - -enum BraveThemeType { - BRAVE_THEME_TYPE_DEFAULT, // Choose theme by channel - BRAVE_THEME_TYPE_DARK, // Use dark theme regardless of channel - BRAVE_THEME_TYPE_LIGHT, // Use light theme regardless of channel -}; - -int GetBraveThemeType(Profile* profile); - -void SetBraveThemeType(Profile* profile, BraveThemeType type); - -void RegisterProfilePrefsForBraveThemeType(user_prefs::PrefRegistrySyncable* registry); - -#endif // BRAVE_BROWSER_THEMES_THEME_UTIL_H_ diff --git a/chromium_src/chrome/browser/themes/theme_service_aurax11.h b/chromium_src/chrome/browser/themes/theme_service_aurax11.h new file mode 100644 index 000000000000..5330250f8900 --- /dev/null +++ b/chromium_src/chrome/browser/themes/theme_service_aurax11.h @@ -0,0 +1,10 @@ +/* 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/. */ + +#include "brave/browser/themes/brave_theme_service.h" + +#undef ThemeService +#define ThemeService BraveThemeService +#include "../../../../../chrome/browser/themes/theme_service_aurax11.h" +#undef ThemeService diff --git a/chromium_src/chrome/browser/themes/theme_service_factory.cc b/chromium_src/chrome/browser/themes/theme_service_factory.cc new file mode 100644 index 000000000000..0fb350c60bd4 --- /dev/null +++ b/chromium_src/chrome/browser/themes/theme_service_factory.cc @@ -0,0 +1,18 @@ +/* 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/. */ + +#include "build/build_config.h" +#include "chrome/browser/themes/theme_service.h" + +#if defined(OS_WIN) +#include "brave/browser/themes/brave_theme_service_win.h" +#undef ThemeServiceWin +#define ThemeServiceWin BraveThemeServiceWin +#elif !defined(USE_X11) +#include "brave/browser/themes/brave_theme_service.h" +#undef ThemeService +#define ThemeService BraveThemeService +#endif + +#include "../../../../../chrome/browser/themes/theme_service_factory.cc" diff --git a/chromium_src/chrome/browser/themes/theme_service_win.h b/chromium_src/chrome/browser/themes/theme_service_win.h new file mode 100644 index 000000000000..394009171b15 --- /dev/null +++ b/chromium_src/chrome/browser/themes/theme_service_win.h @@ -0,0 +1,23 @@ +/* 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/. */ + +#include "brave/browser/themes/brave_theme_service.h" + +#undef DISALLOW_COPY_AND_ASSIGN +#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ + DISALLOW_COPY(TypeName); \ + DISALLOW_ASSIGN(TypeName); \ + friend class BraveThemeServiceWin + +#undef ThemeService +#define ThemeService BraveThemeService + +#include "../../../../../chrome/browser/themes/theme_service_win.h" + +#undef DISALLOW_COPY_AND_ASSIGN +#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ + DISALLOW_COPY(TypeName); \ + DISALLOW_ASSIGN(TypeName) + +#undef ThemeService diff --git a/patches/chrome-browser-themes-theme_service.cc.patch b/patches/chrome-browser-themes-theme_service.cc.patch deleted file mode 100644 index 581822f9d5ba..000000000000 --- a/patches/chrome-browser-themes-theme_service.cc.patch +++ /dev/null @@ -1,26 +0,0 @@ -diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc -index 655be29bffb9c3c6abc063c255fac06aaa69951d..850c8b93645763985d7dd8d6f4e11a4f92c636a1 100644 ---- a/chrome/browser/themes/theme_service.cc -+++ b/chrome/browser/themes/theme_service.cc -@@ -64,6 +64,10 @@ - #include "chrome/browser/supervised_user/supervised_user_theme.h" - #endif - -+#if defined(BRAVE_CHROMIUM_BUILD) -+#include "brave/browser/themes/theme_properties.h" -+#endif -+ - using base::UserMetricsAction; - using content::BrowserThread; - using extensions::Extension; -@@ -771,6 +775,10 @@ SkColor ThemeService::GetColor(int id, - return color; - } - -+#if defined(BRAVE_CHROMIUM_BUILD) -+ MAYBE_OVERRIDE_DEFAULT_COLOR_FOR_BRAVE(id, incognito, profile()) -+#endif -+ - return GetDefaultColor(id, incognito); - } - diff --git a/test/BUILD.gn b/test/BUILD.gn index fd3445199359..b8cc46b71045 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -170,7 +170,7 @@ test("brave_browser_tests") { "//brave/browser/extensions/brave_extension_provider_browsertest.cc", "//brave/renderer/brave_content_settings_observer_browsertest.cc", "//brave/renderer/brave_content_settings_observer_flash_browsertest.cc", - "//brave/browser/themes/brave_theme_type_browsertest.cc", + "//brave/browser/themes/brave_theme_service_browsertest.cc", "//chrome/browser/extensions/browsertest_util.cc", "//chrome/browser/extensions/browsertest_util.h", "//chrome/browser/extensions/extension_browsertest.cc", From 48f555890fda09e8a719859a5812d44081c9a926 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Thu, 23 Aug 2018 20:42:33 +0900 Subject: [PATCH 5/7] Remove BraveThemeTest.ObtainsBraveOverrideColors This unittest can't be used because override is done in ThemeService. BraveThemeServiceTest can cover this test. Also static check is added to prevent other class should not use newly defined DISALLOW_COPY_AND_ASSIGN. --- browser/themes/BUILD.gn | 5 ++++- browser/themes/theme_properties.cc | 3 --- browser/themes/theme_properties_unittest.cc | 13 ------------- .../chrome/browser/themes/theme_service_win.h | 16 +++++++++++++--- common/BUILD.gn | 1 + common/brave_override_util.h | 13 +++++++++++++ test/BUILD.gn | 1 - 7 files changed, 31 insertions(+), 21 deletions(-) delete mode 100644 browser/themes/theme_properties_unittest.cc create mode 100644 common/brave_override_util.h diff --git a/browser/themes/BUILD.gn b/browser/themes/BUILD.gn index bd972c93de6b..8194ec3b7715 100644 --- a/browser/themes/BUILD.gn +++ b/browser/themes/BUILD.gn @@ -10,8 +10,11 @@ source_set("themes") { deps = [ "//base", + "//brave/common", + "//chrome/common", + "//components/prefs", + "//components/pref_registry", "//components/version_info", - "//net", "//skia", ] } diff --git a/browser/themes/theme_properties.cc b/browser/themes/theme_properties.cc index 83983aeeb644..fb0d965d1008 100644 --- a/browser/themes/theme_properties.cc +++ b/browser/themes/theme_properties.cc @@ -72,9 +72,6 @@ base::Optional MaybeGetDefaultColorForBraveUiDevChannel(int id, bool in // Returns a |nullopt| if the UI color is not handled by Brave. base::Optional MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile) { - if (id == BRAVE_COLOR_FOR_TEST) { - return SkColorSetRGB(11, 13, 17); - } #if !defined(OFFICIAL_BUILD) return MaybeGetDefaultColorForBraveUiDevChannel(id, incognito); #else diff --git a/browser/themes/theme_properties_unittest.cc b/browser/themes/theme_properties_unittest.cc deleted file mode 100644 index 914f4c942593..000000000000 --- a/browser/themes/theme_properties_unittest.cc +++ /dev/null @@ -1,13 +0,0 @@ -/* 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/. */ - -#include "brave/browser/themes/theme_properties.h" -#include "chrome/browser/themes/theme_properties.h" -#include "testing/gtest/include/gtest/gtest.h" - -TEST(BraveThemeTest, ObtainsBraveOverrideColors) { - SkColor actualColor = ThemeProperties::GetDefaultColor(BRAVE_COLOR_FOR_TEST, false); - SkColor expectedColor = SkColorSetRGB(11, 13, 17); - ASSERT_EQ(actualColor, expectedColor); -} diff --git a/chromium_src/chrome/browser/themes/theme_service_win.h b/chromium_src/chrome/browser/themes/theme_service_win.h index 394009171b15..2ad45e79cde4 100644 --- a/chromium_src/chrome/browser/themes/theme_service_win.h +++ b/chromium_src/chrome/browser/themes/theme_service_win.h @@ -2,12 +2,21 @@ * 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/. */ +// Include all headers in chrome/browser/themes/theme_service_win.h to +// prevent applying newly defined DISALLOW_COPY_AND_ASSIGN macro. +// If not, many static_assert are catched. +#include "base/optional.h" +#include "base/win/registry.h" +#include "chrome/browser/themes/theme_service.h" + #include "brave/browser/themes/brave_theme_service.h" +#include "brave/common/brave_override_util.h" #undef DISALLOW_COPY_AND_ASSIGN -#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ - DISALLOW_COPY(TypeName); \ - DISALLOW_ASSIGN(TypeName); \ +#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ + DISALLOW_COPY(TypeName); \ + DISALLOW_ASSIGN(TypeName); \ + static_assert(strings_equal(#TypeName, "ThemeServiceWin"), "Use only for ThemeServiceWin"); \ friend class BraveThemeServiceWin #undef ThemeService @@ -21,3 +30,4 @@ DISALLOW_ASSIGN(TypeName) #undef ThemeService +#define ThemeService ThemeService diff --git a/common/BUILD.gn b/common/BUILD.gn index e5238e6155cf..8a1248ad155d 100644 --- a/common/BUILD.gn +++ b/common/BUILD.gn @@ -15,6 +15,7 @@ source_set("channel_info") { source_set("common") { sources = [ + "brave_override_util.h", "brave_paths.cc", "brave_paths.h", "brave_switches.cc", diff --git a/common/brave_override_util.h b/common/brave_override_util.h new file mode 100644 index 000000000000..4bc494cca831 --- /dev/null +++ b/common/brave_override_util.h @@ -0,0 +1,13 @@ +/* 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/. */ + +#ifndef BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ +#define BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ + +// Copied from https://stackoverflow.com/a/27490954 +constexpr bool strings_equal(char const * a, char const * b) { + return *a == *b && (*a == '\0' || strings_equal(a + 1, b + 1)); +} + +#endif // BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ \ No newline at end of file diff --git a/test/BUILD.gn b/test/BUILD.gn index b8cc46b71045..e414b166e4de 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -32,7 +32,6 @@ test("brave_unit_tests") { "//brave/browser/brave_stats_updater_unittest.cc", "//brave/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc", "//brave/browser/net/brave_static_redirect_network_delegate_helper_unittest.cc", - "//brave/browser/themes/theme_properties_unittest.cc", "//brave/chromium_src/chrome/browser/signin/account_consistency_disabled_unittest.cc", "//brave/chromium_src/components/version_info/brave_version_info_unittest.cc", "//brave/common/importer/brave_mock_importer_bridge.cc", From 450eacfabfbab4127b77dfb417bb6cb15251d12d Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 24 Aug 2018 11:50:12 +0900 Subject: [PATCH 6/7] Reverted to patching friend class I tried to conditionally add friend statement in DISALLOW_COPY_AND_ASSIGN. However, I can't find how to do it in macro. --- .../chrome/browser/themes/theme_service_win.h | 20 ------------------- common/BUILD.gn | 1 - common/brave_override_util.h | 13 ------------ ...e-browser-themes-theme_service_win.h.patch | 14 +++++++++++++ 4 files changed, 14 insertions(+), 34 deletions(-) delete mode 100644 common/brave_override_util.h create mode 100644 patches/chrome-browser-themes-theme_service_win.h.patch diff --git a/chromium_src/chrome/browser/themes/theme_service_win.h b/chromium_src/chrome/browser/themes/theme_service_win.h index 2ad45e79cde4..0e9cd4b50109 100644 --- a/chromium_src/chrome/browser/themes/theme_service_win.h +++ b/chromium_src/chrome/browser/themes/theme_service_win.h @@ -2,32 +2,12 @@ * 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/. */ -// Include all headers in chrome/browser/themes/theme_service_win.h to -// prevent applying newly defined DISALLOW_COPY_AND_ASSIGN macro. -// If not, many static_assert are catched. -#include "base/optional.h" -#include "base/win/registry.h" -#include "chrome/browser/themes/theme_service.h" - #include "brave/browser/themes/brave_theme_service.h" -#include "brave/common/brave_override_util.h" - -#undef DISALLOW_COPY_AND_ASSIGN -#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ - DISALLOW_COPY(TypeName); \ - DISALLOW_ASSIGN(TypeName); \ - static_assert(strings_equal(#TypeName, "ThemeServiceWin"), "Use only for ThemeServiceWin"); \ - friend class BraveThemeServiceWin #undef ThemeService #define ThemeService BraveThemeService #include "../../../../../chrome/browser/themes/theme_service_win.h" -#undef DISALLOW_COPY_AND_ASSIGN -#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ - DISALLOW_COPY(TypeName); \ - DISALLOW_ASSIGN(TypeName) - #undef ThemeService #define ThemeService ThemeService diff --git a/common/BUILD.gn b/common/BUILD.gn index 8a1248ad155d..e5238e6155cf 100644 --- a/common/BUILD.gn +++ b/common/BUILD.gn @@ -15,7 +15,6 @@ source_set("channel_info") { source_set("common") { sources = [ - "brave_override_util.h", "brave_paths.cc", "brave_paths.h", "brave_switches.cc", diff --git a/common/brave_override_util.h b/common/brave_override_util.h deleted file mode 100644 index 4bc494cca831..000000000000 --- a/common/brave_override_util.h +++ /dev/null @@ -1,13 +0,0 @@ -/* 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/. */ - -#ifndef BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ -#define BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ - -// Copied from https://stackoverflow.com/a/27490954 -constexpr bool strings_equal(char const * a, char const * b) { - return *a == *b && (*a == '\0' || strings_equal(a + 1, b + 1)); -} - -#endif // BRAVE_COMMON_BRAVE_OVERRIDE_UTIL_H_ \ No newline at end of file diff --git a/patches/chrome-browser-themes-theme_service_win.h.patch b/patches/chrome-browser-themes-theme_service_win.h.patch new file mode 100644 index 000000000000..41b8110dc4c0 --- /dev/null +++ b/patches/chrome-browser-themes-theme_service_win.h.patch @@ -0,0 +1,14 @@ +diff --git a/chrome/browser/themes/theme_service_win.h b/chrome/browser/themes/theme_service_win.h +index eba1cb2596d71b91e06183209503f3fc83b0e593..966a408a3080c6941db428b554ace6c66f6aead6 100644 +--- a/chrome/browser/themes/theme_service_win.h ++++ b/chrome/browser/themes/theme_service_win.h +@@ -18,6 +18,9 @@ class ThemeServiceWin : public ThemeService { + ~ThemeServiceWin() override; + + private: ++#if defined(BRAVE_CHROMIUM_BUILD) ++ friend class BraveThemeServiceWin; ++#endif + // ThemeService: + bool ShouldUseNativeFrame() const override; + SkColor GetDefaultColor(int id, bool incognito) const override; From e53cff59468a67bb48ae289a2934074a6effcdc4 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 24 Aug 2018 12:49:34 +0900 Subject: [PATCH 7/7] Make BraveThemeService observe kBraveThemeType prefs --- browser/themes/brave_theme_service.cc | 28 +++++++++++++------ browser/themes/brave_theme_service.h | 15 +++++++--- .../themes/brave_theme_service_browsertest.cc | 12 ++++++-- browser/themes/theme_properties.cc | 2 ++ ...e-browser-themes-theme_service_win.h.patch | 6 ++-- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/browser/themes/brave_theme_service.cc b/browser/themes/brave_theme_service.cc index 8f50ffe4d791..74c173957a3a 100644 --- a/browser/themes/brave_theme_service.cc +++ b/browser/themes/brave_theme_service.cc @@ -6,8 +6,8 @@ #include "brave/browser/themes/theme_properties.h" #include "brave/common/pref_names.h" -#include "chrome/browser/themes/theme_service_factory.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/themes/theme_service_factory.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_service.h" @@ -18,17 +18,22 @@ void BraveThemeService::RegisterProfilePrefs( } // static -int BraveThemeService::GetBraveThemeType(Profile* profile) { - return profile->GetPrefs()->GetInteger(kBraveThemeType); +BraveThemeService::BraveThemeType BraveThemeService::GetBraveThemeType(Profile* profile) { + return static_cast( + profile->GetPrefs()->GetInteger(kBraveThemeType)); } -// static -void BraveThemeService::SetBraveThemeType(Profile* profile, - BraveThemeType type) { - profile->GetPrefs()->SetInteger(kBraveThemeType, type); +BraveThemeService::BraveThemeService() {} - static_cast(ThemeServiceFactory::GetForProfile(profile)) - ->NotifyThemeChanged(); +BraveThemeService::~BraveThemeService() {} + +void BraveThemeService::Init(Profile* profile) { + brave_theme_type_pref_.Init( + kBraveThemeType, + profile->GetPrefs(), + base::Bind(&BraveThemeService::OnPreferenceChanged, + base::Unretained(this))); + ThemeService::Init(profile); } SkColor BraveThemeService::GetDefaultColor(int id, bool incognito) const { @@ -39,3 +44,8 @@ SkColor BraveThemeService::GetDefaultColor(int id, bool incognito) const { return ThemeService::GetDefaultColor(id, incognito); } + +void BraveThemeService::OnPreferenceChanged(const std::string& pref_name) { + DCHECK(pref_name == kBraveThemeType); + NotifyThemeChanged(); +} diff --git a/browser/themes/brave_theme_service.h b/browser/themes/brave_theme_service.h index 4683a9ca7a76..8edd55d14e19 100644 --- a/browser/themes/brave_theme_service.h +++ b/browser/themes/brave_theme_service.h @@ -6,6 +6,7 @@ #define BRAVE_BROWSER_THEMES_BRAVE_THEME_SERVICE_H_ #include "chrome/browser/themes/theme_service.h" +#include "components/prefs/pref_member.h" namespace user_prefs { class PrefRegistrySyncable; @@ -20,17 +21,23 @@ class BraveThemeService : public ThemeService { }; static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); - static int GetBraveThemeType(Profile* profile); - static void SetBraveThemeType(Profile* profile, BraveThemeType type); + static BraveThemeType GetBraveThemeType(Profile* profile); - BraveThemeService() = default; - ~BraveThemeService() override = default; + BraveThemeService(); + ~BraveThemeService() override; + // ThemeService overrides: + void Init(Profile* profile) override; + protected: // ThemeService overrides: SkColor GetDefaultColor(int id, bool incognito) const override; private: + void OnPreferenceChanged(const std::string& pref_name); + + IntegerPrefMember brave_theme_type_pref_; + DISALLOW_COPY_AND_ASSIGN(BraveThemeService); }; diff --git a/browser/themes/brave_theme_service_browsertest.cc b/browser/themes/brave_theme_service_browsertest.cc index 51861f44348d..a45df5042ab3 100644 --- a/browser/themes/brave_theme_service_browsertest.cc +++ b/browser/themes/brave_theme_service_browsertest.cc @@ -3,15 +3,23 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "brave/browser/themes/brave_theme_service.h" +#include "brave/common/pref_names.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_service.h" #include "chrome/browser/ui/browser.h" +#include "components/prefs/pref_service.h" using BraveThemeServiceTest = InProcessBrowserTest; using BTS = BraveThemeService; +namespace { +void SetBraveThemeType(Profile* profile, BTS::BraveThemeType type) { + profile->GetPrefs()->SetInteger(kBraveThemeType, type); +} +} // namespace + IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, BraveThemeChangeTest) { Profile* profile = browser()->profile(); #if defined(OFFICIAL_BUILD) @@ -23,7 +31,7 @@ IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, BraveThemeChangeTest) { EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DEFAULT, BTS::GetBraveThemeType(profile)); const ui::ThemeProvider& tp = ThemeService::GetThemeProviderForProfile(profile); - BTS::SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_LIGHT); + SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_LIGHT); EXPECT_EQ(BTS::BRAVE_THEME_TYPE_LIGHT, BTS::GetBraveThemeType(profile)); #if defined(OFFICIAL_BUILD) EXPECT_EQ(light_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); @@ -32,7 +40,7 @@ IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, BraveThemeChangeTest) { EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); #endif - BTS::SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_DARK); + SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_DARK); EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DARK, BTS::GetBraveThemeType(profile)); EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME)); } diff --git a/browser/themes/theme_properties.cc b/browser/themes/theme_properties.cc index fb0d965d1008..6b7b94bc87b1 100644 --- a/browser/themes/theme_properties.cc +++ b/browser/themes/theme_properties.cc @@ -5,9 +5,11 @@ #include "brave/browser/themes/theme_properties.h" #include "brave/browser/themes/brave_theme_service.h" +#include "brave/common/pref_names.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/common/channel_info.h" +#include "components/prefs/pref_service.h" #include "components/version_info/channel.h" namespace { diff --git a/patches/chrome-browser-themes-theme_service_win.h.patch b/patches/chrome-browser-themes-theme_service_win.h.patch index 41b8110dc4c0..70bb9e90baec 100644 --- a/patches/chrome-browser-themes-theme_service_win.h.patch +++ b/patches/chrome-browser-themes-theme_service_win.h.patch @@ -1,14 +1,12 @@ diff --git a/chrome/browser/themes/theme_service_win.h b/chrome/browser/themes/theme_service_win.h -index eba1cb2596d71b91e06183209503f3fc83b0e593..966a408a3080c6941db428b554ace6c66f6aead6 100644 +index eba1cb2596d71b91e06183209503f3fc83b0e593..e6261a982859a6a66caa00d362f290d247ce7922 100644 --- a/chrome/browser/themes/theme_service_win.h +++ b/chrome/browser/themes/theme_service_win.h -@@ -18,6 +18,9 @@ class ThemeServiceWin : public ThemeService { +@@ -18,6 +18,7 @@ class ThemeServiceWin : public ThemeService { ~ThemeServiceWin() override; private: -+#if defined(BRAVE_CHROMIUM_BUILD) + friend class BraveThemeServiceWin; -+#endif // ThemeService: bool ShouldUseNativeFrame() const override; SkColor GetDefaultColor(int id, bool incognito) const override;