GTK: Fix highlight and image colors in the new wrench menu.

GTK: Fix highlight and image colors in the new wrench menu.

- The states weren't being set properly to replicate the menu item
  highlight effect. The button background needs to be STATE_SELECTED
  while the actual widget needs PRELIGHT. This removes a bunch of
  crud.
- The fullscreen button needs to be tinted multiple times to the
  different label states. Now the fullscreen icon is the correct
  label color. Required adding infrastructure for GtkIconSets.

BUG=45757
TEST=All labels and images in buttons should match the normal and
prelight states of the labels in normal menu items. Make sure to
try themes that have different normal and prelight states (DarkLooks)
and themes that have the same color (Ambiance).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51805

[Elliot Glaysher, 2010-07-07 22:32:32]

Only thing left before turning this on by default is making the buttons the same
size.

[Evan Martin, 2010-07-07 22:41:25]

LGTM

http://codereview.chromium.org/2864044/diff/4001/5003
File chrome/browser/gtk/gtk_custom_menu_item.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5003#newcode28
chrome/browser/gtk/gtk_custom_menu_item.cc:28: image ? image :
Can't you just always use get_child?  It'll get the image or the label from the
button, no?

http://codereview.chromium.org/2864044/diff/4001/5003#newcode29
chrome/browser/gtk/gtk_custom_menu_item.cc:29:
gtk_bin_get_child(GTK_BIN(item->currently_selected_button)),
might be a bit clearer if you indent this line a bit more (same with same
pattern below)

http://codereview.chromium.org/2864044/diff/4001/5004
File chrome/browser/gtk/gtk_theme_provider.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5004#newcode795
chrome/browser/gtk/gtk_theme_provider.cc:795:
icon_sets_[IDR_FULLSCREEN_MENU_BUTTON] = icon_set;
This is the only icon in the menus?  Why use a set<> then?

[Elliot Glaysher, 2010-07-07 22:51:08]

http://codereview.chromium.org/2864044/diff/4001/5003
File chrome/browser/gtk/gtk_custom_menu_item.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5003#newcode28
chrome/browser/gtk/gtk_custom_menu_item.cc:28: image ? image :
On 2010/07/07 22:41:26, Evan Martin wrote:
> Can't you just always use get_child?  It'll get the image or the label from
the
> button, no?

No, get_child doesn't return the GtkImage in those cases (it places it in a
vbox+hbox). But you are correct that this isn't needed; setting it on the parent
works fine.

http://codereview.chromium.org/2864044/diff/4001/5004
File chrome/browser/gtk/gtk_theme_provider.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5004#newcode795
chrome/browser/gtk/gtk_theme_provider.cc:795:
icon_sets_[IDR_FULLSCREEN_MENU_BUTTON] = icon_set;
On 2010/07/07 22:41:26, Evan Martin wrote:
> This is the only icon in the menus?  Why use a set<> then?

Wanted to be ready for more later, since I assume we'll be shoving more crazy
crap in menus. Simplified to a single member pointer for now.

[Evan Stade, 2010-07-07 22:56:00]

http://codereview.chromium.org/2864044/diff/4001/5006
File chrome/browser/gtk/menu_gtk.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5006#newcode124
chrome/browser/gtk/menu_gtk.cc:124: reinterpret_cast<void*>(menu_gtk_delegate));
is this case necessary?

http://codereview.chromium.org/2864044/diff/4001/5009
File gfx/skbitmap_operations.h (right):

http://codereview.chromium.org/2864044/diff/4001/5009#newcode68
gfx/skbitmap_operations.h:68: static SkBitmap CreateColorReplacedBitmap(const
SkBitmap& bitmap,
can you use CreateMaskedBitmap(solidcolorbitmap, alphabitmap) instead of
creating this new function?

[Elliot Glaysher, 2010-07-07 23:24:55]

http://codereview.chromium.org/2864044/diff/4001/5006
File chrome/browser/gtk/menu_gtk.cc (right):

http://codereview.chromium.org/2864044/diff/4001/5006#newcode124
chrome/browser/gtk/menu_gtk.cc:124: reinterpret_cast<void*>(menu_gtk_delegate));
On 2010/07/07 22:56:00, Evan Stade wrote:
> is this case necessary?

Was copying from other code. It's not, and I've removed it from several places
in this file.

http://codereview.chromium.org/2864044/diff/4001/5009
File gfx/skbitmap_operations.h (right):

http://codereview.chromium.org/2864044/diff/4001/5009#newcode68
gfx/skbitmap_operations.h:68: static SkBitmap CreateColorReplacedBitmap(const
SkBitmap& bitmap,
On 2010/07/07 22:56:00, Evan Stade wrote:
> can you use CreateMaskedBitmap(solidcolorbitmap, alphabitmap) instead of
> creating this new function?

Done.

[Evan Stade, 2010-07-07 23:33:50]

lgtm