Rejigger ThemeService: move exposure of ThemeProvider interface to a

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

Index: chrome/browser/ui/views/toolbar/toolbar_action_view.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
index 524fb9525cedfeabefa855b52dad6f68772f4f7d..6907dec1c35443ac6c58dac4bee2cbeb0a00dc1a 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
@@ -10,8 +10,6 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
-#include "chrome/browser/themes/theme_service.h"
-#include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/browser/ui/view_ids.h"
@@ -86,14 +84,6 @@ ToolbarActionView::ToolbarActionView(
       kInkDropLargeSize, kInkDropLargeCornerRadius, kInkDropSmallSize,
       kInkDropSmallCornerRadius);
 
-  // We also listen for browser theme changes on linux because a switch from or
-  // to GTK requires that we regrab our browser action images.
-  registrar_.Add(
-      this,
-      chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
-      content::Source<ThemeService>(
-          ThemeServiceFactory::GetForProfile(profile_)));
-
   // If the button is within a menu, we need to make it focusable in order to
   // have it accessible via keyboard navigation, but it shouldn't request focus
   // (because that would close the menu).
@@ -111,6 +101,10 @@ ToolbarActionView::~ToolbarActionView() {
   view_controller_->SetDelegate(nullptr);
 }
 
+void ToolbarActionView::OnThemeChanged() {
+  UpdateState();
+}

[pkotwicz, 2015/12/04 23:16:50]

Is OnThemeChanged() necessary? I don't see where this class accesses theme data.

Regardless of whether OnThemeChanged() is necessary, splitting off the changes
to this file into a separate CL sounds like a good idea.

[Evan Stade, 2015/12/09 00:57:11]

you're right, it's not necessary. I forgot to remove this when I changed
UpdateState to not care about the theme (since it doesn't seem to actually be a
themed image).

+
 void ToolbarActionView::GetAccessibleState(ui::AXViewState* state) {
   views::MenuButton::GetAccessibleState(state);
   state->role = ui::AX_ROLE_BUTTON;
@@ -157,9 +151,8 @@ void ToolbarActionView::UpdateState() {
                                 GetPreferredSize()).AsImageSkia());
 
   if (!icon.isNull()) {
-    ThemeService* theme = ThemeServiceFactory::GetForProfile(profile_);
-
-    gfx::ImageSkia bg = *theme->GetImageSkiaNamed(IDR_BROWSER_ACTION);
+    gfx::ImageSkia bg = *ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+        IDR_BROWSER_ACTION);
     SetImage(views::Button::STATE_NORMAL,
              gfx::ImageSkiaOperations::CreateSuperimposedImage(bg, icon));
   }
@@ -184,13 +177,6 @@ void ToolbarActionView::OnMenuButtonClicked(views::View* sender,
   }
 }
 
-void ToolbarActionView::Observe(int type,
-                                const content::NotificationSource& source,
-                                const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, type);
-  UpdateState();
-}
-
 void ToolbarActionView::AddInkDropLayer(ui::Layer* ink_drop_layer) {
   SetPaintToLayer(true);
   SetFillsBoundsOpaquely(false);