GTK: Fix regression where we don't build tinted buttons in GTK theme mode.

(Caused by my r34486)
TEST=Use GTK+ mode. page/app menu button should have same tint as title bar.
BUG=NONE

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

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

6 files changed, 69 insertions, 40 deletions

diff --git a/chrome/browser/browser_theme_pack.cc b/chrome/browser/browser_theme_pack.cc
index f30bd6b..85801ae 100644
--- a/chrome/browser/browser_theme_pack.cc
+++ b/chrome/browser/browser_theme_pack.cc
@@ -157,26 +157,6 @@ const int kPreloadIDs[] = {
   IDR_THEME_WINDOW_CONTROL_BACKGROUND
 };
 
-// The image resources that will be tinted by the 'button' tint value.
-const int kToolbarButtonIDs[] = {
-  IDR_BACK, IDR_BACK_D, IDR_BACK_H, IDR_BACK_P,
-  IDR_FORWARD, IDR_FORWARD_D, IDR_FORWARD_H, IDR_FORWARD_P,
-  IDR_RELOAD, IDR_RELOAD_H, IDR_RELOAD_P,
-  IDR_HOME, IDR_HOME_H, IDR_HOME_P,
-  IDR_STAR, IDR_STAR_NOBORDER, IDR_STAR_NOBORDER_CENTER, IDR_STAR_D, IDR_STAR_H,
-  IDR_STAR_P,
-  IDR_STARRED, IDR_STARRED_NOBORDER, IDR_STARRED_NOBORDER_CENTER, IDR_STARRED_H,
-  IDR_STARRED_P,
-  IDR_GO, IDR_GO_NOBORDER, IDR_GO_NOBORDER_CENTER, IDR_GO_H, IDR_GO_P,
-  IDR_STOP, IDR_STOP_NOBORDER, IDR_STOP_NOBORDER_CENTER, IDR_STOP_H, IDR_STOP_P,
-  IDR_MENU_BOOKMARK,
-  IDR_MENU_PAGE, IDR_MENU_PAGE_RTL,
-  IDR_MENU_CHROME, IDR_MENU_CHROME_RTL,
-  IDR_MENU_DROPARROW,
-  IDR_THROBBER, IDR_THROBBER_WAITING, IDR_THROBBER_LIGHT,
-  IDR_LOCATIONBG
-};
-
 // Returns a piece of memory with the contents of the file |path|.
 RefCountedMemory* ReadFileData(const FilePath& path) {
   if (!path.empty()) {
@@ -199,7 +179,7 @@ RefCountedMemory* ReadFileData(const FilePath& path) {
 }
 
 // Does error checking for invalid incoming data while trying to read an
-// floaing point value.
+// floating point value.
 bool ValidRealValue(ListValue* tint_list, int index, double* out) {
   if (tint_list->GetReal(index, out))
     return true;
@@ -258,7 +238,7 @@ BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
   pack->GenerateTabBackgroundImages(&pack->image_cache_);
 
   // Repack all the images from |image_cache_| into |image_memory_| for
-  // writing to the data pack
+  // writing to the data pack.
   pack->RepackImageCacheToImageMemory();
 
   // The BrowserThemePack is now in a consistent state.
@@ -777,10 +757,12 @@ void BrowserThemePack::GenerateTintedButtons(
     ImageCache* processed_bitmaps) const {
   if (button_tint.h != -1 || button_tint.s != -1 || button_tint.l != -1) {
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-    for (size_t i = 0; i < arraysize(kToolbarButtonIDs); ++i) {
-      scoped_ptr<SkBitmap> button(
-          new SkBitmap(*rb.GetBitmapNamed(kToolbarButtonIDs[i])));
-      (*processed_bitmaps)[kToolbarButtonIDs[i]] = new SkBitmap(
+    const std::set<int>& ids =
+        BrowserThemeProvider::GetTintableToolbarButtons();
+    for (std::set<int>::const_iterator it = ids.begin(); it != ids.end();
+         ++it) {
+      scoped_ptr<SkBitmap> button(new SkBitmap(*rb.GetBitmapNamed(*it)));
+      (*processed_bitmaps)[*it] = new SkBitmap(
           SkBitmapOperations::CreateHSLShiftedBitmap(*button, button_tint));
     }
   }
diff --git a/chrome/browser/browser_theme_pack.h b/chrome/browser/browser_theme_pack.h
index 4287617..9b30872 100644
--- a/chrome/browser/browser_theme_pack.h
+++ b/chrome/browser/browser_theme_pack.h
@@ -41,9 +41,9 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
   // on a separate thread as it takes so long.
   static BrowserThemePack* BuildFromExtension(Extension* extension);
 
-  // Builds the theme pack from a previously WriteToDisk(). This operation
-  // should be relatively fast, as it should be an mmap() and some pointer
-  // swizzling. Returns NULL on any error attempting to read |path|.
+  // Builds the theme pack from a previously performed WriteToDisk(). This
+  // operation should be relatively fast, as it should be an mmap() and some
+  // pointer swizzling. Returns NULL on any error attempting to read |path|.
   static scoped_refptr<BrowserThemePack> BuildFromDataPack(
       FilePath path, const std::string& expected_id);
 
diff --git a/chrome/browser/browser_theme_pack_unittest.cc b/chrome/browser/browser_theme_pack_unittest.cc
index 1c9081c..b8e5425 100644
--- a/chrome/browser/browser_theme_pack_unittest.cc
+++ b/chrome/browser/browser_theme_pack_unittest.cc
@@ -174,7 +174,7 @@ TEST_F(BrowserThemePackTest, DeriveUnderlineLinkColor) {
 }
 
 TEST_F(BrowserThemePackTest, ProvideUnderlineLinkColor) {
-  // If we specify the underline color, it shouldn't try to generate one.x
+  // If we specify the underline color, it shouldn't try to generate one.
   std::string color_json = "{ \"ntp_link\": [128, 128, 128],"
                            "  \"ntp_link_underline\": [255, 255, 255],"
                            "  \"ntp_section_link\": [128, 128, 128],"
diff --git a/chrome/browser/browser_theme_provider.cc b/chrome/browser/browser_theme_provider.cc
index daf7b68..3887c2f 100644
--- a/chrome/browser/browser_theme_provider.cc
+++ b/chrome/browser/browser_theme_provider.cc
@@ -126,14 +126,34 @@ const int kThemeableImages[] = {
 };
 
 bool HasThemeableImage(int themeable_image_id) {
-  static std::map<int, bool> themeable_images;
+  static std::set<int> themeable_images;
   if (themeable_images.empty()) {
-    for (size_t i = 0; i < arraysize(kThemeableImages); ++i)
-      themeable_images[kThemeableImages[i]] = true;
+    themeable_images.insert(
+        kThemeableImages, kThemeableImages + arraysize(kThemeableImages));
   }
   return themeable_images.count(themeable_image_id) > 0;
 }
 
+// The image resources that will be tinted by the 'button' tint value.
+const int kToolbarButtonIDs[] = {
+  IDR_BACK, IDR_BACK_D, IDR_BACK_H, IDR_BACK_P,
+  IDR_FORWARD, IDR_FORWARD_D, IDR_FORWARD_H, IDR_FORWARD_P,
+  IDR_RELOAD, IDR_RELOAD_H, IDR_RELOAD_P,
+  IDR_HOME, IDR_HOME_H, IDR_HOME_P,
+  IDR_STAR, IDR_STAR_NOBORDER, IDR_STAR_NOBORDER_CENTER, IDR_STAR_D, IDR_STAR_H,
+  IDR_STAR_P,
+  IDR_STARRED, IDR_STARRED_NOBORDER, IDR_STARRED_NOBORDER_CENTER, IDR_STARRED_H,
+  IDR_STARRED_P,
+  IDR_GO, IDR_GO_NOBORDER, IDR_GO_NOBORDER_CENTER, IDR_GO_H, IDR_GO_P,
+  IDR_STOP, IDR_STOP_NOBORDER, IDR_STOP_NOBORDER_CENTER, IDR_STOP_H, IDR_STOP_P,
+  IDR_MENU_BOOKMARK,
+  IDR_MENU_PAGE, IDR_MENU_PAGE_RTL,
+  IDR_MENU_CHROME, IDR_MENU_CHROME_RTL,
+  IDR_MENU_DROPARROW,
+  IDR_THROBBER, IDR_THROBBER_WAITING, IDR_THROBBER_LIGHT,
+  IDR_LOCATIONBG
+};
+
 // Writes the theme pack to disk on a separate thread.
 class WritePackToDiskTask : public Task {
  public:
@@ -446,6 +466,18 @@ bool BrowserThemeProvider::GetDefaultDisplayProperty(int id, int* result) {
   return false;
 }
 
+// static
+const std::set<int>& BrowserThemeProvider::GetTintableToolbarButtons() {
+  static std::set<int> button_set;
+  if (button_set.empty()) {
+    button_set = std::set<int>(
+        kToolbarButtonIDs,
+        kToolbarButtonIDs + arraysize(kToolbarButtonIDs));
+  }
+
+  return button_set;
+}
+
 color_utils::HSL BrowserThemeProvider::GetTint(int id) const {
   DCHECK(CalledOnValidThread());
 
diff --git a/chrome/browser/browser_theme_provider.h b/chrome/browser/browser_theme_provider.h
index 62736b4..e6275d6 100644
--- a/chrome/browser/browser_theme_provider.h
+++ b/chrome/browser/browser_theme_provider.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_BROWSER_THEME_PROVIDER_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -153,6 +154,9 @@ class BrowserThemeProvider : public NonThreadSafe,
   // is valid.
   static bool GetDefaultDisplayProperty(int id, int* result);
 
+  // Returns the set of IDR_* resources that should be tinted.
+  static const std::set<int>& GetTintableToolbarButtons();
+
   // Save the images to be written to disk, mapping file path to id.
   typedef std::map<FilePath, int> ImagesDiskCache;
 
diff --git a/chrome/browser/gtk/gtk_theme_provider.cc b/chrome/browser/gtk/gtk_theme_provider.cc
index 9330f30..7881bae 100644
--- a/chrome/browser/gtk/gtk_theme_provider.cc
+++ b/chrome/browser/gtk/gtk_theme_provider.cc
@@ -71,12 +71,16 @@ const int kThemeImages[] = {
 };
 
 bool IsOverridableImage(int id) {
-  for (size_t i = 0; i < arraysize(kThemeImages); ++i) {
-    if (kThemeImages[i] == id)
-      return true;
+  static std::set<int> images;
+  if (images.empty()) {
+    images.insert(kThemeImages, kThemeImages + arraysize(kThemeImages));
+
+    const std::set<int>& buttons =
+        BrowserThemeProvider::GetTintableToolbarButtons();
+    images.insert(buttons.begin(), buttons.end());
   }
 
-  return false;
+  return images.count(id) > 0;
 }
 
 }  // namespace
@@ -619,10 +623,17 @@ SkBitmap* GtkThemeProvider::GenerateGtkThemeBitmap(int id) const {
       return GenerateFrameImage(
           BrowserThemeProvider::TINT_FRAME_INCOGNITO_INACTIVE);
     }
+    default: {
+      // This is a tinted button. Tint it and return it.
+      ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+      scoped_ptr<SkBitmap> button(new SkBitmap(*rb.GetBitmapNamed(id)));
+      TintMap::const_iterator it = tints_.find(
+          BrowserThemeProvider::TINT_BUTTONS);
+      DCHECK(it != tints_.end());
+      return new SkBitmap(SkBitmapOperations::CreateHSLShiftedBitmap(
+          *button, it->second));
+    }
   }
-
-  NOTREACHED() << "Invalid bitmap request " << id;
-  return NULL;
 }
 
 SkBitmap* GtkThemeProvider::GenerateFrameImage(int tint_id) const {