Don't show invert bubble on Mac in increased contrast mode

http://crrev.com/c/2118693 caused the invert bubble to start showing up
on Mac in increased contrast mode when a new incognito window was
opened. http://crrev.com/c/2247407 addressed this temporarily by
disabling the bubble for incognito windows.

This change is a follow-up to http://crrev.com/c/2247407 as a more
permanent solution to the original problem. The invert bubble should
show up in the High Contrast dark theme on Windows. However,
GetHighContrastColorScheme() was calling into UsesHighContrastColors()
to detect this.

The problem with this is that on Mac, UsesHighContrastColors() has a
different meaning than on Windows. To fix this, make
GetHighContrastColorScheme() a virtual function and only return the
High Contrast theme on Windows.

Bug: 1095855
Change-Id: I24a7ed2dd5b9194a795d1560ba1e5f4be85afd98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250224Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#782052}

chrome/browser/ui/views/accessibility/invert_bubble_view.cc

@@ -157,14 +157,12 @@ void InvertBubbleView::OpenLink(const std::string& url,
 
 void MaybeShowInvertBubbleView(BrowserView* browser_view) {
   Browser* browser = browser_view->browser();
-  if (browser->profile()->IsIncognitoProfile())
-    return;
   PrefService* pref_service = browser->profile()->GetPrefs();
   views::View* anchor =
       browser_view->toolbar_button_provider()->GetAppMenuButton();
   if (anchor && anchor->GetWidget() &&
-      anchor->GetNativeTheme()->GetHighContrastColorScheme() ==
-          ui::NativeTheme::HighContrastColorScheme::kDark &&
+      anchor->GetNativeTheme()->GetPlatformHighContrastColorScheme() ==
+          ui::NativeTheme::PlatformHighContrastColorScheme::kDark &&
       !pref_service->GetBoolean(prefs::kInvertNotificationShown)) {
     pref_service->SetBoolean(prefs::kInvertNotificationShown, true);
     ShowInvertBubbleView(browser, anchor);

chrome/browser/ui/webui/ntp/ntp_resource_cache.cc

@@ -98,8 +98,8 @@ SkColor GetThemeColor(const ui::ThemeProvider& tp, int id) {
   // If web contents are being inverted because the system is in high-contrast
   // mode, any system theme colors we use must be inverted too to cancel out.
   return ui::NativeTheme::GetInstanceForNativeUi()
-                     ->GetHighContrastColorScheme() ==
-                 ui::NativeTheme::HighContrastColorScheme::kDark
+                     ->GetPlatformHighContrastColorScheme() ==
+                 ui::NativeTheme::PlatformHighContrastColorScheme::kDark
              ? color_utils::InvertColor(color)
              : color;
 }

content/browser/accessibility/browser_accessibility_state_impl.cc

@@ -177,8 +177,9 @@ void BrowserAccessibilityStateImpl::UpdateHistogramsOnUIThread() {
 #if defined(OS_WIN)
   UMA_HISTOGRAM_ENUMERATION(
       "Accessibility.WinHighContrastTheme",
-      ui::NativeTheme::GetInstanceForNativeUi()->GetHighContrastColorScheme(),
-      ui::NativeTheme::HighContrastColorScheme::kMaxValue);
+      ui::NativeTheme::GetInstanceForNativeUi()
+          ->GetPlatformHighContrastColorScheme(),
+      ui::NativeTheme::PlatformHighContrastColorScheme::kMaxValue);
 #endif
 }
 

ui/native_theme/native_theme.cc

@@ -254,18 +254,13 @@ bool NativeTheme::UsesHighContrastColors() const {
   return is_high_contrast_;
 }
 
-NativeTheme::HighContrastColorScheme NativeTheme::GetHighContrastColorScheme()
-    const {
-  if (!UsesHighContrastColors())
-    return NativeTheme::HighContrastColorScheme::kNone;
-  switch (GetPreferredColorScheme()) {
-    case NativeTheme::PreferredColorScheme::kDark:
-      return HighContrastColorScheme::kDark;
-    case NativeTheme::PreferredColorScheme::kLight:
-      return HighContrastColorScheme::kLight;
-  }
-  NOTREACHED();
-  return NativeTheme::HighContrastColorScheme::kNone;
+NativeTheme::PlatformHighContrastColorScheme
+NativeTheme::GetPlatformHighContrastColorScheme() const {
+  if (GetDefaultSystemColorScheme() != ColorScheme::kPlatformHighContrast)
+    return PlatformHighContrastColorScheme::kNone;
+  return (GetPreferredColorScheme() == PreferredColorScheme::kDark)
+             ? PlatformHighContrastColorScheme::kDark
+             : PlatformHighContrastColorScheme::kLight;
 }
 
 NativeTheme::PreferredColorScheme NativeTheme::GetPreferredColorScheme() const {

ui/native_theme/native_theme.h

@@ -117,9 +117,9 @@ class NATIVE_THEME_EXPORT NativeTheme {
   // This enum is reporting in metrics. Do not reorder; add additional values at
   // the end.
   //
-  // This represents the OS-level high contrast theme. kNone if high contrast is
-  // not enabled.
-  enum class HighContrastColorScheme {
+  // This represents the OS-level high contrast theme. kNone unless the default
+  // system color scheme is kPlatformHighContrast.
+  enum class PlatformHighContrastColorScheme {
     kNone = 0,
     kDark = 1,
     kLight = 2,
@@ -420,9 +420,10 @@ class NATIVE_THEME_EXPORT NativeTheme {
   // system accessibility settings and the system theme.
   virtual bool UsesHighContrastColors() const;
 
-  // Returns the HighContrastColorScheme used by the OS. Returns kNone if high
-  // contrast is not enabled.
-  HighContrastColorScheme GetHighContrastColorScheme() const;
+  // Returns the PlatformHighContrastColorScheme used by the OS. Returns a value
+  // other than kNone only if the default system color scheme is
+  // kPlatformHighContrast.
+  PlatformHighContrastColorScheme GetPlatformHighContrastColorScheme() const;
 
   // Returns true when the NativeTheme uses a light-on-dark color scheme. If
   // you're considering using this function to choose between two hard-coded

ui/native_theme/native_theme_mac_unittest.cc

@@ -24,4 +24,31 @@ TEST(NativeThemeMacTest, SystemColorsExist) {
   }
 }
 
+TEST(NativeThemeMacTest, GetPlatformHighContrastColorScheme) {
+  using PrefScheme = NativeTheme::PreferredColorScheme;
+
+  constexpr NativeTheme::PlatformHighContrastColorScheme kNone =
+      NativeTheme::PlatformHighContrastColorScheme::kNone;
+
+  NativeTheme* native_theme = NativeTheme::GetInstanceForNativeUi();
+  ASSERT_TRUE(native_theme);
+
+  native_theme->set_high_contrast(false);
+  native_theme->set_preferred_color_scheme(PrefScheme::kDark);
+  EXPECT_EQ(native_theme->GetPlatformHighContrastColorScheme(), kNone);
+
+  native_theme->set_preferred_color_scheme(PrefScheme::kLight);
+  EXPECT_EQ(native_theme->GetPlatformHighContrastColorScheme(), kNone);
+
+  native_theme->set_high_contrast(true);
+  native_theme->set_preferred_color_scheme(PrefScheme::kDark);
+  EXPECT_EQ(native_theme->GetPlatformHighContrastColorScheme(), kNone);
+
+  native_theme->set_preferred_color_scheme(PrefScheme::kLight);
+  EXPECT_EQ(native_theme->GetPlatformHighContrastColorScheme(), kNone);
+
+  native_theme->set_high_contrast(false);
+  EXPECT_EQ(native_theme->GetPlatformHighContrastColorScheme(), kNone);
+}
+
 }  // namespace ui

ui/native_theme/native_theme_win_unittest.cc

@@ -9,10 +9,8 @@
 
 namespace ui {
 
-using ColorScheme = NativeTheme::ColorScheme;
 using PrefScheme = NativeTheme::PreferredColorScheme;
 using SystemThemeColor = NativeTheme::SystemThemeColor;
-using ColorId = NativeTheme::ColorId;
 
 class TestNativeThemeWin : public NativeThemeWin {
  public:
@@ -20,109 +18,122 @@ class TestNativeThemeWin : public NativeThemeWin {
   ~TestNativeThemeWin() override = default;
 
   // NativeTheme:
-  bool UsesHighContrastColors() const override { return high_contrast_; }
-  bool ShouldUseDarkColors() const override { return dark_mode_; }
-
-  void SetDarkMode(bool dark_mode) { dark_mode_ = dark_mode; }
-  void SetUsesHighContrastColors(bool high_contrast) {
-    high_contrast_ = high_contrast;
-  }
   void SetSystemColor(SystemThemeColor system_color, SkColor color) {
     system_colors_[system_color] = color;
   }
 
- private:
-  bool dark_mode_ = false;
-  bool high_contrast_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(TestNativeThemeWin);
 };
 
 TEST(NativeThemeWinTest, CalculatePreferredColorScheme) {
   TestNativeThemeWin theme;
 
-  theme.SetUsesHighContrastColors(false);
-  theme.SetDarkMode(true);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
+  theme.set_high_contrast(false);
+  theme.set_use_dark_colors(true);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
 
-  theme.SetDarkMode(false);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
+  theme.set_use_dark_colors(false);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
 
-  theme.SetUsesHighContrastColors(true);
+  theme.set_high_contrast(true);
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorBLACK);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
 
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorWHITE);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
 
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorBLUE);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kDark);
 
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorYELLOW);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
 
-  theme.SetUsesHighContrastColors(false);
-  ASSERT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
+  theme.set_high_contrast(false);
+  EXPECT_EQ(theme.CalculatePreferredColorScheme(), PrefScheme::kLight);
 }
 
 TEST(NativeThemeWinTest, GetDefaultSystemColorScheme) {
-  TestNativeThemeWin theme;
+  using ColorScheme = NativeTheme::ColorScheme;
 
-  theme.SetUsesHighContrastColors(false);
-  theme.SetDarkMode(true);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kDark);
+  TestNativeThemeWin theme;
+  theme.set_high_contrast(false);
+  theme.set_use_dark_colors(true);
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kDark);
 
-  theme.SetDarkMode(false);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kLight);
+  theme.set_use_dark_colors(false);
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kLight);
 
