Add ColorProviderManager::Key::FrameType

Bug: 1304441 Change-Id: I83012c9348bba9df2063ab0450455470e47fa208 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510376 Reviewed-by: Peter Kasting <[email protected]> Reviewed-by: Thomas Anderson <[email protected]> Commit-Queue: Keren Zhu <[email protected]> Cr-Commit-Position: refs/heads/main@{#979902}
scheib · Mar 10, 2022 · ebe7cbd · ebe7cbd
1 parent c397363
commit ebe7cbd
Show file tree

Hide file tree

Showing 12 changed files with 78 additions and 12 deletions.
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
@@ -73,6 +73,10 @@
 #include "extensions/browser/extension_registry_observer.h"
 #endif
 
+#if BUILDFLAG(IS_LINUX)
+#include "ui/views/linux_ui/linux_ui.h"
+#endif
+
 using TP = ThemeProperties;
 
 // Helpers --------------------------------------------------------------------
@@ -490,12 +494,27 @@ ThemeService::BrowserThemeProvider::GetColorProviderColor(int id) const {
   if (base::FeatureList::IsEnabled(
           features::kColorProviderRedirectionForThemeProvider)) {
     if (auto provider_color_id = ThemeProviderColorIdToColorId(id)) {
-      const auto* const native_theme =
-          incognito_ ? ui::NativeTheme::GetInstanceForDarkUI()
-                     : ui::NativeTheme::GetInstanceForNativeUi();
+      const ui::NativeTheme* native_theme = nullptr;
+
+      if (incognito_) {
+        native_theme = ui::NativeTheme::GetInstanceForDarkUI();
+      } else {
+        native_theme = ui::NativeTheme::GetInstanceForNativeUi();
+#if BUILDFLAG(IS_LINUX)
+        if (const auto* linux_ui = views::LinuxUI::instance()) {
+          // TODO(crbug.com/1304441): Naively passing nullptr might be
+          // problematic. If this is not an issue, remove this parameter from
+          // GetNativeTheme().
+          native_theme = linux_ui->GetNativeTheme(nullptr);
+        }
+#endif
+      }
+
+      auto color_provider_key = native_theme->GetColorProviderKey(
+          GetThemeSupplier(), delegate_->ShouldUseCustomFrame());
       auto* color_provider =
           ui::ColorProviderManager::Get().GetColorProviderFor(
-              native_theme->GetColorProviderKey(GetThemeSupplier()));
+              color_provider_key);
       return color_provider->GetColor(provider_color_id.value());
     }
   }
@@ -602,6 +621,14 @@ CustomThemeSupplier* ThemeService::GetThemeSupplier() const {
   return theme_supplier_.get();
 }
 
+bool ThemeService::ShouldUseCustomFrame() const {
+#if BUILDFLAG(IS_LINUX)
+  return profile_->GetPrefs()->GetBoolean(prefs::kUseCustomChromeFrame);
+#else
+  return true;
+#endif
+}
+
 void ThemeService::SetTheme(const extensions::Extension* extension) {
   DoSetTheme(extension, true);
 }

diff --git a/chrome/browser/themes/theme_service.h b/chrome/browser/themes/theme_service.h
@@ -41,6 +41,7 @@ class ThemeServiceTest;
 class BrowserThemeProviderDelegate {
  public:
   virtual CustomThemeSupplier* GetThemeSupplier() const = 0;
+  virtual bool ShouldUseCustomFrame() const = 0;
 };
 
 class ThemeService : public KeyedService,
@@ -87,6 +88,7 @@ class ThemeService : public KeyedService,
 
   // Overridden from BrowserThemeProviderDelegate:
   CustomThemeSupplier* GetThemeSupplier() const override;
+  bool ShouldUseCustomFrame() const override;
 
   // Set the current theme to the theme defined in |extension|.
   // |extension| must already be added to this profile's

diff --git a/chrome/browser/ui/color/tools/dump_colors.cc b/chrome/browser/ui/color/tools/dump_colors.cc
@@ -51,7 +51,8 @@ int main(int argc, const char* argv[]) {
                              auto contrast_mode) {
     const ui::ColorProviderManager::Key key = {
         color_mode, contrast_mode,
-        ui::ColorProviderManager::SystemTheme::kDefault, nullptr};
+        ui::ColorProviderManager::SystemTheme::kDefault,
+        ui::ColorProviderManager::FrameType::kChromium, nullptr};
     ui::AddColorMixers(provider, key);
     AddChromeColorMixers(provider, key);
   };

