Skip to content

Commit

Permalink
Fixes theme changing.
Browse files Browse the repository at this point in the history
This commit is a squash of 4 commits from #3281.

- Fix setting preferred color scheme logic for brave theme change
- Fix system dark mode change affect brave theme on Win10
    When brave theme is set to dark or light, system dark mode change
    should not affect brave theme.
- CleanUp: Delete SystemThemeSupportDarkMode
    Upstream added ui::NativeTheme::SystemDarkModeSupported().
    Replace ours with upstream one.
- Add dark mode change test by modifying reg value for Windows
    This test can verify whether theme change notification is delivered
    or not.
    If brave theme option is not default, system dark theme change should
    not trigger theme change notificagtion delivery.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/67fd4d9f35650a079f6a636b5d19de77856bd2cf

commit 67fd4d9f35650a079f6a636b5d19de77856bd2cf
Author: Alison Maher <almaher@microsoft.com>
Date:   Fri Jul 12 18:34:21 2019 +0000

    Set preferred color scheme for high contrast

    This change adds logic to update the preferred color scheme based on
    the OS high contrast theme. If high contrast is not enabled, preferred
    color scheme will continue to be based on the OS dark mode state.

    A PreferredColorScheme enum was added to NativeTheme to keep track
    of the current high contrast/dark mode preferred color scheme.

    The updated preferred color scheme is used to evaluate the
    prefers-color-scheme media query.

    Bug: 970285
  • Loading branch information
simonhong authored and mkarolin committed Sep 4, 2019
1 parent 508e825 commit 7bc4c2a
Show file tree
Hide file tree
Showing 16 changed files with 261 additions and 81 deletions.
3 changes: 2 additions & 1 deletion browser/themes/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ source_set("themes") {
"brave_theme_service_win.h",
"brave_theme_utils.cc",
"brave_theme_utils.h",
"brave_theme_utils_internal.cc",
"brave_theme_utils_internal.h",
"brave_theme_utils_mac.mm",
"brave_theme_utils_win.cc",
"theme_properties.cc",
"theme_properties.h",
]
Expand Down
13 changes: 12 additions & 1 deletion browser/themes/brave_theme_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ void BraveThemeService::Init(Profile* profile) {
if (brave::IsTorProfile(profile) || brave::IsGuestProfile(profile))
return;

#if defined(OS_WIN)
ui::IgnoreSystemDarkModeChange(
profile->GetPrefs()->GetInteger(kBraveThemeType) !=
BraveThemeType::BRAVE_THEME_TYPE_DEFAULT);
#endif
// Start with proper system theme to make brave theme and
// base ui components theme use same theme.
SetSystemTheme(static_cast<BraveThemeType>(
Expand Down Expand Up @@ -224,6 +229,12 @@ void BraveThemeService::OnPreferenceChanged(const std::string& pref_name) {
// Changing theme type means default theme is not overridden anymore.
profile()->GetPrefs()->SetBoolean(kUseOverriddenBraveThemeType, false);

#if defined(OS_WIN)
ui::IgnoreSystemDarkModeChange(
profile()->GetPrefs()->GetInteger(kBraveThemeType) !=
BraveThemeType::BRAVE_THEME_TYPE_DEFAULT);
#endif

SetSystemTheme(static_cast<BraveThemeType>(
profile()->GetPrefs()->GetInteger(kBraveThemeType)));
}
Expand Down Expand Up @@ -266,5 +277,5 @@ bool BraveThemeService::SystemThemeModeEnabled() {
switches::kForceDarkMode))
return true;

return SystemThemeSupportDarkMode();
return ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeSupported();
}
78 changes: 74 additions & 4 deletions browser/themes/brave_theme_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
#include "ui/native_theme/native_theme_dark_aura.h"
#include "ui/native_theme/native_theme_observer.h"

#if defined(OS_WIN)
#include "base/run_loop.h"
#include "base/time/time.h"
#include "base/win/registry.h"
#endif

using BraveThemeServiceTest = InProcessBrowserTest;
using BTS = BraveThemeService;

Expand Down Expand Up @@ -94,9 +100,9 @@ IN_PROC_BROWSER_TEST_F(BraveThemeServiceTestWithoutSystemTheme,
tp_private.GetColor(test_theme_property));
}