-  theme.SetUsesHighContrastColors(true);
+  theme.set_high_contrast(true);
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorBLACK);
   theme.SetSystemColor(SystemThemeColor::kWindowText, SK_ColorWHITE);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(),
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(),
             ColorScheme::kPlatformHighContrast);
 
   theme.SetSystemColor(SystemThemeColor::kWindow, SK_ColorWHITE);
   theme.SetSystemColor(SystemThemeColor::kWindowText, SK_ColorBLACK);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(),
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(),
             ColorScheme::kPlatformHighContrast);
 
   theme.SetSystemColor(SystemThemeColor::kWindowText, SK_ColorBLUE);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(),
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(),
             ColorScheme::kPlatformHighContrast);
 
-  theme.SetUsesHighContrastColors(false);
-  ASSERT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kLight);
+  theme.set_high_contrast(false);
+  EXPECT_EQ(theme.GetDefaultSystemColorScheme(), ColorScheme::kLight);
 }
 
 TEST(NativeThemeWinTest, GetPlatformHighContrastColor) {
-  TestNativeThemeWin theme;
+  using ColorId = NativeTheme::ColorId;
 
   // These specific colors don't matter, but should be unique.
   constexpr SkColor kWindowTextColor = SK_ColorGREEN;
   constexpr SkColor kHighlightColor = SK_ColorYELLOW;
   constexpr SkColor kHighlightTextColor = SK_ColorBLUE;
 
+  TestNativeThemeWin theme;
   theme.SetSystemColor(SystemThemeColor::kWindowText, kWindowTextColor);
   theme.SetSystemColor(SystemThemeColor::kHighlight, kHighlightColor);
   theme.SetSystemColor(SystemThemeColor::kHighlightText, kHighlightTextColor);
 
   // Test that we get regular colors when HC is off.
-  theme.SetUsesHighContrastColors(false);
-  ASSERT_NE(theme.GetSystemColor(ColorId::kColorId_LabelEnabledColor),
+  theme.set_high_contrast(false);
+  EXPECT_NE(theme.GetSystemColor(ColorId::kColorId_LabelEnabledColor),
             kWindowTextColor);
-  ASSERT_NE(theme.GetSystemColor(ColorId::kColorId_ProminentButtonColor),
+  EXPECT_NE(theme.GetSystemColor(ColorId::kColorId_ProminentButtonColor),
             kHighlightColor);
-  ASSERT_NE(theme.GetSystemColor(ColorId::kColorId_TextOnProminentButtonColor),
+  EXPECT_NE(theme.GetSystemColor(ColorId::kColorId_TextOnProminentButtonColor),
             kHighlightTextColor);
 
   // Test that we get HC colors when HC is on.
-  theme.SetUsesHighContrastColors(true);
-  ASSERT_EQ(theme.GetSystemColor(ColorId::kColorId_LabelEnabledColor),
+  theme.set_high_contrast(true);
+  EXPECT_EQ(theme.GetSystemColor(ColorId::kColorId_LabelEnabledColor),
             kWindowTextColor);
-  ASSERT_EQ(theme.GetSystemColor(ColorId::kColorId_ProminentButtonColor),
+  EXPECT_EQ(theme.GetSystemColor(ColorId::kColorId_ProminentButtonColor),
             kHighlightColor);
-  ASSERT_EQ(theme.GetSystemColor(ColorId::kColorId_TextOnProminentButtonColor),
+  EXPECT_EQ(theme.GetSystemColor(ColorId::kColorId_TextOnProminentButtonColor),
             kHighlightTextColor);
 }
 
+TEST(NativeThemeWinTest, GetPlatformHighContrastColorScheme) {
+  using HCColorScheme = NativeTheme::PlatformHighContrastColorScheme;
+
+  TestNativeThemeWin theme;
+  theme.set_high_contrast(false);
+  theme.set_preferred_color_scheme(PrefScheme::kDark);
+  EXPECT_EQ(theme.GetPlatformHighContrastColorScheme(), HCColorScheme::kNone);
+
+  theme.set_preferred_color_scheme(PrefScheme::kLight);
+  EXPECT_EQ(theme.GetPlatformHighContrastColorScheme(), HCColorScheme::kNone);
+
+  theme.set_high_contrast(true);
+  theme.set_preferred_color_scheme(PrefScheme::kDark);
+  EXPECT_EQ(theme.GetPlatformHighContrastColorScheme(), HCColorScheme::kDark);
+
+  theme.set_preferred_color_scheme(PrefScheme::kLight);
+  EXPECT_EQ(theme.GetPlatformHighContrastColorScheme(), HCColorScheme::kLight);
+
+  theme.set_high_contrast(false);
+  EXPECT_EQ(theme.GetPlatformHighContrastColorScheme(), HCColorScheme::kNone);
+}
+
 }  // namespace ui

ui/views/controls/button/label_button_unittest.cc

@@ -627,8 +627,8 @@ TEST_F(LabelButtonTest, HighlightedButtonStyle) {
 // Ensure the label resets the enabled color after LabelButton::OnThemeChanged()
 // is invoked.
 TEST_F(LabelButtonTest, OnThemeChanged) {
-  ASSERT_NE(button_->GetNativeTheme()->GetHighContrastColorScheme(),
-            ui::NativeTheme::HighContrastColorScheme::kDark);
+  ASSERT_NE(button_->GetNativeTheme()->GetPlatformHighContrastColorScheme(),
+            ui::NativeTheme::PlatformHighContrastColorScheme::kDark);
   ASSERT_NE(button_->label()->GetBackgroundColor(), SK_ColorBLACK);
   EXPECT_EQ(themed_normal_text_color_, button_->label()->GetEnabledColor());