Linux/X11: Respond to changes in cursor theme

This change makes it so you don't have to resetart Chrome to see cursor theme
changes. Also move some X11 code out of gtk_ui.cc which didn't belong there.

BUG=1013426

Change-Id: I1bd3b98cf374783c6805ac23fc839bbf4b21395b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869557
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709216}

chrome/browser/ui/libgtkui/gtk_ui.cc

@@ -39,6 +39,7 @@
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkShader.h"
+#include "ui/base/cursor/cursor_theme_manager_linux_observer.h"
 #include "ui/base/ime/linux/fake_input_method_context.h"
 #include "ui/base/ime/linux/linux_input_method_context.h"
 #include "ui/base/ime/linux/linux_input_method_context_factory.h"
@@ -435,6 +436,10 @@ void GtkUi::Initialize() {
                          G_CALLBACK(OnThemeChangedThunk), this);
   g_signal_connect_after(settings, "notify::gtk-application-prefer-dark-theme",
                          G_CALLBACK(OnThemeChangedThunk), this);
+  g_signal_connect_after(settings, "notify::gtk-cursor-theme-name",
+                         G_CALLBACK(OnCursorThemeNameChangedThunk), this);
+  g_signal_connect_after(settings, "notify::gtk-cursor-theme-size",
+                         G_CALLBACK(OnCursorThemeSizeChangedThunk), this);
 
   GdkScreen* screen = gdk_screen_get_default();
   // Listen for DPI changes.