// Test whether appropriate native theme observer is called when brave theme is
// changed.
IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, NativeThemeObserverTest) {
// Test whether appropriate native/web theme observer is called when brave theme
// is changed.
IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, ThemeObserverTest) {
Profile* profile = browser()->profile();
// Initially set to light.
SetBraveThemeType(profile, BraveThemeType::BRAVE_THEME_TYPE_LIGHT);
Expand All @@ -107,10 +113,17 @@ IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, NativeThemeObserverTest) {
EXPECT_CALL(
native_theme_observer,
OnNativeThemeUpdated(ui::NativeTheme::GetInstanceForNativeUi())).Times(2);

ui::NativeTheme::GetInstanceForNativeUi()->AddObserver(
&native_theme_observer);

TestNativeThemeObserver web_theme_observer;
EXPECT_CALL(
web_theme_observer,
OnNativeThemeUpdated(ui::NativeTheme::GetInstanceForWeb())).Times(2);

ui::NativeTheme::GetInstanceForWeb()->AddObserver(
&web_theme_observer);

SetBraveThemeType(profile, BraveThemeType::BRAVE_THEME_TYPE_DARK);
SetBraveThemeType(profile, BraveThemeType::BRAVE_THEME_TYPE_LIGHT);
}
Expand Down Expand Up @@ -140,3 +153,60 @@ IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, SystemThemeChangeTest) {
ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeEnabled());
}
}

#if defined(OS_WIN)
// Test native theme notification is called properly by changing reg value.
// This simulates dark mode setting from Windows settings.
// And Toggle it twice from initial value to go back to initial value because
// reg value changes system value. Otherwise, dark mode config could be changed
// after running this test.
IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, DarkModeChangeByRegTest) {
if (!ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeSupported())
return;

base::win::RegKey hkcu_themes_regkey;
bool key_open_succeeded = hkcu_themes_regkey.Open(
HKEY_CURRENT_USER,
L"Software\\Microsoft\\Windows\\CurrentVersion\\"
L"Themes\\Personalize",
KEY_WRITE) == ERROR_SUCCESS;
DCHECK(key_open_succeeded);

DWORD apps_use_light_theme = 1;
hkcu_themes_regkey.ReadValueDW(L"AppsUseLightTheme",
&apps_use_light_theme);
const bool initial_dark_mode = apps_use_light_theme == 0;

// Toggle dark mode and check get notification for default type (same as...).
auto* profile = browser()->profile();
SetBraveThemeType(profile, BraveThemeType::BRAVE_THEME_TYPE_DEFAULT);

apps_use_light_theme = !initial_dark_mode ? 0 : 1;
hkcu_themes_regkey.WriteValue(L"AppsUseLightTheme", apps_use_light_theme);

TestNativeThemeObserver native_theme_observer_for_default;
EXPECT_CALL(
native_theme_observer_for_default,
OnNativeThemeUpdated(ui::NativeTheme::GetInstanceForNativeUi())).Times(1);
ui::NativeTheme::GetInstanceForNativeUi()->AddObserver(
&native_theme_observer_for_default);

// Toggle dark mode and |native_theme_observer_for_light| will not get
// notification for light type.
SetBraveThemeType(profile, BraveThemeType::BRAVE_THEME_TYPE_LIGHT);

TestNativeThemeObserver native_theme_observer_for_light;
EXPECT_CALL(
native_theme_observer_for_light,
OnNativeThemeUpdated(ui::NativeTheme::GetInstanceForNativeUi())).Times(0);
ui::NativeTheme::GetInstanceForNativeUi()->AddObserver(
&native_theme_observer_for_light);

apps_use_light_theme = initial_dark_mode ? 0 : 1;
hkcu_themes_regkey.WriteValue(L"AppsUseLightTheme", apps_use_light_theme);

// Timeout is used because we can't get notifiication with light theme.
base::RunLoop run_loop;
run_loop.RunWithTimeout(base::TimeDelta::FromMilliseconds(500));;
}
#endif
34 changes: 18 additions & 16 deletions browser/themes/brave_theme_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,27 @@

#include "brave/browser/themes/brave_theme_utils.h"

#include "brave/browser/themes/brave_theme_utils_internal.h"
#include "ui/native_theme/native_theme.h"

#if defined(OS_WIN)
#include "ui/native_theme/native_theme_win.h"
#endif