diff --git a/chrome/browser/ui/web_applications/app_browser_controller.cc b/chrome/browser/ui/web_applications/app_browser_controller.cc
@@ -8,6 +8,7 @@
 #include "base/feature_list.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
@@ -21,6 +22,7 @@
 #include "chrome/browser/web_applications/system_web_apps/system_web_app_delegate.h"
 #include "chrome/browser/web_applications/system_web_apps/system_web_app_types.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/themes/autogenerated_theme_util.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/security_state/core/security_state.h"
@@ -376,6 +378,15 @@ CustomThemeSupplier* AppBrowserController::GetThemeSupplier() const {
   return theme_pack_.get();
 }
 
+bool AppBrowserController::ShouldUseCustomFrame() const {
+#if BUILDFLAG(IS_LINUX)
+  return browser_->profile()->GetPrefs()->GetBoolean(
+      prefs::kUseCustomChromeFrame);
+#else
+  return true;
+#endif
+}
+
 void AppBrowserController::OnReceivedInitialURL() {
   UpdateCustomTabBarVisibility(/*animate=*/false);
 

diff --git a/chrome/browser/ui/web_applications/app_browser_controller.h b/chrome/browser/ui/web_applications/app_browser_controller.h
@@ -199,6 +199,7 @@ class AppBrowserController : public TabStripModelObserver,
 
   // BrowserThemeProviderDelegate:
   CustomThemeSupplier* GetThemeSupplier() const override;
+  bool ShouldUseCustomFrame() const override;
 
   void UpdateDraggableRegion(const SkRegion& region);
   const absl::optional<SkRegion>& draggable_region() const {

diff --git a/chrome/test/base/test_browser_window.cc b/chrome/test/base/test_browser_window.cc
@@ -123,7 +123,8 @@ const ui::ColorProvider* TestBrowserWindow::GetColorProvider() const {
   return ui::ColorProviderManager::Get().GetColorProviderFor(
       {ui::ColorProviderManager::ColorMode::kLight,
        ui::ColorProviderManager::ContrastMode::kNormal,
-       ui::ColorProviderManager::SystemTheme::kDefault, nullptr});
+       ui::ColorProviderManager::SystemTheme::kDefault,
+       ui::ColorProviderManager::FrameType::kChromium, nullptr});
 }
 
 ui::ElementContext TestBrowserWindow::GetElementContext() {

diff --git a/ui/color/color_provider_manager.cc b/ui/color/color_provider_manager.cc
@@ -46,16 +46,19 @@ ColorProviderManager::Key::Key()
     : Key(ColorMode::kLight,
           ContrastMode::kNormal,
           SystemTheme::kDefault,
+          FrameType::kChromium,
           nullptr) {}
 
 ColorProviderManager::Key::Key(ColorMode color_mode,
                                ContrastMode contrast_mode,
                                SystemTheme system_theme,
+                               FrameType frame_type,
                                scoped_refptr<InitializerSupplier> custom_theme)
     : color_mode(color_mode),
       contrast_mode(contrast_mode),
       elevation_mode(ElevationMode::kLow),
       system_theme(system_theme),
+      frame_type(frame_type),
       custom_theme(std::move(custom_theme)) {}
 
 ColorProviderManager::Key::Key(const Key&) = default;

diff --git a/ui/color/color_provider_manager.h b/ui/color/color_provider_manager.h
@@ -41,9 +41,18 @@ class COMPONENT_EXPORT(COLOR) ColorProviderManager {
     kHigh,
   };
   enum class SystemTheme {
+    // Classic theme, used in the default or users' chosen theme.
     kDefault,
+    // Custom theme that follow the system style,
+    // currently used only when GTK theme is on.
     kCustom,
   };
+  enum class FrameType {
+    // Chrome renders the browser frame.
+    kChromium,
+    // Native system renders the browser frame. Currently GTK only.
+    kNative,
+  };
 
   // Threadsafe not because ColorProviderManager requires it but because a
   // concrete subclass does.
@@ -73,6 +82,7 @@ class COMPONENT_EXPORT(COLOR) ColorProviderManager {
     Key(ColorMode color_mode,
         ContrastMode contrast_mode,
         SystemTheme system_theme,
+        FrameType frame_type,
         scoped_refptr<InitializerSupplier> custom_theme);
     Key(const Key&);
     Key& operator=(const Key&);
@@ -81,6 +91,7 @@ class COMPONENT_EXPORT(COLOR) ColorProviderManager {
     ContrastMode contrast_mode;
     ElevationMode elevation_mode;
     SystemTheme system_theme;
+    FrameType frame_type;
     scoped_refptr<InitializerSupplier> custom_theme;
 
     bool operator<(const Key& other) const {

diff --git a/ui/color/color_provider_manager_unittest.cc b/ui/color/color_provider_manager_unittest.cc
@@ -31,7 +31,8 @@ ColorProvider* GetLightNormalColorProvider() {
   return ColorProviderManager::GetForTesting().GetColorProviderFor(
       {ColorProviderManager::ColorMode::kLight,
        ColorProviderManager::ContrastMode::kNormal,
-       ColorProviderManager::SystemTheme::kDefault, nullptr});
+       ColorProviderManager::SystemTheme::kDefault,
+       ColorProviderManager::FrameType::kChromium, nullptr});
 }
 
 }  // namespace

diff --git a/ui/gtk/gtk_ui.cc b/ui/gtk/gtk_ui.cc
@@ -920,7 +920,11 @@ void GtkUi::UpdateColors() {
            (color_scheme == ui::NativeTheme::ColorScheme::kPlatformHighContrast)
                ? ui::ColorProviderManager::ContrastMode::kHigh
                : ui::ColorProviderManager::ContrastMode::kNormal,
-           ui::ColorProviderManager::SystemTheme::kCustom, nullptr});
+           ui::ColorProviderManager::SystemTheme::kCustom,
+           // Some theme colors, e.g. COLOR_NTP_LINK, are derived from color
+           // provider colors. We assume that those sources' colors won't change
+           // with frame type.
+           ui::ColorProviderManager::FrameType::kChromium, nullptr});
 
   SkColor location_bar_border = GetBorderColor("GtkEntry#entry");
   if (SkColorGetA(location_bar_border))

