Make browser_preferred_color_scheme light when custom theme is on.

Issue:
When chrome has a browser theme on that changes the browser's color and
the user preferences are set to dark mode, UsedColorSchemeRootScrollbars
makes root scrollbars be dark in pages where no color scheme is
specified. This becomes an issue when the browser's theme is a light
color, which contrasts too starkly with the dark scrollbars. To repro
this issue, please see the attached bug.

The `browser_preferred_color_scheme` flag is used to send the preferred
color scheme from the browser process to blink in the renderer process
to help with the selection of a color scheme for scrollbars.

Fix:
- This CL changes the `browser_preferred_color_scheme` browser setting
to be set to the default value (kLight color scheme) when a custom theme
is detected in the browser. With this change,
UsedColorSchemeRootScrollbars doesn't override the root scrollbar by
changing it to dark mode.
- The `browser_preferred_color_scheme` setting is also being renamed to
better represent what it pertains to, and is now called
`preferred_root_scrollbar_color_scheme`. All the relevant references and
functions have been renamed to reflect this.

Bug: 337904215
Change-Id: I2517747f2a4cef061187daf4191c0fc8660d7fcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5502831
Reviewed-by: Yaroslav Shalivskyy <yshalivskyy@microsoft.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Gaston Rodriguez <gastonr@microsoft.com>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1298816}

chrome/browser/chrome_content_browser_client.cc

@@ -164,6 +164,8 @@
 #include "chrome/browser/supervised_user/supervised_user_google_auth_navigation_throttle.h"
 #include "chrome/browser/supervised_user/supervised_user_navigation_throttle.h"
 #include "chrome/browser/task_manager/sampling/task_manager_impl.h"
+#include "chrome/browser/themes/theme_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/tracing/chrome_tracing_delegate.h"
 #include "chrome/browser/translate/translate_service.h"
 #include "chrome/browser/ui/blocked_content/blocked_window_params.h"
@@ -3701,7 +3703,7 @@ bool UpdatePreferredColorScheme(WebPreferences* web_prefs,
         delegate->IsNightModeEnabled()
             ? blink::mojom::PreferredColorScheme::kDark
             : blink::mojom::PreferredColorScheme::kLight;
-    web_prefs->browser_preferred_color_scheme =
+    web_prefs->preferred_root_scrollbar_color_scheme =
         web_prefs->preferred_color_scheme;
   }
 #else
