linux_aura: Keep GTK state from leaking between profiles.

The views::LinuxUI* object is global across profiles. However, it stored profile specific state in the form of a |use_gtk_| variable. This fixes Gtk2Borders so that they ask their owning view for a ThemeProvider and expands the ui::ThemeProvider interface so that it exposes the UsingNativeTheme() call from the ThemeService layer, instead of asking the views::LinuxUI provider directly, which will likely return stale state. BUG=340805, 340799 Review URL: https://codereview.chromium.org/171413002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252085 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Feb 19, 2014 · d585229 · d585229
1 parent c40bb80
commit d585229
Show file tree

Hide file tree

Showing 15 changed files with 39 additions and 46 deletions.
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
@@ -118,6 +118,10 @@ gfx::Image ThemeService::GetImageNamed(int id) const {
   return image;
 }
 
+bool ThemeService::UsingNativeTheme() const {
+  return UsingDefaultTheme();
+}
+
 gfx::ImageSkia* ThemeService::GetImageSkiaNamed(int id) const {
   gfx::Image image = GetImageNamed(id);
   if (image.IsEmpty())
@@ -384,10 +388,6 @@ bool ThemeService::UsingDefaultTheme() const {
       id == kDefaultThemeGalleryID;
 }
 
-bool ThemeService::UsingNativeTheme() const {
-  return UsingDefaultTheme();
-}
-
 std::string ThemeService::GetThemeID() const {
   return profile_->GetPrefs()->GetString(prefs::kCurrentThemeID);
 }

diff --git a/chrome/browser/themes/theme_service.h b/chrome/browser/themes/theme_service.h
@@ -76,6 +76,7 @@ class ThemeService : public base::NonThreadSafe,
   virtual gfx::Image GetImageNamed(int id) const;
 
   // Overridden from ui::ThemeProvider:
+  virtual bool UsingNativeTheme() const OVERRIDE;
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
   virtual int GetDisplayProperty(int id) const OVERRIDE;
@@ -119,10 +120,6 @@ class ThemeService : public base::NonThreadSafe,
   // if we're using the GTK theme.
   virtual bool UsingDefaultTheme() const;
 
-  // Whether we're using the native theme (which may or may not be the
-  // same as the default theme).
-  virtual bool UsingNativeTheme() const;
-
   // Gets the id of the last installed theme. (The theme may have been further
   // locally customized.)
   virtual std::string GetThemeID() const;

diff --git a/chrome/browser/themes/theme_service_aurax11.cc b/chrome/browser/themes/theme_service_aurax11.cc
@@ -41,16 +41,10 @@ NativeThemeX11::NativeThemeX11(PrefService* pref_service)
       pref_service_(pref_service) {}
 
 void NativeThemeX11::StartUsingTheme() {
-  if (linux_ui_)
-    linux_ui_->SetUseSystemTheme(true);
-
   pref_service_->SetBoolean(prefs::kUsesSystemTheme, true);
 }
 
 void NativeThemeX11::StopUsingTheme() {
-  if (linux_ui_)
-    linux_ui_->SetUseSystemTheme(false);
-
   pref_service_->SetBoolean(prefs::kUsesSystemTheme, false);
 }
 

diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm
@@ -209,6 +209,9 @@ - (void)setDragDataNode:(const BookmarkNode*)node {
   FakeTheme(NSColor* color) : color_(color) {}
   base::scoped_nsobject<NSColor> color_;
 
+  virtual bool UsingNativeTheme() const OVERRIDE {
+    return true;
+  }
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE {
     return NULL;
   }

diff --git a/chrome/browser/ui/cocoa/download/background_theme.h b/chrome/browser/ui/cocoa/download/background_theme.h
@@ -16,6 +16,7 @@ class BackgroundTheme : public ui::ThemeProvider {
   virtual ~BackgroundTheme();
 
   // Overridden from ui::ThemeProvider:
+  virtual bool UsingNativeTheme() const OVERRIDE;
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
   virtual int GetDisplayProperty(int id) const OVERRIDE;

diff --git a/chrome/browser/ui/cocoa/download/background_theme.mm b/chrome/browser/ui/cocoa/download/background_theme.mm
@@ -27,6 +27,10 @@
 
 BackgroundTheme::~BackgroundTheme() {}
 
+bool BackgroundTheme::UsingNativeTheme() const {
+  return true;
+}
+
 gfx::ImageSkia* BackgroundTheme::GetImageSkiaNamed(int id) const {
   return NULL;
 }

diff --git a/chrome/browser/ui/libgtk2ui/gtk2_border.cc b/chrome/browser/ui/libgtk2ui/gtk2_border.cc
@@ -8,6 +8,7 @@
 
 #include "chrome/browser/ui/libgtk2ui/gtk2_ui.h"
 #include "third_party/skia/include/effects/SkLerpXfermode.h"
+#include "ui/base/theme_provider.h"
 #include "ui/gfx/animation/animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image_skia_source.h"
@@ -73,7 +74,6 @@ Gtk2Border::Gtk2Border(Gtk2UI* gtk2_ui,
                        views::LabelButton* owning_button,
                        scoped_ptr<views::Border> border)
     : gtk2_ui_(gtk2_ui),
-      use_gtk_(gtk2_ui_->GetUseSystemTheme()),
       owning_button_(owning_button),
       border_(border.Pass()) {
   gtk2_ui_->AddGtkBorder(this);
@@ -83,7 +83,7 @@ Gtk2Border::~Gtk2Border() {
   gtk2_ui_->RemoveGtkBorder(this);
 }
 
-void Gtk2Border::InvalidateAndSetUsesGtk(bool use_gtk) {
+void Gtk2Border::InvalidateGtkImages() {
   for (int i = 0; i < kNumberOfFocusedStates; ++i) {
     for (int j = 0; j < views::Button::STATE_COUNT; ++j) {
       button_images_[i][j] = gfx::ImageSkia();
@@ -93,12 +93,11 @@ void Gtk2Border::InvalidateAndSetUsesGtk(bool use_gtk) {
   // Our owning view must have its layout invalidated because the insets could
   // have changed.
   owning_button_->InvalidateLayout();
-
-  use_gtk_ = use_gtk;
 }
 
 void Gtk2Border::Paint(const views::View& view, gfx::Canvas* canvas) {
-  if (!use_gtk_) {
+  ui::ThemeProvider* provider = owning_button_->GetThemeProvider();
+  if (!provider || !provider->UsingNativeTheme()) {
     border_->Paint(view, canvas);
     return;
   }
@@ -133,14 +132,16 @@ void Gtk2Border::Paint(const views::View& view, gfx::Canvas* canvas) {
 }
 
 gfx::Insets Gtk2Border::GetInsets() const {
-  if (!use_gtk_)
+  ui::ThemeProvider* provider = owning_button_->GetThemeProvider();
+  if (!provider || !provider->UsingNativeTheme())
     return border_->GetInsets();
 
   return gtk2_ui_->GetButtonInsets();
 }
 
 gfx::Size Gtk2Border::GetMinimumSize() const {
-  if (!use_gtk_)
+  ui::ThemeProvider* provider = owning_button_->GetThemeProvider();
+  if (!provider || !provider->UsingNativeTheme())
     return border_->GetMinimumSize();
 
   gfx::Insets insets = GetInsets();

diff --git a/chrome/browser/ui/libgtk2ui/gtk2_border.h b/chrome/browser/ui/libgtk2ui/gtk2_border.h
@@ -30,9 +30,9 @@ class Gtk2Border : public views::Border {
              scoped_ptr<views::Border> border);
   virtual ~Gtk2Border();
 
-  // Called on theme changes. We invalidate the layout, drop our cached images,
-  // and update our GTK state.
-  void InvalidateAndSetUsesGtk(bool use_gtk);
+  // Called on theme changes. We invalidate the layout and drop our cached GTK
+  // rendered images.
+  void InvalidateGtkImages();
 
   // Overridden from views::Border:
   virtual void Paint(const views::View& view, gfx::Canvas* canvas) OVERRIDE;
@@ -48,7 +48,6 @@ class Gtk2Border : public views::Border {
   bool ShouldDrawBorder(bool focused, views::Button::ButtonState state);
 
   Gtk2UI* gtk2_ui_;
-  bool use_gtk_;
 
   gfx::ImageSkia button_images_[2][views::Button::STATE_COUNT];
 

diff --git a/chrome/browser/ui/libgtk2ui/gtk2_ui.cc b/chrome/browser/ui/libgtk2ui/gtk2_ui.cc
@@ -50,7 +50,6 @@
 //
 // TODO(erg): There's still a lot that needs ported or done for the first time:
 //
-// - Render and inject the button overlay from the gtk theme.
 // - Render and inject the omnibox background.
 // - Listen for the "style-set" signal on |fake_frame_| and recreate theme
 //   colors and images.
@@ -315,7 +314,7 @@ color_utils::HSL GetDefaultTint(int id) {
 
 namespace libgtk2ui {
 
-Gtk2UI::Gtk2UI() : use_gtk_(false) {
+Gtk2UI::Gtk2UI() {
   GtkInitFromCommandLine(*CommandLine::ForCurrentProcess());
 }
 
@@ -366,7 +365,7 @@ gfx::Image Gtk2UI::GetThemeImageNamed(int id) const {
   if (it != gtk_images_.end())
     return it->second;
 
-  if (/*use_gtk_ && */ IsOverridableImage(id)) {
+  if (IsOverridableImage(id)) {
     gfx::Image image = gfx::Image(
         gfx::ImageSkia::CreateFrom1xBitmap(GenerateGtkThemeBitmap(id)));
     gtk_images_[id] = image;
@@ -443,19 +442,7 @@ double Gtk2UI::GetCursorBlinkInterval() const {
 }
 
 ui::NativeTheme* Gtk2UI::GetNativeTheme() const {
-  return use_gtk_ ? NativeThemeGtk2::instance() :
-                    ui::NativeTheme::instance();
-}
-
-void Gtk2UI::SetUseSystemTheme(bool use_system_theme) {
-  use_gtk_ = use_system_theme;
-
-  FOR_EACH_OBSERVER(Gtk2Border, border_list_,
-                    InvalidateAndSetUsesGtk(use_system_theme));
-}
-
-bool Gtk2UI::GetUseSystemTheme() const {
-  return use_gtk_;
+  return NativeThemeGtk2::instance();
 }
 
 bool Gtk2UI::GetDefaultUsesSystemTheme() const {

diff --git a/chrome/browser/ui/libgtk2ui/gtk2_ui.h b/chrome/browser/ui/libgtk2ui/gtk2_ui.h
@@ -89,8 +89,6 @@ class Gtk2UI : public views::LinuxUI {
   virtual SkColor GetInactiveSelectionFgColor() const OVERRIDE;
   virtual double GetCursorBlinkInterval() const OVERRIDE;
   virtual ui::NativeTheme* GetNativeTheme() const OVERRIDE;
-  virtual void SetUseSystemTheme(bool use_system_theme) OVERRIDE;
-  virtual bool GetUseSystemTheme() const OVERRIDE;
   virtual bool GetDefaultUsesSystemTheme() const OVERRIDE;
   virtual void SetDownloadCount(int count) const OVERRIDE;
   virtual void SetProgressFraction(float percentage) const OVERRIDE;

diff --git a/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc b/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
@@ -39,6 +39,9 @@ class DesktopThemeProvider : public ui::ThemeProvider {
       : delegate_(delegate) {
   }
 
+  virtual bool UsingNativeTheme() const OVERRIDE {
+    return delegate_->UsingNativeTheme();
+  }
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE {
     return delegate_->GetImageSkiaNamed(
         chrome::MapThemeImage(chrome::HOST_DESKTOP_TYPE_NATIVE, id));

diff --git a/ui/base/default_theme_provider.cc b/ui/base/default_theme_provider.cc
@@ -17,6 +17,10 @@ DefaultThemeProvider::DefaultThemeProvider() {}
 
 DefaultThemeProvider::~DefaultThemeProvider() {}
 
+bool DefaultThemeProvider::UsingNativeTheme() const {
+  return true;
+}
+
 gfx::ImageSkia* DefaultThemeProvider::GetImageSkiaNamed(int id) const {
   return ResourceBundle::GetSharedInstance().GetImageSkiaNamed(id);
 }

diff --git a/ui/base/default_theme_provider.h b/ui/base/default_theme_provider.h
@@ -24,6 +24,7 @@ class UI_BASE_EXPORT DefaultThemeProvider : public ThemeProvider {
   virtual ~DefaultThemeProvider();
 
   // Overridden from ui::ThemeProvider:
+  virtual bool UsingNativeTheme() const OVERRIDE;
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
   virtual int GetDisplayProperty(int id) const OVERRIDE;

diff --git a/ui/base/theme_provider.h b/ui/base/theme_provider.h
@@ -50,6 +50,10 @@ class UI_BASE_EXPORT ThemeProvider {
  public:
   virtual ~ThemeProvider();
 
+  // Whether we're using the native theme (which may or may not be the
+  // same as the default theme).
+  virtual bool UsingNativeTheme() const = 0;
+
   // Get the image specified by |id|. An implementation of ThemeProvider should
   // have its own source of ids (e.g. an enum, or external resource bundle).
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const = 0;

diff --git a/ui/views/linux_ui/linux_ui.h b/ui/views/linux_ui/linux_ui.h
@@ -77,9 +77,6 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
   // style widgets.
   virtual ui::NativeTheme* GetNativeTheme() const = 0;
 
-  virtual void SetUseSystemTheme(bool use_system_theme) = 0;
-  virtual bool GetUseSystemTheme() const = 0;
-
   // Returns whether we should be using the native theme provided by this
   // object by default.
   virtual bool GetDefaultUsesSystemTheme() const = 0;