Commit 7397db48 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

themes: track dark mode changes in high contrast

Currently, high contrast colors come from IncreasedContrastThemeSupplier,
which needs to be recreated when the dark mode state changes. This change
has IncreasedContrastThemeSupplier observe the backing NativeTheme for
dark mode changes so that recreating it isn't necessary here.

Bug: 1046494
Change-Id: Ib5955064b43577e9836c3f4864e77601bdb3fbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067468
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744697}
parent bf333c3b
...@@ -4,11 +4,19 @@ ...@@ -4,11 +4,19 @@
#include "chrome/browser/themes/increased_contrast_theme_supplier.h" #include "chrome/browser/themes/increased_contrast_theme_supplier.h"
#include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_properties.h"
#include "ui/native_theme/native_theme.h"
IncreasedContrastThemeSupplier::IncreasedContrastThemeSupplier( IncreasedContrastThemeSupplier::IncreasedContrastThemeSupplier(
bool is_dark_mode) ui::NativeTheme* native_theme)
: CustomThemeSupplier(INCREASED_CONTRAST), is_dark_mode_(is_dark_mode) {} : CustomThemeSupplier(INCREASED_CONTRAST),
IncreasedContrastThemeSupplier::~IncreasedContrastThemeSupplier() {} native_theme_(native_theme),
is_dark_mode_(native_theme->ShouldUseDarkColors()) {
native_theme->AddObserver(this);
}
IncreasedContrastThemeSupplier::~IncreasedContrastThemeSupplier() {
native_theme_->RemoveObserver(this);
}
// TODO(ellyjones): Follow up with a11y designers about these color choices. // TODO(ellyjones): Follow up with a11y designers about these color choices.
bool IncreasedContrastThemeSupplier::GetColor(int id, SkColor* color) const { bool IncreasedContrastThemeSupplier::GetColor(int id, SkColor* color) const {
...@@ -53,3 +61,8 @@ bool IncreasedContrastThemeSupplier::GetColor(int id, SkColor* color) const { ...@@ -53,3 +61,8 @@ bool IncreasedContrastThemeSupplier::GetColor(int id, SkColor* color) const {
bool IncreasedContrastThemeSupplier::CanUseIncognitoColors() const { bool IncreasedContrastThemeSupplier::CanUseIncognitoColors() const {
return false; return false;
} }
void IncreasedContrastThemeSupplier::OnNativeThemeUpdated(
ui::NativeTheme* native_theme) {
is_dark_mode_ = native_theme->ShouldUseDarkColors();
}
...@@ -6,13 +6,19 @@ ...@@ -6,13 +6,19 @@
#define CHROME_BROWSER_THEMES_INCREASED_CONTRAST_THEME_SUPPLIER_H_ #define CHROME_BROWSER_THEMES_INCREASED_CONTRAST_THEME_SUPPLIER_H_
#include "chrome/browser/themes/custom_theme_supplier.h" #include "chrome/browser/themes/custom_theme_supplier.h"
#include "ui/native_theme/native_theme_observer.h"
namespace ui {
class NativeTheme;
} // namespace ui
// A theme supplier that maximizes the contrast between UI elements and // A theme supplier that maximizes the contrast between UI elements and
// especially the visual prominence of key UI elements (omnibox, active vs // especially the visual prominence of key UI elements (omnibox, active vs
// inactive tab distinction). // inactive tab distinction).
class IncreasedContrastThemeSupplier : public CustomThemeSupplier { class IncreasedContrastThemeSupplier : public CustomThemeSupplier,
public ui::NativeThemeObserver {
public: public:
explicit IncreasedContrastThemeSupplier(bool is_dark_mode); explicit IncreasedContrastThemeSupplier(ui::NativeTheme* theme);
bool GetColor(int id, SkColor* color) const override; bool GetColor(int id, SkColor* color) const override;
bool CanUseIncognitoColors() const override; bool CanUseIncognitoColors() const override;
...@@ -21,6 +27,9 @@ class IncreasedContrastThemeSupplier : public CustomThemeSupplier { ...@@ -21,6 +27,9 @@ class IncreasedContrastThemeSupplier : public CustomThemeSupplier {
~IncreasedContrastThemeSupplier() override; ~IncreasedContrastThemeSupplier() override;
private: private:
void OnNativeThemeUpdated(ui::NativeTheme* native_theme) override;
ui::NativeTheme* native_theme_;
bool is_dark_mode_; bool is_dark_mode_;
DISALLOW_COPY_AND_ASSIGN(IncreasedContrastThemeSupplier); DISALLOW_COPY_AND_ASSIGN(IncreasedContrastThemeSupplier);
......
...@@ -320,8 +320,8 @@ void ThemeService::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) { ...@@ -320,8 +320,8 @@ void ThemeService::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) {
if (UsingDefaultTheme()) { if (UsingDefaultTheme()) {
scoped_refptr<CustomThemeSupplier> supplier; scoped_refptr<CustomThemeSupplier> supplier;
if (theme_helper_.ShouldUseIncreasedContrastThemeSupplier(observed_theme)) { if (theme_helper_.ShouldUseIncreasedContrastThemeSupplier(observed_theme)) {
supplier = base::MakeRefCounted<IncreasedContrastThemeSupplier>( supplier =
observed_theme->ShouldUseDarkColors()); base::MakeRefCounted<IncreasedContrastThemeSupplier>(observed_theme);
} }
SwapThemeSupplier(supplier); SwapThemeSupplier(supplier);
} }
...@@ -356,8 +356,7 @@ void ThemeService::UseDefaultTheme() { ...@@ -356,8 +356,7 @@ void ThemeService::UseDefaultTheme() {
ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi(); ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi();
if (theme_helper_.ShouldUseIncreasedContrastThemeSupplier(native_theme)) { if (theme_helper_.ShouldUseIncreasedContrastThemeSupplier(native_theme)) {
SetCustomDefaultTheme(new IncreasedContrastThemeSupplier( SetCustomDefaultTheme(new IncreasedContrastThemeSupplier(native_theme));
native_theme->ShouldUseDarkColors()));
// Early return here because SetCustomDefaultTheme does ClearAllThemeData // Early return here because SetCustomDefaultTheme does ClearAllThemeData
// and NotifyThemeChanged when it needs to. Without this return, the // and NotifyThemeChanged when it needs to. Without this return, the
// IncreasedContrastThemeSupplier would get immediately removed if this // IncreasedContrastThemeSupplier would get immediately removed if this
......
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/color_palette.h" #include "ui/gfx/color_palette.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
#include "ui/native_theme/test_native_theme.h"
#if BUILDFLAG(ENABLE_SUPERVISED_USERS) #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
...@@ -184,6 +185,7 @@ class ThemeServiceTest : public extensions::ExtensionServiceTestBase { ...@@ -184,6 +185,7 @@ class ThemeServiceTest : public extensions::ExtensionServiceTestBase {
} }
protected: protected:
ui::TestNativeTheme native_theme_;
extensions::ExtensionRegistry* registry_ = nullptr; extensions::ExtensionRegistry* registry_ = nullptr;
ThemeService* theme_service_ = nullptr; ThemeService* theme_service_ = nullptr;
}; };
...@@ -460,12 +462,13 @@ TEST_F(ThemeServiceTest, UseDefaultTheme_DisableExtensionTest) { ...@@ -460,12 +462,13 @@ TEST_F(ThemeServiceTest, UseDefaultTheme_DisableExtensionTest) {
TEST_F(ThemeServiceTest, OmniboxContrast) { TEST_F(ThemeServiceTest, OmniboxContrast) {
using TP = ThemeProperties; using TP = ThemeProperties;
for (bool dark : {false, true}) { for (bool dark : {false, true}) {
native_theme_.SetDarkMode(dark);
for (bool high_contrast : {false, true}) { for (bool high_contrast : {false, true}) {
set_theme_supplier( set_theme_supplier(
theme_service_, theme_service_,
high_contrast high_contrast ? base::MakeRefCounted<IncreasedContrastThemeSupplier>(
? base::MakeRefCounted<IncreasedContrastThemeSupplier>(dark) &native_theme_)
: nullptr); : nullptr);
constexpr int contrasting_ids[][2] = { constexpr int contrasting_ids[][2] = {
{TP::COLOR_OMNIBOX_TEXT, TP::COLOR_OMNIBOX_BACKGROUND}, {TP::COLOR_OMNIBOX_TEXT, TP::COLOR_OMNIBOX_BACKGROUND},
{TP::COLOR_OMNIBOX_TEXT, TP::COLOR_OMNIBOX_BACKGROUND_HOVERED}, {TP::COLOR_OMNIBOX_TEXT, TP::COLOR_OMNIBOX_BACKGROUND_HOVERED},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment