GTK Themes: Set the text color of bookmark buttons to theme color.

http://crbug.com/13967

Review URL: http://codereview.chromium.org/155108

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20000 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/gtk/bookmark_bar_gtk.cc

@@ -46,13 +46,6 @@ const int kInstructionsPadding = 6;
 // Color of the instructional text.
 const GdkColor kInstructionsColor = GDK_COLOR_RGB(128, 128, 142);
 
-void SetUseSystemThemeGraphicsOnToolbarItems(GtkToolItem* item, bool use_gtk) {
-  GtkWidget* child = gtk_bin_get_child(GTK_BIN(item));
-  if (GTK_IS_CHROME_BUTTON(child)) {
-    gtk_chrome_button_set_use_gtk_rendering(GTK_CHROME_BUTTON(child), use_gtk);
-  }
-}
-
 }  // namespace
 
 BookmarkBarGtk::BookmarkBarGtk(Profile* profile, Browser* browser,
@@ -164,20 +157,20 @@ void BookmarkBarGtk::Init(Profile* profile) {
   ConnectFolderButtonEvents(other_bookmarks_button_);
 
   GtkWidget* image = gtk_image_new_from_pixbuf(folder_icon);
-  GtkWidget* label = gtk_label_new(
+  other_bookmarks_label_ = gtk_label_new(
       l10n_util::GetStringUTF8(IDS_BOOMARK_BAR_OTHER_BOOKMARKED).c_str());
-  bookmark_utils::SetButtonTextColors(label);
+  GtkThemeProperties properties(profile);
+  bookmark_utils::SetButtonTextColors(other_bookmarks_label_, &properties);
 
   GtkWidget* box = gtk_hbox_new(FALSE, bookmark_utils::kBarButtonPadding);
   gtk_box_pack_start(GTK_BOX(box), image, FALSE, FALSE, 0);
-  gtk_box_pack_start(GTK_BOX(box), label, FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(box), other_bookmarks_label_, FALSE, FALSE, 0);
   gtk_container_add(GTK_CONTAINER(other_bookmarks_button_), box);
 
   gtk_box_pack_start(GTK_BOX(bookmark_hbox_.get()), other_bookmarks_button_,
                      FALSE, FALSE, 0);
 
   // Set the current theme state for all the buttons.
-  UserChangedTheme(profile);
   gtk_widget_set_size_request(bookmark_hbox_.get(), -1, 0);
 
   slide_animation_.reset(new SlideAnimation(this));
@@ -274,8 +267,6 @@ void BookmarkBarGtk::BookmarkNodeAdded(BookmarkModel* model,
   GtkToolItem* item = CreateBookmarkToolItem(parent->GetChild(index));
   gtk_toolbar_insert(GTK_TOOLBAR(bookmark_toolbar_.get()),
                      item, index);
-  bool use_gtk = GtkThemeProvider::UseSystemThemeGraphics(profile_);
-  SetUseSystemThemeGraphicsOnToolbarItems(item, use_gtk);
 
   SetInstructionState(parent);
 }
@@ -309,7 +300,8 @@ void BookmarkBarGtk::BookmarkNodeChanged(BookmarkModel* model,
   GtkToolItem* item = gtk_toolbar_get_nth_item(
       GTK_TOOLBAR(bookmark_toolbar_.get()), index);
   GtkWidget* button = gtk_bin_get_child(GTK_BIN(item));
-  bookmark_utils::ConfigureButtonForNode(node, model, button);
+  GtkThemeProperties properties(profile_);
+  bookmark_utils::ConfigureButtonForNode(node, model, button, &properties);
 }
 
 void BookmarkBarGtk::BookmarkNodeFavIconLoaded(BookmarkModel* model,
@@ -334,10 +326,6 @@ void BookmarkBarGtk::CreateAllBookmarkButtons(const BookmarkNode* node) {
     gtk_toolbar_insert(GTK_TOOLBAR(bookmark_toolbar_.get()), item, -1);
   }
 
-  // Now that we've made a bunch of toolbar items, we need to make sure they
-  // have the correct theme state.
-  UserChangedTheme(profile_);
-
   SetInstructionState(node);
 }
 
@@ -367,17 +355,24 @@ bool BookmarkBarGtk::IsAlwaysShown() {
   return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
 }
 
-void BookmarkBarGtk::UserChangedTheme(Profile* profile) {
-  bool use_gtk = GtkThemeProvider::UseSystemThemeGraphics(profile);
-
+void BookmarkBarGtk::UserChangedTheme(GtkThemeProperties* properties) {
   gtk_chrome_button_set_use_gtk_rendering(
-      GTK_CHROME_BUTTON(other_bookmarks_button_), use_gtk);
-
-  gtk_container_foreach(
-      GTK_CONTAINER(bookmark_toolbar_.get()),
-      reinterpret_cast<void (*)(GtkWidget*, void*)>(
-          SetUseSystemThemeGraphicsOnToolbarItems),
-      reinterpret_cast<void*>(use_gtk));
+      GTK_CHROME_BUTTON(other_bookmarks_button_),
+      properties->use_gtk_rendering);
+  bookmark_utils::SetButtonTextColors(other_bookmarks_label_, properties);
+
+  if (model_) {
+    // Regenerate the bookmark bar with all new objects with their theme
+    // properties set correctly for the new theme.
+    RemoveAllBookmarkButtons();
+
+    const BookmarkNode* node = model_->GetBookmarkBarNode();
+    DCHECK(node && model_->other_node());
+    CreateAllBookmarkButtons(node);
+  } else {
+    DLOG(ERROR) << "Received a theme change notification while we don't have a "
+                << "BookmarkModel. Taking no action.";
+  }
 }
 
 void BookmarkBarGtk::AnimationProgressed(const Animation* animation) {
@@ -397,7 +392,8 @@ void BookmarkBarGtk::AnimationEnded(const Animation* animation) {
 
 GtkWidget* BookmarkBarGtk::CreateBookmarkButton(const BookmarkNode* node) {
   GtkWidget* button = gtk_chrome_button_new();
-  bookmark_utils::ConfigureButtonForNode(node, model_, button);
+  GtkThemeProperties properties(profile_);
+  bookmark_utils::ConfigureButtonForNode(node, model_, button, &properties);
 
   // The tool item is also a source for dragging
   gtk_drag_source_set(button, GDK_BUTTON1_MASK,
@@ -583,7 +579,9 @@ void BookmarkBarGtk::OnButtonDragBegin(GtkWidget* button,
   bar->dragged_node_ = node;
   DCHECK(bar->dragged_node_);
 
-  GtkWidget* window = bookmark_utils::GetDragRepresentation(node, bar->model_);
+  GtkThemeProperties properties(bar->profile_);
+  GtkWidget* window = bookmark_utils::GetDragRepresentation(node, bar->model_,
+                                                            &properties);
   gint x, y;
   gtk_widget_get_pointer(button, &x, &y);
   gtk_drag_set_icon_widget(drag_context, window, x, y);

chrome/browser/gtk/bookmark_bar_gtk.h

@@ -21,6 +21,7 @@ class CustomContainerButton;
 class NineBox;
 class PageNavigator;
 class Profile;
+struct GtkThemeProperties;
 
 class BookmarkBarGtk : public AnimationDelegate,
                        public BookmarkModelObserver {
@@ -68,7 +69,7 @@ class BookmarkBarGtk : public AnimationDelegate,
   bool IsAlwaysShown();
 
   // Alerts us that the theme changed, and we might need to change theme images.
-  void UserChangedTheme(Profile* profile);
+  void UserChangedTheme(GtkThemeProperties* properties);
 
   // AnimationDelegate implementation ------------------------------------------
   virtual void AnimationProgressed(const Animation* animation);
@@ -211,6 +212,10 @@ class BookmarkBarGtk : public AnimationDelegate,
   // The other bookmarks button.
   GtkWidget* other_bookmarks_button_;
 
+  // The label inside |other_bookmarks_button_|. We keep a reference so we can
+  // change the text color.
+  GtkWidget* other_bookmarks_label_;
+
   // Whether we should ignore the next button release event (because we were
   // dragging).
   bool ignore_button_release_;

chrome/browser/gtk/bookmark_menu_controller_gtk.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/gtk/bookmark_context_menu.h"
 #include "chrome/browser/gtk/bookmark_utils_gtk.h"
 #include "chrome/browser/gtk/gtk_dnd_util.h"
+#include "chrome/browser/gtk/gtk_theme_provider.h"
 #include "chrome/browser/gtk/menu_gtk.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/page_navigator.h"
@@ -224,8 +225,9 @@ void BookmarkMenuController::OnMenuItemDragBegin(
   controller->ignore_button_release_ = true;
 
   const BookmarkNode* node = bookmark_utils::BookmarkNodeForWidget(menu_item);
-  GtkWidget* window = bookmark_utils::GetDragRepresentation(node,
-                                                            controller->model_);
+  GtkThemeProperties properties(controller->profile_);
+  GtkWidget* window = bookmark_utils::GetDragRepresentation(
+      node, controller->model_, &properties);
   gint x, y;
   gtk_widget_get_pointer(menu_item, &x, &y);
   gtk_drag_set_icon_widget(drag_context, window, x, y);

chrome/browser/gtk/bookmark_utils_gtk.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/gtk/gtk_chrome_button.h"
 #include "chrome/browser/gtk/gtk_dnd_util.h"
+#include "chrome/browser/gtk/gtk_theme_provider.h"
 #include "chrome/browser/profile.h"
 #include "grit/app_resources.h"
 #include "grit/generated_resources.h"
@@ -29,14 +30,6 @@ const size_t kMaxCharsOnAButton = 15;
 // Only used for the background of the drag widget.
 const GdkColor kBackgroundColor = GDK_COLOR_RGB(0xe6, 0xed, 0xf4);
 
-// Color of the button text, taken from TextButtonView.
-const GdkColor kEnabledColor = GDK_COLOR_RGB(6, 45, 117);
-const GdkColor kDisabledColor = GDK_COLOR_RGB(161, 161, 146);
-// TextButtonView uses 255, 255, 255 with opacity of 200. We don't support
-// transparent text though, so just use a slightly lighter version of
-// kEnabledColor.
-const GdkColor kHighlightColor = GDK_COLOR_RGB(56, 95, 167);
-
 void* AsVoid(const BookmarkNode* node) {
   return const_cast<BookmarkNode*>(node);
 }
@@ -82,10 +75,15 @@ GdkPixbuf* GetPixbufForNode(const BookmarkNode* node, BookmarkModel* model) {
 }
 
 GtkWidget* GetDragRepresentation(const BookmarkNode* node,
-                                 BookmarkModel* model) {
+                                 BookmarkModel* model,
+                                 GtkThemeProperties* properties) {
   // Build a windowed representation for our button.
   GtkWidget* window = gtk_window_new(GTK_WINDOW_POPUP);
-  gtk_widget_modify_bg(window, GTK_STATE_NORMAL, &kBackgroundColor);
+  if (!properties->use_gtk_rendering) {
+    // TODO(erg): Theme wise, which color should I be picking here?
+    // COLOR_BUTTON_BACKGROUND doesn't match the default theme!
+    gtk_widget_modify_bg(window, GTK_STATE_NORMAL, &kBackgroundColor);
+  }
   gtk_widget_realize(window);
 
   GtkWidget* frame = gtk_frame_new(NULL);
@@ -94,7 +92,8 @@ GtkWidget* GetDragRepresentation(const BookmarkNode* node,
   gtk_widget_show(frame);
 
   GtkWidget* floating_button = gtk_chrome_button_new();
-  bookmark_utils::ConfigureButtonForNode(node, model, floating_button);
+  bookmark_utils::ConfigureButtonForNode(node, model, floating_button,
+                                         properties);
   gtk_container_add(GTK_CONTAINER(frame), floating_button);
   gtk_widget_show(floating_button);
 
@@ -102,7 +101,7 @@ GtkWidget* GetDragRepresentation(const BookmarkNode* node,
 }
 
 void ConfigureButtonForNode(const BookmarkNode* node, BookmarkModel* model,
-                            GtkWidget* button) {
+                            GtkWidget* button, GtkThemeProperties* properties) {
   GtkWidget* former_child = gtk_bin_get_child(GTK_BIN(button));
   if (former_child)
     gtk_container_remove(GTK_CONTAINER(button), former_child);
@@ -126,10 +125,13 @@ void ConfigureButtonForNode(const BookmarkNode* node, BookmarkModel* model,
   gtk_box_pack_start(GTK_BOX(box), label, FALSE, FALSE, 0);
   gtk_container_add(GTK_CONTAINER(button), box);
 
-  SetButtonTextColors(label);
+  SetButtonTextColors(label, properties);
   g_object_set_data(G_OBJECT(button), bookmark_utils::kBookmarkNode,
                     AsVoid(node));
 
+  gtk_chrome_button_set_use_gtk_rendering(GTK_CHROME_BUTTON(button),
+                                          properties->use_gtk_rendering);
+
   gtk_widget_show_all(box);
 }
 
@@ -144,11 +146,20 @@ const BookmarkNode* BookmarkNodeForWidget(GtkWidget* widget) {
       g_object_get_data(G_OBJECT(widget), bookmark_utils::kBookmarkNode));
 }
 
-void SetButtonTextColors(GtkWidget* label) {
-  gtk_widget_modify_fg(label, GTK_STATE_NORMAL, &kEnabledColor);
-  gtk_widget_modify_fg(label, GTK_STATE_ACTIVE, &kEnabledColor);
-  gtk_widget_modify_fg(label, GTK_STATE_PRELIGHT, &kHighlightColor);
-  gtk_widget_modify_fg(label, GTK_STATE_INSENSITIVE, &kDisabledColor);
+void SetButtonTextColors(GtkWidget* label, GtkThemeProperties* properties) {
+  if (properties->use_gtk_rendering) {
+    gtk_widget_modify_fg(label, GTK_STATE_NORMAL, NULL);
+    gtk_widget_modify_fg(label, GTK_STATE_ACTIVE, NULL);
+    gtk_widget_modify_fg(label, GTK_STATE_PRELIGHT, NULL);
+    gtk_widget_modify_fg(label, GTK_STATE_INSENSITIVE, NULL);
+  } else {
+    GdkColor color = properties->GetGdkColor(
+        BrowserThemeProvider::COLOR_BOOKMARK_TEXT);
+    gtk_widget_modify_fg(label, GTK_STATE_NORMAL, &color);
+    gtk_widget_modify_fg(label, GTK_STATE_ACTIVE, &color);
+    gtk_widget_modify_fg(label, GTK_STATE_PRELIGHT, &color);
+    gtk_widget_modify_fg(label, GTK_STATE_INSENSITIVE, &color);
+  }
 }
 
 // DnD-related -----------------------------------------------------------------

chrome/browser/gtk/bookmark_utils_gtk.h

@@ -12,6 +12,7 @@
 class BookmarkModel;
 class BookmarkNode;
 class Profile;
+struct GtkThemeProperties;
 
 namespace bookmark_utils {
 
@@ -32,12 +33,13 @@ GdkPixbuf* GetPixbufForNode(const BookmarkNode* node, BookmarkModel* model);
 // Returns a GtkWindow with a visual hierarchy for passing to
 // gtk_drag_set_icon_widget().
 GtkWidget* GetDragRepresentation(const BookmarkNode* node,
-                                 BookmarkModel* model);
+                                 BookmarkModel* model,
+                                 GtkThemeProperties* properties);
 
 // Helper function that sets visual properties of GtkButton |button| to the
 // contents of |node|.
 void ConfigureButtonForNode(const BookmarkNode* node, BookmarkModel* model,
-                            GtkWidget* button);
+                            GtkWidget* button, GtkThemeProperties* properties);
 
 // Returns the tooltip.
 std::string BuildTooltipFor(const BookmarkNode* node);
@@ -47,7 +49,7 @@ const BookmarkNode* BookmarkNodeForWidget(GtkWidget* widget);
 
 // This function is a temporary hack to fix fonts on dark system themes.
 // TODO(estade): remove this function.
-void SetButtonTextColors(GtkWidget* label);
+void SetButtonTextColors(GtkWidget* label, GtkThemeProperties* properties);
 
 // Drag and drop. --------------------------------------------------------------
 

chrome/browser/gtk/browser_window_gtk.cc

@@ -717,7 +717,8 @@ void BrowserWindowGtk::UserChangedTheme() {
       &GTK_WIDGET(window_)->allocation, TRUE);
 
   toolbar_->UserChangedTheme();
-  bookmark_bar_->UserChangedTheme(browser_->profile());
+  GtkThemeProperties properties(browser_->profile());
+  bookmark_bar_->UserChangedTheme(&properties);
 }
 
 int BrowserWindowGtk::GetExtraRenderViewHeight() const {

chrome/browser/gtk/gtk_theme_provider.cc

@@ -13,6 +13,7 @@
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace {
 
@@ -152,3 +153,15 @@ void GtkThemeProvider::SetThemeTintFromGtk(const char* id, GdkColor* color,
   SetTint(id, hsl);
 }
 
+GtkThemeProperties::GtkThemeProperties(Profile* profile)
+    : use_gtk_rendering(GtkThemeProvider::UseSystemThemeGraphics(profile)),
+      provider(profile->GetThemeProvider()) {
+}
+
+GdkColor GtkThemeProperties::GetGdkColor(int id) {
+  SkColor color = provider->GetColor(id);
+  GdkColor gdkcolor =
+      GDK_COLOR_RGB(SkColorGetR(color), SkColorGetG(color),
+                    SkColorGetB(color));
+  return gdkcolor;
+}

chrome/browser/gtk/gtk_theme_provider.h

@@ -58,4 +58,20 @@ class GtkThemeProvider : public BrowserThemeProvider {
   GtkWidget* fake_window_;
 };
 
+// A Helper struct used throughout the GTK themeing code. It retrieves settings
+// from the Profile once at creation time, instead of at every access time. A
+// large motivation for making this struct is so that we only have to pass one
+// parameter around when configuring things due to a theme event. This way, we
+// can use a lot of GTK convenience features likt gtk_container_foreach().
+struct GtkThemeProperties {
+  GtkThemeProperties(Profile* profile);
+
+  // A wrapper around ThemeProvider::GetColor, transforming the result to a
+  // GdkColor.
+  GdkColor GetGdkColor(int id);
+
+  bool use_gtk_rendering;
+  ThemeProvider* provider;
+};
+
 #endif  // CHROME_BROWSER_GTK_GTK_THEME_PROVIDER_H_