@@ -628,14 +633,14 @@ std::unique_ptr<views::Border> GtkUi::CreateNativeBorder(
     bool focus;
     views::Button::ButtonState state;
   } const paintstate[] = {
-      { !kFocus, views::Button::STATE_NORMAL, },
-      { !kFocus, views::Button::STATE_HOVERED, },
-      { !kFocus, views::Button::STATE_PRESSED, },
-      { !kFocus, views::Button::STATE_DISABLED, },
-      { kFocus, views::Button::STATE_NORMAL, },
-      { kFocus, views::Button::STATE_HOVERED, },
-      { kFocus, views::Button::STATE_PRESSED, },
-      { kFocus, views::Button::STATE_DISABLED, },
+      {!kFocus, views::Button::STATE_NORMAL},
+      {!kFocus, views::Button::STATE_HOVERED},
+      {!kFocus, views::Button::STATE_PRESSED},
+      {!kFocus, views::Button::STATE_DISABLED},
+      {kFocus, views::Button::STATE_NORMAL},
+      {kFocus, views::Button::STATE_HOVERED},
+      {kFocus, views::Button::STATE_PRESSED},
+      {kFocus, views::Button::STATE_DISABLED},
   };
 
   for (unsigned i = 0; i < base::size(paintstate); i++) {
@@ -804,6 +809,25 @@ base::flat_map<std::string, std::string> GtkUi::GetKeyboardLayoutMap() {
   return layouts->GetFirstAsciiCapableLayout()->GetMap();
 }
 
+std::string GtkUi::GetCursorThemeName() {
+  gchar* theme = nullptr;
+  g_object_get(gtk_settings_get_default(), "gtk-cursor-theme-name", &theme,
+               nullptr);
+  std::string theme_string;
+  if (theme) {
+    theme_string = theme;
+    g_free(theme);
+  }
+  return theme_string;
+}
+
+int GtkUi::GetCursorThemeSize() {
+  gint size = 0;
+  g_object_get(gtk_settings_get_default(), "gtk-cursor-theme-size", &size,
+               nullptr);
+  return size;
+}
+
 bool GtkUi::MatchEvent(const ui::Event& event,
                        std::vector<ui::TextEditCommandAuraLinux>* commands) {
   // Ensure that we have a keyboard handler.
@@ -822,6 +846,20 @@ void GtkUi::OnThemeChanged(GtkSettings* settings, GtkParamSpec* param) {
   native_theme_->NotifyObservers();
 }
 
+void GtkUi::OnCursorThemeNameChanged(GtkSettings* settings,
+                                     GtkParamSpec* param) {
+  std::string cursor_theme_name = GetCursorThemeName();
+  for (auto& observer : cursor_theme_observers())
+    observer.OnCursorThemeNameChanged(cursor_theme_name);
+}
+
+void GtkUi::OnCursorThemeSizeChanged(GtkSettings* settings,
+                                     GtkParamSpec* param) {
+  int cursor_theme_size = GetCursorThemeSize();
+  for (auto& observer : cursor_theme_observers())
+    observer.OnCursorThemeSizeChanged(cursor_theme_size);
+}
+
 void GtkUi::OnDeviceScaleFactorMaybeChanged(void*, GParamSpec*) {
   UpdateDeviceScaleFactor();
 }
@@ -832,7 +870,6 @@ void GtkUi::LoadGtkValues() {
   // we'd regress startup time. Figure out how to do that when we can't access
   // the prefs system from here.
   UpdateDeviceScaleFactor();
-  UpdateCursorTheme();
   UpdateColors();
 }
 
@@ -999,31 +1036,6 @@ void GtkUi::UpdateColors() {
   }
 }
 
-void GtkUi::UpdateCursorTheme() {
-#if defined(USE_X11)
-  GtkSettings* settings = gtk_settings_get_default();
-
-  gchar* theme = nullptr;
-  gint size = 0;
-  g_object_get(settings, "gtk-cursor-theme-name", &theme,
-               "gtk-cursor-theme-size", &size, nullptr);
-
-  if (theme)
-    XcursorSetTheme(gfx::GetXDisplay(), theme);
-  if (size)
-    XcursorSetDefaultSize(gfx::GetXDisplay(), size);
-
-  g_free(theme);
-#else
-  // TODO(thomasanderson): GtkUi shouldn't be the class to make X11 or wayland
-  // calls. Instead, this function should be a getter for X11
-  // (XCursorSetTheme/XcursorSetDefaultSize) and Wayland (wl_cursor_theme_load)
-  // cursor code to call into. In addition, cursor theme name/size changes are
-  // not handled, so an observer interface should be added as well.
-  NOTIMPLEMENTED();
-#endif
-}
-
 void GtkUi::UpdateDefaultFont() {
   gfx::SetFontRenderParamsDeviceScaleFactor(device_scale_factor_);
 

chrome/browser/ui/libgtkui/gtk_ui.h

@@ -104,6 +104,8 @@ class GtkUi : public views::LinuxUI {
   std::unique_ptr<views::NavButtonProvider> CreateNavButtonProvider() override;
 #endif
   base::flat_map<std::string, std::string> GetKeyboardLayoutMap() override;
+  std::string GetCursorThemeName() override;
+  int GetCursorThemeSize() override;
 
   // ui::TextEditKeybindingDelegate:
   bool MatchEvent(const ui::Event& event,
@@ -114,6 +116,18 @@ class GtkUi : public views::LinuxUI {
 
   CHROMEG_CALLBACK_1(GtkUi, void, OnThemeChanged, GtkSettings*, GtkParamSpec*);
 
+  CHROMEG_CALLBACK_1(GtkUi,
+                     void,
+                     OnCursorThemeNameChanged,
+                     GtkSettings*,
+                     GtkParamSpec*);
+
+  CHROMEG_CALLBACK_1(GtkUi,
+                     void,
+                     OnCursorThemeSizeChanged,
+                     GtkSettings*,
+                     GtkParamSpec*);
+
   CHROMEG_CALLBACK_1(GtkUi,
                      void,
                      OnDeviceScaleFactorMaybeChanged,
@@ -127,9 +141,6 @@ class GtkUi : public views::LinuxUI {
   // ThemeService interface and the colors we send to Blink.
   void UpdateColors();
 
-  // Sets the Xcursor theme and size with the GTK theme and size.
-  void UpdateCursorTheme();
-
   // Updates |default_font_*|.
   void UpdateDefaultFont();
 

ui/base/BUILD.gn

@@ -511,6 +511,14 @@ jumbo_component("base") {
     deps += [ "//third_party/fontconfig" ]
   }
 
+  if (is_desktop_linux) {
+    sources += [
+      "cursor/cursor_theme_manager_linux.cc",
+      "cursor/cursor_theme_manager_linux.h",
+      "cursor/cursor_theme_manager_linux_observer.h",
+    ]
+  }
+
   if (use_glib) {
     configs += [ "//build/config/linux:glib" ]
     sources += [

ui/base/cursor/cursor_loader_x11.cc

@@ -81,7 +81,14 @@ CursorLoaderX11::ImageCursor::~ImageCursor() {
 
 CursorLoaderX11::CursorLoaderX11()
     : display_(gfx::GetXDisplay()),
-      invisible_cursor_(CreateInvisibleCursor(), gfx::GetXDisplay()) {}
+      invisible_cursor_(CreateInvisibleCursor(), gfx::GetXDisplay()) {
+  auto* cursor_theme_manager = CursorThemeManagerLinux::GetInstance();
+  if (cursor_theme_manager) {
+    cursor_theme_observer_.Add(cursor_theme_manager);
+    OnCursorThemeNameChanged(cursor_theme_manager->GetCursorThemeName());
+    OnCursorThemeSizeChanged(cursor_theme_manager->GetCursorThemeSize());
+  }
+}
 
 CursorLoaderX11::~CursorLoaderX11() {
   UnloadAll();
@@ -152,14 +159,23 @@ const XcursorImage* CursorLoaderX11::GetXcursorImageForTest(CursorType id) {
   return test::GetCachedXcursorImage(image_cursors_[id]->cursor);
 }
 
+void CursorLoaderX11::OnCursorThemeNameChanged(
+    const std::string& cursor_theme_name) {
+  XcursorSetTheme(display_, cursor_theme_name.c_str());
+  ClearThemeCursors();
+}
+
+void CursorLoaderX11::OnCursorThemeSizeChanged(int cursor_theme_size) {
+  XcursorSetDefaultSize(display_, cursor_theme_size);
+  ClearThemeCursors();
+}
+
 bool CursorLoaderX11::IsImageCursor(gfx::NativeCursor native_cursor) {
   CursorType type = native_cursor.native_type();
   return image_cursors_.count(type) || animated_cursors_.count(type);
 }
 
 ::Cursor CursorLoaderX11::CursorFromId(CursorType id) {
-  const char* css_name = CursorCssNameFromId(id);
-
   auto font_it = font_cursors_.find(id);
   if (font_it != font_cursors_.end())
     return font_it->second;
@@ -174,6 +190,7 @@ bool CursorLoaderX11::IsImageCursor(gfx::NativeCursor native_cursor) {
   }
 
   // First try to load the cursor directly.
+  const char* css_name = CursorCssNameFromId(id);
   ::Cursor cursor = XcursorLibraryLoadCursor(display_, css_name);
   if (cursor == x11::None) {
     // Try a similar cursor supplied by the native cursor theme.
@@ -208,4 +225,9 @@ bool CursorLoaderX11::IsImageCursor(gfx::NativeCursor native_cursor) {
   return cursor;
 }
 
+void CursorLoaderX11::ClearThemeCursors() {
+  font_cursors_.clear();
+  image_cursors_.clear();
+}
+
 }  // namespace ui

ui/base/cursor/cursor_loader_x11.h

@@ -9,15 +9,19 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "ui/base/cursor/cursor.h"
 #include "ui/base/cursor/cursor_loader.h"
+#include "ui/base/cursor/cursor_theme_manager_linux.h"
+#include "ui/base/cursor/cursor_theme_manager_linux_observer.h"
 #include "ui/base/ui_base_export.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/gfx/x/x11.h"
 
 namespace ui {
 
-class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader {
+class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader,
+                                       public CursorThemeManagerLinuxObserver {
  public:
   CursorLoaderX11();
   ~CursorLoaderX11() override;
@@ -35,6 +39,11 @@ class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader {
 
   const XcursorImage* GetXcursorImageForTest(CursorType id);
 
+ protected:
+  // CursorThemeManagerLinux:
+  void OnCursorThemeNameChanged(const std::string& cursor_theme_name) override;
+  void OnCursorThemeSizeChanged(int cursor_theme_size) override;
+
  private:
   struct ImageCursor {
     ImageCursor(XcursorImage* x_image,
@@ -53,6 +62,8 @@ class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader {
   // Loads a new cursor corresponding to |id|.
   ::Cursor CursorFromId(CursorType id);
 
+  void ClearThemeCursors();
+
   XDisplay* display_;
 
   // A map from a cursor native type to X cursor.
@@ -69,6 +80,9 @@ class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader {
 
   const XScopedCursor invisible_cursor_;
 
+  ScopedObserver<CursorThemeManagerLinux, CursorThemeManagerLinuxObserver>
+      cursor_theme_observer_{this};
+
   DISALLOW_COPY_AND_ASSIGN(CursorLoaderX11);
 };
 

ui/base/cursor/cursor_theme_manager_linux.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/base/cursor/cursor_theme_manager_linux.h"
+
+namespace ui {
+
+// static
+CursorThemeManagerLinux* CursorThemeManagerLinux::instance_ = nullptr;
+
+// static
+void CursorThemeManagerLinux::SetInstance(CursorThemeManagerLinux* instance) {
+  instance_ = instance;
+}
+
+// static
+CursorThemeManagerLinux* CursorThemeManagerLinux::GetInstance() {
+  return instance_;
+}
+
+CursorThemeManagerLinux::CursorThemeManagerLinux() = default;
+
+CursorThemeManagerLinux::~CursorThemeManagerLinux() = default;
+
+void CursorThemeManagerLinux::AddObserver(
+    CursorThemeManagerLinuxObserver* observer) {
+  cursor_theme_observers_.AddObserver(observer);
+}
+
+void CursorThemeManagerLinux::RemoveObserver(
+    CursorThemeManagerLinuxObserver* observer) {
+  cursor_theme_observers_.RemoveObserver(observer);
+}
+
+}  // namespace ui

ui/base/cursor/cursor_theme_manager_linux.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_H_
+#define UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "ui/base/cursor/cursor_theme_manager_linux_observer.h"
+#include "ui/base/ui_base_export.h"
+
+namespace ui {
+
+class UI_BASE_EXPORT CursorThemeManagerLinux {
+ public:
+  virtual ~CursorThemeManagerLinux();
+
+  static void SetInstance(CursorThemeManagerLinux* instance);
+
+  static CursorThemeManagerLinux* GetInstance();
+
+  virtual std::string GetCursorThemeName() = 0;
+  virtual int GetCursorThemeSize() = 0;
+  void AddObserver(CursorThemeManagerLinuxObserver* observer);
+  void RemoveObserver(CursorThemeManagerLinuxObserver* observer);
+
+ protected:
+  CursorThemeManagerLinux();
+
+  const base::ObserverList<CursorThemeManagerLinuxObserver>&
+  cursor_theme_observers() {
+    return cursor_theme_observers_;
+  }
+
+ private:
+  static CursorThemeManagerLinux* instance_;
+
+  base::ObserverList<CursorThemeManagerLinuxObserver> cursor_theme_observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(CursorThemeManagerLinux);
+};
+
+}  // namespace ui
+
+#endif  // UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_H_

ui/base/cursor/cursor_theme_manager_linux_observer.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_OBSERVER_H_
+#define UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_OBSERVER_H_
+
+#include <string>
+
+#include "base/observer_list_types.h"
+#include "ui/base/ui_base_export.h"
+
+namespace ui {
+
+class UI_BASE_EXPORT CursorThemeManagerLinuxObserver
+    : public base::CheckedObserver {
+ public:
+  virtual void OnCursorThemeNameChanged(
+      const std::string& cursor_theme_name) = 0;
+  virtual void OnCursorThemeSizeChanged(int cursor_theme_size) = 0;
+
+ protected:
+  ~CursorThemeManagerLinuxObserver() override = default;
+};
+
+}  // namespace ui
+
+#endif  // UI_BASE_CURSOR_CURSOR_THEME_MANAGER_LINUX_OBSERVER_H_

ui/views/linux_ui/linux_ui.cc

@@ -28,10 +28,15 @@ void LinuxUI::SetInstance(LinuxUI* instance) {
   SkiaFontDelegate::SetInstance(instance);
   ShellDialogLinux::SetInstance(instance);
   ui::SetTextEditKeyBindingsDelegate(instance);
+  ui::CursorThemeManagerLinux::SetInstance(instance);
 }
 
 LinuxUI* LinuxUI::instance() {
   return g_linux_ui;
 }
 
+LinuxUI::LinuxUI() = default;
+
+LinuxUI::~LinuxUI() = default;
+
 }  // namespace views

ui/views/linux_ui/linux_ui.h

@@ -8,8 +8,10 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/macros.h"
 #include "build/buildflag.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/base/cursor/cursor_theme_manager_linux.h"
 #include "ui/base/ime/linux/linux_input_method_context_factory.h"
 #include "ui/base/ime/linux/text_edit_key_bindings_delegate_auralinux.h"
 #include "ui/gfx/skia_font_delegate.h"
@@ -59,8 +61,12 @@ class NavButtonProvider;
 class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
                              public gfx::SkiaFontDelegate,
                              public ui::ShellDialogLinux,
-                             public ui::TextEditKeyBindingsDelegateAuraLinux {
+                             public ui::TextEditKeyBindingsDelegateAuraLinux,
+                             public ui::CursorThemeManagerLinux {
  public:
+  using NativeThemeGetter =
+      base::RepeatingCallback<ui::NativeTheme*(aura::Window* window)>;
+
   // Describes the window management actions that could be taken in response to
   // a middle click in the non client area.
   enum class WindowFrameAction {
@@ -78,10 +84,7 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
     kRightClick,
   };
 
-  using NativeThemeGetter =
-      base::RepeatingCallback<ui::NativeTheme*(aura::Window* window)>;
-
-  ~LinuxUI() override {}
+  ~LinuxUI() override;
 
   // Sets the dynamically loaded singleton that draws the desktop native UI.
   static void SetInstance(LinuxUI* instance);
@@ -100,7 +103,7 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
                         PrefService* pref_service) const = 0;
   virtual bool GetDisplayProperty(int id, int* result) const = 0;
 
-  // Returns the preferences that we pass to WebKit.
+  // Returns the preferences that we pass to Blink.
   virtual SkColor GetFocusRingColor() const = 0;
   virtual SkColor GetActiveSelectionBgColor() const = 0;
   virtual SkColor GetActiveSelectionFgColor() const = 0;
@@ -122,8 +125,8 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
   // Returns the icon for a given content type from the icon theme.
   // TODO(davidben): Add an observer for the theme changing, so we can drop the
   // caches.
-  virtual gfx::Image GetIconForContentType(
-      const std::string& content_type, int size) const = 0;
+  virtual gfx::Image GetIconForContentType(const std::string& content_type,
+                                           int size) const = 0;
 
   // Builds a Border which paints the native button style.
   virtual std::unique_ptr<Border> CreateNativeBorder(
@@ -180,6 +183,12 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
 
   // Returns a map of KeyboardEvent code to KeyboardEvent key values.
   virtual base::flat_map<std::string, std::string> GetKeyboardLayoutMap() = 0;
+
+ protected:
+  LinuxUI();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LinuxUI);
 };
 
 }  // namespace views