namespace ui {
void SetDarkMode(bool dark_mode) {
ui::NativeTheme::GetInstanceForNativeUi()->set_dark_mode(dark_mode);
} // namespace ui
// static
void BraveThemeUtils::SetDarkMode(bool dark_mode) {
NativeTheme::GetInstanceForNativeUi()->set_dark_mode(dark_mode);
NativeTheme::GetInstanceForWeb()->set_dark_mode(dark_mode);
}

// static
void BraveThemeUtils::ReCalcAndSetPreferredColorScheme() {
auto scheme =
NativeTheme::GetInstanceForNativeUi()->CalculatePreferredColorScheme();
NativeTheme::GetInstanceForNativeUi()->set_preferred_color_scheme(scheme);
NativeTheme::GetInstanceForWeb()->set_preferred_color_scheme(scheme);
}

#if defined(OS_WIN)
// This resets dark mode to os theme when user changes brave theme from
Expand All @@ -33,7 +44,9 @@ void SetSystemTheme(BraveThemeType type) {
// Follow os theme type for default type.
if (type == BraveThemeType::BRAVE_THEME_TYPE_DEFAULT) {
#if defined(OS_WIN)
DCHECK(SystemThemeSupportDarkMode());
DCHECK(
ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeSupported());
// This sets preferred color scheme on its own.
ui::UpdateDarkModeStatus();
return;
#else
Expand All @@ -42,17 +55,6 @@ void SetSystemTheme(BraveThemeType type) {
NOTREACHED();
#endif
}

ui::SetDarkMode(type == BraveThemeType::BRAVE_THEME_TYPE_DARK);
// Have to notify to observer explicitly because |ui::SetDarkMode()| just
// set ui::NativeTheme:dark_mode_ value.
ui::NativeTheme::GetInstanceForNativeUi()->NotifyObservers();
}
#endif

#if defined(OS_LINUX)
bool SystemThemeSupportDarkMode() {
// Linux doesn't support dark mode yet.
return false;
internal::SetSystemThemeForNonDarkModePlatform(type);
}
#endif
19 changes: 11 additions & 8 deletions browser/themes/brave_theme_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

#include "brave/browser/themes/brave_theme_service.h"

bool SystemThemeSupportDarkMode();

// When system supports system per-application system theme changing, set it.
// Currently, only MacOS support it.
// Otherewise, we need to overrides from native theme level and explicitly
Expand All @@ -18,13 +16,18 @@ bool SystemThemeSupportDarkMode();
void SetSystemTheme(BraveThemeType type);

// Inserted in the ui namespace to add into ui::NativeTheme/NativeThemeWin as a
// friend function. This function calls protected/private method of
// ui::NativeTheme::set_dark_mode(). It's a protected method that called by
// platform specific subclasses whenever system os theme is changed.
// But we want to change it for using brave theme also for webui/base ui
// modules like context menu.
// friend class. These methods call protected methods of ui::NativeTheme. They
// are protected methods that called by platform specific subclasses whenever
// system os theme is changed. But we want to change it for using brave theme
// also for webui/base ui modules like context menu.
namespace ui {
void SetDarkMode(bool dark_mode);
class BraveThemeUtils {
public:
static void SetDarkMode(bool dark_mode);
// Recalculate preferred color scheme based on current dark mode that set by
// SetDarkMode() and set it to NativeTheme.
static void ReCalcAndSetPreferredColorScheme();
};
} // namespace ui

#endif // BRAVE_BROWSER_THEMES_BRAVE_THEME_UTILS_H_
28 changes: 28 additions & 0 deletions browser/themes/brave_theme_utils_internal.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* 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/. */

#include "brave/browser/themes/brave_theme_utils.h"

#include "ui/native_theme/native_theme.h"

namespace internal {

void SetSystemThemeForNonDarkModePlatform(BraveThemeType type) {
// Call SetDarkMode() first then call ReCalcPreferredColorScheme() because
// ReCalcPreferredColorScheme() calculates preferred color scheme based on dark
// mode.
ui::BraveThemeUtils::SetDarkMode(type ==
BraveThemeType::BRAVE_THEME_TYPE_DARK);
ui::BraveThemeUtils::ReCalcAndSetPreferredColorScheme();
// Have to notify observers explicitly because
// |ui::BraveThemeUtils::SetDarkMode()| and
// |ui::BraveThemeUtils::ReCalcPreferredColorScheme| just set
// ui::NativeTheme:dark_mode_ and ui::NativeTheme:preferred_color_scheme_
// values.
ui::NativeTheme::GetInstanceForNativeUi()->NotifyObservers();
ui::NativeTheme::GetInstanceForWeb()->NotifyObservers();
}

} // namespace internal
18 changes: 18 additions & 0 deletions browser/themes/brave_theme_utils_internal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* 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/. */

#ifndef BRAVE_BROWSER_THEMES_BRAVE_THEME_UTILS_INTERNAL_H_
#define BRAVE_BROWSER_THEMES_BRAVE_THEME_UTILS_INTERNAL_H_

#include "brave/browser/themes/brave_theme_service.h"

namespace internal {

// Reset dark mode and preferred color scheme and notify.
void SetSystemThemeForNonDarkModePlatform(BraveThemeType type);

} // namespace internal

#endif // BRAVE_BROWSER_THEMES_BRAVE_THEME_UTILS_INTERNAL_H_
14 changes: 4 additions & 10 deletions browser/themes/brave_theme_utils_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,13 @@
#import <Cocoa/Cocoa.h>

#include "base/mac/sdk_forward_declarations.h"
#include "brave/browser/themes/brave_theme_utils_internal.h"
#include "ui/native_theme/native_theme.h"

bool SystemThemeSupportDarkMode() {
// Dark mode is supported since Mojave.
if (@available(macOS 10.14, *))
return true;
return false;
}

void SetSystemTheme(BraveThemeType type) {
if (type == BRAVE_THEME_TYPE_DEFAULT) {
DCHECK(SystemThemeSupportDarkMode());
DCHECK(
ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeSupported());
[NSApp setAppearance:nil];
return;
}
Expand All @@ -30,7 +25,6 @@ void SetSystemTheme(BraveThemeType type) {
: NSAppearanceNameAqua;
[NSApp setAppearance:[NSAppearance appearanceNamed:new_appearance_name]];
} else {
ui::SetDarkMode(type == BraveThemeType::BRAVE_THEME_TYPE_DARK);
ui::NativeTheme::GetInstanceForNativeUi()->NotifyObservers();
internal::SetSystemThemeForNonDarkModePlatform(type);
}
}
25 changes: 0 additions & 25 deletions browser/themes/brave_theme_utils_win.cc

This file was deleted.

15 changes: 8 additions & 7 deletions chromium_src/chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@

#if defined(OS_LINUX)
#include "brave/browser/ui/views/brave_browser_main_extra_parts_views_linux.h"
#define ChromeBrowserMainExtraPartsViewsLinux BraveBrowserMainExtraPartsViewsLinux // NOLINT
#define ChromeBrowserMainExtraPartsViewsLinux \
BraveBrowserMainExtraPartsViewsLinux // NOLINT
#endif

#define HandleNewTabURLRewrite HandleNewTabURLRewrite_ChromiumImpl
#define HandleNewTabURLReverseRewrite HandleNewTabURLReverseRewrite_ChromiumImpl

namespace search {
bool HandleNewTabURLRewrite(GURL * url, content::BrowserContext * bc) {
return false;
}
bool HandleNewTabURLReverseRewrite(GURL * url, content::BrowserContext * bc) {
return false;
}
bool HandleNewTabURLRewrite(GURL* url, content::BrowserContext* bc) {
return false;
}
bool HandleNewTabURLReverseRewrite(GURL* url, content::BrowserContext* bc) {
return false;
}
} // namespace search

#include "../../../../chrome/browser/chrome_content_browser_client.cc" // NOLINT
Expand Down
17 changes: 17 additions & 0 deletions chromium_src/ui/native_theme/native_theme.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2019 The Brave Authors
// 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_CHROMIUM_SRC_UI_NATIVE_THEME_NATIVE_THEME_H_
#define BRAVE_CHROMIUM_SRC_UI_NATIVE_THEME_NATIVE_THEME_H_

namespace ui {
class BraveThemeUtils;
} // namespace ui

#define BRAVE_UI_NATIVE_THEME_NATIVE_THEME_H_ friend class ui::BraveThemeUtils;
#include "../../../../ui/native_theme/native_theme.h"
#undef BRAVE_UI_NATIVE_THEME_NATIVE_THEME_H_

#endif // BRAVE_CHROMIUM_SRC_UI_NATIVE_THEME_NATIVE_THEME_H_
Loading

0 comments on commit 7bc4c2a

Please sign in to comment.