diff --git a/ui/native_theme/native_theme.cc b/ui/native_theme/native_theme.cc
@@ -38,8 +38,8 @@ bool NativeTheme::SystemDarkModeSupported() {
 #endif
 
 ColorProviderManager::Key NativeTheme::GetColorProviderKey(
-    scoped_refptr<ColorProviderManager::InitializerSupplier> custom_theme)
-    const {
+    scoped_refptr<ColorProviderManager::InitializerSupplier> custom_theme,
+    bool use_custom_frame) const {
   return ColorProviderManager::Key(
       (GetDefaultSystemColorScheme() == ColorScheme::kDark)
           ? ColorProviderManager::ColorMode::kDark
@@ -48,6 +48,8 @@ ColorProviderManager::Key NativeTheme::GetColorProviderKey(
                                   : ColorProviderManager::ContrastMode::kNormal,
       is_custom_system_theme_ ? ColorProviderManager::SystemTheme::kCustom
                               : ColorProviderManager::SystemTheme::kDefault,
+      use_custom_frame ? ui::ColorProviderManager::FrameType::kChromium
+                       : ui::ColorProviderManager::FrameType::kNative,
       std::move(custom_theme));
 }
 

diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h
@@ -385,9 +385,11 @@ class NATIVE_THEME_EXPORT NativeTheme {
   };
 
   // Returns the key corresponding to this native theme object.
+  // Use `use_custom_frame` == true when Chromium renders the titlebar.
+  // False when the window manager renders the titlebar (currently GTK only).
   ColorProviderManager::Key GetColorProviderKey(
-      scoped_refptr<ColorProviderManager::InitializerSupplier> custom_theme)
-      const;
+      scoped_refptr<ColorProviderManager::InitializerSupplier> custom_theme,
+      bool use_custom_frame = true) const;
 
   // Returns a shared instance of the native theme that should be used for web
   // rendering. Do not use it in a normal application context (i.e. browser).