@@ -3709,10 +3711,22 @@ bool UpdatePreferredColorScheme(WebPreferences* web_prefs,
   web_prefs->preferred_color_scheme =
       ToBlinkPreferredColorScheme(native_theme->GetPreferredColorScheme());
 
+  bool using_different_colored_frame = false;
+  if (Profile* profile =
+          Profile::FromBrowserContext(web_contents->GetBrowserContext())) {
+    if (ThemeService* theme_service =
+            ThemeServiceFactory::GetForProfile(profile)) {
+      using_different_colored_frame = !theme_service->UsingDefaultTheme() ||
+                                      theme_service->GetUserColor().has_value();
+    }
+  }
+
   // Update based on the ColorProvider associated with `web_contents`. Depends
-  // on the browser color mode settings.
-  web_prefs->browser_preferred_color_scheme =
-      web_contents->GetColorMode() == ui::ColorProviderKey::ColorMode::kLight
+  // on the browser color mode settings and whether the user profile has set a
+  // custom coloring for the browser ui.
+  web_prefs->preferred_root_scrollbar_color_scheme =
+      web_contents->GetColorMode() == ui::ColorProviderKey::ColorMode::kLight ||
+              using_different_colored_frame
           ? blink::mojom::PreferredColorScheme::kLight
           : blink::mojom::PreferredColorScheme::kDark;
 #endif  // BUILDFLAG(IS_ANDROID)

chrome/browser/chrome_content_browser_client_browsertest.cc

@@ -22,6 +22,8 @@
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
+#include "chrome/browser/themes/theme_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/search/instant_test_base.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -65,6 +67,7 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "third_party/blink/public/mojom/webpreferences/web_preferences.mojom.h"
 #include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
 #include "ui/color/color_provider.h"
 #include "ui/color/color_provider_key.h"
@@ -691,6 +694,80 @@ INSTANTIATE_TEST_SUITE_P(All,
                          PrefersColorSchemeTest,
                          testing::Combine(testing::Bool(), testing::Bool()));
 
+class PreferredRootScrollbarColorSchemeChromeClientTest
+    : public testing::WithParamInterface<std::tuple<bool, bool>>,
+      public InProcessBrowserTest {
+ protected:
+  PreferredRootScrollbarColorSchemeChromeClientTest()
+      : dark_mode_(std::get<0>(GetParam())),
+        uses_custom_theme_(std::get<1>(GetParam())),
+        theme_client_(&test_theme_) {
+    test_theme_.SetDarkMode(dark_mode_);
+  }
+
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+    original_client_ = SetBrowserClientForTesting(&theme_client_);
+    test_theme_.SetDarkMode(dark_mode_);
+    ui::NativeTheme::GetInstanceForNativeUi()->set_use_dark_colors(dark_mode_);
+    ThemeService* theme_service =
+        ThemeServiceFactory::GetForProfile(browser()->profile());
+    if (uses_custom_theme_) {
+      theme_service->BuildAutogeneratedThemeFromColor(SK_ColorRED);
+    } else {
+      theme_service->UseDefaultTheme();
+    }
+  }
+
+  ~PreferredRootScrollbarColorSchemeChromeClientTest() override {
+    CHECK_EQ(&theme_client_, SetBrowserClientForTesting(original_client_));
+  }
+
+  blink::mojom::PreferredColorScheme ExpectedColorScheme() const {
+    return dark_mode_ && !uses_custom_theme_
+               ? blink::mojom::PreferredColorScheme::kDark
+               : blink::mojom::PreferredColorScheme::kLight;
+  }
+
+ private:
+  class ChromeContentBrowserClientWithWebTheme
+      : public ChromeContentBrowserClient {
+   public:
+    explicit ChromeContentBrowserClientWithWebTheme(
+        const ui::NativeTheme* theme)
+        : theme_(theme) {}
+
+   protected:
+    const ui::NativeTheme* GetWebTheme() const override { return theme_; }
+
+   private:
+    const raw_ptr<const ui::NativeTheme> theme_;
+  };
+
+  const bool dark_mode_ = false;
+  const bool uses_custom_theme_ = false;
+  raw_ptr<content::ContentBrowserClient> original_client_ = nullptr;
+  ui::TestNativeTheme test_theme_;
+  ChromeContentBrowserClientWithWebTheme theme_client_;
+};
+
+// This test verifies that the preferred color scheme for root scrollbars is set
+// appropriately following the web content's color scheme and the presence of
+// a custom theme.
+IN_PROC_BROWSER_TEST_P(PreferredRootScrollbarColorSchemeChromeClientTest,
+                       ScrollbarFollowsPreferredColorScheme) {
+  EXPECT_EQ(browser()
+                ->tab_strip_model()
+                ->GetActiveWebContents()
+                ->GetOrCreateWebPreferences()
+                .preferred_root_scrollbar_color_scheme,
+            ExpectedColorScheme());
+}
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         PreferredRootScrollbarColorSchemeChromeClientTest,
+                         testing::Combine(testing::Bool(), testing::Bool()));
+
 class PrefersContrastTest
     : public testing::WithParamInterface<ui::NativeTheme::PreferredContrast>,
       public InProcessBrowserTest {

third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc

@@ -219,7 +219,8 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
   out->require_transient_activation_for_html_fullscreen =
       data.require_transient_activation_for_html_fullscreen();
   out->in_forced_colors = data.in_forced_colors();
-  out->browser_preferred_color_scheme = data.browser_preferred_color_scheme();
+  out->preferred_root_scrollbar_color_scheme =
+      data.preferred_root_scrollbar_color_scheme();
   out->preferred_color_scheme = data.preferred_color_scheme();
   out->preferred_contrast = data.preferred_contrast();
   out->picture_in_picture_enabled = data.picture_in_picture_enabled();

third_party/blink/public/common/web_preferences/web_preferences.h

@@ -355,10 +355,12 @@ struct BLINK_COMMON_EXPORT WebPreferences {
   // when to apply system color overrides to author specified styles.
   bool in_forced_colors = false;
 
-  // The preferred color scheme set by the user's browser settings. The scheme
-  // is used to evaluate the used color scheme. Currently, only used for the
-  // scrollbars' used color scheme.
-  blink::mojom::PreferredColorScheme browser_preferred_color_scheme =
+  // The preferred color scheme set by the user's browser settings. The variable
+  // follows the browser's color mode setting unless a browser theme (custom or
+  // not) is defined, in which case the color scheme is set to the default
+  // value. This value is used to evaluate the used color scheme in non overlay
+  // root scrollbars.
+  blink::mojom::PreferredColorScheme preferred_root_scrollbar_color_scheme =
       blink::mojom::PreferredColorScheme::kLight;
 
   // The preferred color scheme for the web content. The scheme is used to

third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h

@@ -713,9 +713,10 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::WebPreferencesDataView,
     return r.in_forced_colors;
   }
 
-  static blink::mojom::PreferredColorScheme browser_preferred_color_scheme(
+  static blink::mojom::PreferredColorScheme
+  preferred_root_scrollbar_color_scheme(
       const blink::web_pref::WebPreferences& r) {
-    return r.browser_preferred_color_scheme;
+    return r.preferred_root_scrollbar_color_scheme;
   }
 
   static blink::mojom::PreferredColorScheme preferred_color_scheme(

third_party/blink/public/mojom/webpreferences/web_preferences.mojom

@@ -419,10 +419,12 @@ struct WebPreferences {
   // when to apply system color overrides to author specified styles.
   bool in_forced_colors;
 
-  // The preferred color scheme set by the user's browser settings. The scheme
-  // is used to evaluate the used color scheme. Currently, only used for the
-  // scrollbars' used color scheme.
-  PreferredColorScheme browser_preferred_color_scheme;
+  // The preferred color scheme set by the user's browser settings. The variable
+  // follows the browser's color mode setting unless a browser theme (custom or
+  // not) is defined, in which case the color scheme is set to the default
+  // value. This value is used to evaluate the used color scheme in non overlay
+  // root scrollbars.
+  PreferredColorScheme preferred_root_scrollbar_color_scheme;
 
   // The preferred color scheme for the web content. The scheme is used to
   // evaluate the prefers-color-scheme media query and resolve UA color scheme

third_party/blink/public/web/web_settings.h

@@ -269,7 +269,7 @@ class WebSettings {
   virtual void SetLazyLoadingImageMarginPx4G(int) = 0;
   virtual void SetForceDarkModeEnabled(bool) = 0;
   virtual void SetInForcedColors(bool) = 0;
-  virtual void SetBrowserPreferredColorScheme(
+  virtual void SetPreferredRootScrollbarColorScheme(
       blink::mojom::PreferredColorScheme) = 0;
   virtual void SetPreferredColorScheme(blink::mojom::PreferredColorScheme) = 0;
   virtual void SetPreferredContrast(mojom::PreferredContrast) = 0;

third_party/blink/renderer/core/exported/web_settings_impl.cc

@@ -757,9 +757,9 @@ void WebSettingsImpl::SetInForcedColors(bool in_forced_colors) {
   settings_->SetInForcedColors(in_forced_colors);
 }
 
-void WebSettingsImpl::SetBrowserPreferredColorScheme(
+void WebSettingsImpl::SetPreferredRootScrollbarColorScheme(
     mojom::blink::PreferredColorScheme color_scheme) {
-  settings_->SetBrowserPreferredColorScheme(color_scheme);
+  settings_->SetPreferredRootScrollbarColorScheme(color_scheme);
 }
 
 void WebSettingsImpl::SetPreferredColorScheme(

third_party/blink/renderer/core/exported/web_settings_impl.h

@@ -222,7 +222,7 @@ class CORE_EXPORT WebSettingsImpl final : public WebSettings {
 
   void SetForceDarkModeEnabled(bool) override;
   void SetInForcedColors(bool) override;
-  void SetBrowserPreferredColorScheme(
+  void SetPreferredRootScrollbarColorScheme(
       mojom::blink::PreferredColorScheme) override;
   void SetPreferredColorScheme(mojom::blink::PreferredColorScheme) override;
   void SetPreferredContrast(mojom::blink::PreferredContrast) override;

third_party/blink/renderer/core/exported/web_view_impl.cc

@@ -1792,8 +1792,8 @@ void WebView::ApplyWebPreferences(const web_pref::WebPreferences& prefs,
 
   settings->SetLazyLoadEnabled(prefs.lazy_load_enabled);
   settings->SetInForcedColors(prefs.in_forced_colors);
-  settings->SetBrowserPreferredColorScheme(
-      prefs.browser_preferred_color_scheme);
+  settings->SetPreferredRootScrollbarColorScheme(
+      prefs.preferred_root_scrollbar_color_scheme);
   settings->SetPreferredColorScheme(prefs.preferred_color_scheme);
   settings->SetPreferredContrast(prefs.preferred_contrast);
 

third_party/blink/renderer/core/frame/settings.json5

@@ -1092,11 +1092,13 @@
       type: "bool",
     },
 
-    // Preferred color scheme from the browser settings passed to the renderer
-    // for evaluating the used color scheme. Currently, only used for the
-    // scrollbars' used color scheme.
+    // The preferred color scheme set by the user's browser settings. The
+    // variable follows the browser's color mode setting unless a browser theme
+    // (custom or not) is defined, in which case the color scheme is set to the
+    // default value. This value is used to evaluate the used color scheme in
+    // non overlay root scrollbars.
     {
-      name: "browserPreferredColorScheme",
+      name: "preferredRootScrollbarColorScheme",
       initial: "mojom::blink::PreferredColorScheme::kLight",
       invalidate: ["ColorScheme"],
       type: "mojom::blink::PreferredColorScheme",

third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc

@@ -1262,12 +1262,16 @@ mojom::blink::ColorScheme PaintLayerScrollableArea::UsedColorSchemeScrollbars()
   //     specified),
   //   - the preferred color scheme is dark (OS-based),
   //   - the browser preferred color scheme is dark.
+  //   - there is no custom browser theme active
+  //   - there is no color-picked browser theme active
+  //     (both theme conditions are embedded into
+  //        `GetPreferredRootScrollbarColorScheme()`)
   if (IsGlobalRootNonOverlayScroller() &&
       layout_box->StyleRef().ColorSchemeFlagsIsNormal()) {
     const auto& document = layout_box->GetDocument();
     if (document.GetPreferredColorScheme() ==
             mojom::blink::PreferredColorScheme::kDark &&
-        document.GetSettings()->GetBrowserPreferredColorScheme() ==
+        document.GetSettings()->GetPreferredRootScrollbarColorScheme() ==
             mojom::blink::PreferredColorScheme::kDark) {
       UseCounter::Count(GetLayoutBox()->GetDocument(),
                         WebFeature::kUsedColorSchemeRootScrollbarsDark);

third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc

@@ -81,7 +81,7 @@ class PaintLayerScrollableAreaTest : public PaintControllerPaintTest {
   // Default browser preferred color scheme is light. The method sets both
   // browser-based and the OS-based preferred color schemes to dark.
   void SetPreferredColorSchemesToDark(ColorSchemeHelper& color_scheme_helper) {
-    color_scheme_helper.SetBrowserPreferredColorScheme(
+    color_scheme_helper.SetPreferredRootScrollbarColorScheme(
         mojom::blink::PreferredColorScheme::kDark);
     color_scheme_helper.SetPreferredColorScheme(
         mojom::blink::PreferredColorScheme::kDark);
@@ -90,8 +90,9 @@ class PaintLayerScrollableAreaTest : public PaintControllerPaintTest {
   void AssertDefaultPreferredColorSchemes() const {
     ASSERT_EQ(GetDocument().GetPreferredColorScheme(),
               mojom::blink::PreferredColorScheme::kLight);
-    ASSERT_EQ(GetDocument().GetSettings()->GetBrowserPreferredColorScheme(),
-              mojom::blink::PreferredColorScheme::kLight);
+    ASSERT_EQ(
+        GetDocument().GetSettings()->GetPreferredRootScrollbarColorScheme(),
+        mojom::blink::PreferredColorScheme::kLight);
   }
 
   void ExpectEqAllScrollControlsNeedPaintInvalidation(
@@ -1640,7 +1641,7 @@ TEST_P(PaintLayerScrollableAreaTest, UsedColorSchemeRootScrollbarsDark) {
             mojom::blink::ColorScheme::kLight);
 
   // Change browser preferred color scheme to dark.
-  color_scheme_helper.SetBrowserPreferredColorScheme(
+  color_scheme_helper.SetPreferredRootScrollbarColorScheme(
       mojom::blink::PreferredColorScheme::kDark);
   UpdateAllLifecyclePhasesForTest();
 

third_party/blink/renderer/core/testing/color_scheme_helper.cc

@@ -12,8 +12,8 @@ namespace blink {
 
 ColorSchemeHelper::ColorSchemeHelper(Document& document)
     : settings_(*document.GetSettings()) {
-  default_browser_preferred_color_scheme_ =
-      settings_.GetBrowserPreferredColorScheme();
+  default_preferred_root_scrollbar_color_scheme_ =
+      settings_.GetPreferredRootScrollbarColorScheme();
   default_preferred_color_scheme_ = settings_.GetPreferredColorScheme();
   default_preferred_contrast_ = settings_.GetPreferredContrast();
   default_in_forced_colors_ = settings_.GetInForcedColors();
@@ -21,8 +21,8 @@ ColorSchemeHelper::ColorSchemeHelper(Document& document)
 
 ColorSchemeHelper::ColorSchemeHelper(Page& page)
     : settings_(page.GetSettings()) {
-  default_browser_preferred_color_scheme_ =
-      settings_.GetBrowserPreferredColorScheme();
+  default_preferred_root_scrollbar_color_scheme_ =
+      settings_.GetPreferredRootScrollbarColorScheme();
   default_preferred_color_scheme_ = settings_.GetPreferredColorScheme();
   default_preferred_contrast_ = settings_.GetPreferredContrast();
   default_in_forced_colors_ = settings_.GetInForcedColors();
@@ -31,16 +31,17 @@ ColorSchemeHelper::ColorSchemeHelper(Page& page)
 ColorSchemeHelper::~ColorSchemeHelper() {
   // Reset preferred color scheme, preferred contrast and forced colors to their
   // original values.
-  settings_.SetBrowserPreferredColorScheme(
-      default_browser_preferred_color_scheme_);
+  settings_.SetPreferredRootScrollbarColorScheme(
+      default_preferred_root_scrollbar_color_scheme_);
   settings_.SetPreferredColorScheme(default_preferred_color_scheme_);
   settings_.SetPreferredContrast(default_preferred_contrast_);
   settings_.SetInForcedColors(default_in_forced_colors_);
 }
 
-void ColorSchemeHelper::SetBrowserPreferredColorScheme(
-    blink::mojom::PreferredColorScheme browser_preferred_color_scheme) {
-  settings_.SetBrowserPreferredColorScheme(browser_preferred_color_scheme);
+void ColorSchemeHelper::SetPreferredRootScrollbarColorScheme(
+    blink::mojom::PreferredColorScheme preferred_root_scrollbar_color_scheme) {
+  settings_.SetPreferredRootScrollbarColorScheme(
+      preferred_root_scrollbar_color_scheme);
 }
 
 void ColorSchemeHelper::SetPreferredColorScheme(

third_party/blink/renderer/core/testing/color_scheme_helper.h

@@ -17,7 +17,7 @@ class Settings;
 // ColorSchemeHelper is used to update the following values and eventually reset
 // the values back to their default upon deconstruction for testing.
 // Values include:
-//   - BrowserPreferredColorScheme,
+//   - PreferredRootScrollbarColorScheme,
 //   - PreferredColorScheme,
 //   - PreferredContrast,
 //   - ForcedColors.
@@ -27,8 +27,8 @@ class ColorSchemeHelper {
   ColorSchemeHelper(Page& page);
   ~ColorSchemeHelper();
 
-  void SetBrowserPreferredColorScheme(
-      mojom::PreferredColorScheme browser_preferred_color_scheme);
+  void SetPreferredRootScrollbarColorScheme(
+      mojom::PreferredColorScheme preferred_root_scrollbar_color_scheme);
   void SetPreferredColorScheme(
       mojom::PreferredColorScheme preferred_color_scheme);
   void SetPreferredContrast(mojom::PreferredContrast preferred_contrast);
@@ -37,7 +37,7 @@ class ColorSchemeHelper {
 
  private:
   Settings& settings_;
-  mojom::PreferredColorScheme default_browser_preferred_color_scheme_ =
+  mojom::PreferredColorScheme default_preferred_root_scrollbar_color_scheme_ =
       mojom::PreferredColorScheme::kLight;
   mojom::PreferredColorScheme default_preferred_color_scheme_ =
       mojom::PreferredColorScheme::kLight;