Integrate new MD update menu icons.

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

Index: chrome/browser/ui/views/toolbar/app_menu_button.cc
diff --git a/chrome/browser/ui/views/toolbar/app_menu_button.cc b/chrome/browser/ui/views/toolbar/app_menu_button.cc
index ca12f2f1d1dd190b9bbe03d43558d92d344fc758..33e7a8fdef34a627863b13f7bdd310e953eed6b6 100644
--- a/chrome/browser/ui/views/toolbar/app_menu_button.cc
+++ b/chrome/browser/ui/views/toolbar/app_menu_button.cc
@@ -134,6 +134,7 @@ void AppMenuButton::ScheduleAppMenuIconPaint() {
 
 void AppMenuButton::UpdateIcon() {
   DCHECK(ui::MaterialDesignController::IsModeMaterial());
+  gfx::VectorIconId icon_id = gfx::VectorIconId::BROWSER_TOOLS;
   SkColor color = gfx::kPlaceholderColor;
   switch (severity_) {
     case AppMenuIconPainter::SEVERITY_NONE:
@@ -142,17 +143,19 @@ void AppMenuButton::UpdateIcon() {
       break;
     case AppMenuIconPainter::SEVERITY_LOW:
       color = gfx::kGoogleGreen700;
+      icon_id = gfx::VectorIconId::UPDATE_MENU_SEVERITY_LOW;
       break;
     case AppMenuIconPainter::SEVERITY_MEDIUM:
       color = gfx::kGoogleYellow700;
+      icon_id = gfx::VectorIconId::UPDATE_MENU_SEVERITY_MEDIUM;
       break;
     case AppMenuIconPainter::SEVERITY_HIGH:
       color = gfx::kGoogleRed700;
+      icon_id = gfx::VectorIconId::UPDATE_MENU_SEVERITY_HIGH;
       break;
   }

[kylix_rd, 2016/05/10 14:20:10]

TODO: Determine other state information to select "update failed", "new
extension" & "severity warning" icons.

Is this the right/wrong place for this?

[Evan Stade, 2016/05/10 16:48:14]

You're assuming the menu is changing color due to an update when that isn't
necessarily the case. See AppMenuBadgeController::BadgeType

I don't think fixing this should be a TODO because showing an upgrade arrow for
a global error strikes me as a regression.

[kylix_rd, 2016/05/10 17:49:01]

The AppMenuBadgeController doesn't update the BadgeType on the AppMenuButton
instance; only the severity is updated. Should this information now be plumbed
through to this class?
It's only a TODO in term of this review, not in terms of the overall patch.
Subsequent patches should resolve these issues.

[Evan Stade, 2016/05/10 18:04:13]

I suppose it must, or the logic of mapping state to icon should move out of this
class. I would actually consider merging the two enums because it looks like
GLOBAL_ERROR and INCOMPATIBILITY_WARNING imply a certain severity (medium) so
these aren't really tangential concepts any more. All of this is made more
difficult by the need to continue supporting pre-md.

Also I think we should worry about how this looks in incognito. You might want
slightly different numerical values there (e.g. kGoogleRed300 or something).
sg

[kylix_rd, 2016/05/10 18:10:51]

I've arrived at the same conclusion. A subsequent patch is forthcoming that
should accomplish this. Right now, I'm keeping the enumerations separate for, as
you said, simplicity in supporting pre-md.
 
Is Incognito mode available as a global state?
 

[Evan Stade, 2016/05/10 18:48:39]

no. First step would be to see if designers actually do want a different color
for incognito. Second step would probably involve putting the colors in
NativeTheme.

[kylix_rd, 2016/05/10 19:17:02]

Ah, so the question was more rhetorical and speculative than an actual decision
or direction to follow.

Do we want to do this now or drop in a TODO and assume this patch is
good-enough?

 
-  SetImage(views::Button::STATE_NORMAL,
-           gfx::CreateVectorIcon(gfx::VectorIconId::BROWSER_TOOLS, color));
+  SetImage(views::Button::STATE_NORMAL, gfx::CreateVectorIcon(icon_id, color));

[kylix_rd, 2016/05/10 16:23:47]

In seems that calling this gfx::CreateVectorIcon() overload doesn't scale the
icon down the the correct size as shown here:
https://drive.google.com/a/google.com/file/d/0B6LQg8CWo5bNOGFKZ0g5OVVWVkU/vie...

However, I'm reluctant to call this overload:

GFX_EXPORT ImageSkia CreateVectorIcon(VectorIconId id,
                                      size_t dip_size,
                                      SkColor color);

with a hard-coded value. Is there a constant or some other way to know what to
pass in as the dip_size?

[Evan Stade, 2016/05/10 16:48:14]

hardcode a value (16)

[kylix_rd, 2016/05/10 17:49:01]

Done.

 }
 
 void AppMenuButton::SetTrailingMargin(int margin) {