Update search chip icon for MD, vectorize.

Update search chip icon for MD, vectorize.

Also fixes the icons for the omnibox search extension API in MD mode.

BUG=505953
TBR=sadrul@chromium.org

Committed: https://crrev.com/81a623407a261c91e90b24266c12fdc8cf17c10b
Cr-Commit-Position: refs/heads/master@{#356409}

[Evan Stade, 2015-10-23 21:49:31]

Description was changed from

==========
Update search chip icon for MD, vectorize.

BUG=505953
==========

to

==========
Update search chip icon for MD, vectorize.

Also fixes the icons for the omnibox search extension API in MD mode.

BUG=505953
==========

[Evan Stade, 2015-10-23 21:49:31]

estade@chromium.org changed reviewers:
+ pkasting@chromium.org

[Evan Stade, 2015-10-23 21:49:40]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (left):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:24: :
IconLabelBubbleView(IDR_KEYWORD_SEARCH_MAGNIFIER,
this was actually never visible because it got overridden in
location_bar_view.cc

[Evan Stade, 2015-10-23 21:49:44]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-23 21:50:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421683002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421683002/20001

[commit-bot: I haz the power, 2015-10-23 23:40:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-23 23:40:08]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2015-10-23 23:44:09]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:107: bool
is_extension_icon = ui::MaterialDesignController::IsModeMaterial() ?
Nit: Seems clearer?

  bool is_extension_icon =
      is_extension_icon_ && !ui::MaterialDesignController::IsModeMaterial();

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:108: false :
is_extension_icon_;
Nit: Seems clearer?

  bool is_extension_icon =
      is_extension_icon_ && !ui::MaterialDesignController::IsModeMaterial();

Although since this is the only place this variable is handled, maybe instead we
should put the conditional in set_extension_icon(), since that way when we
switch to full-time MD, it will be obvious we don't need the member anymore?

Also since the only way we use this is by negating it below, maybe we should
name it something else like is_default_icon or icon_needs_padding or something?

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:24: #include
"ui/views/controls/image_view.h"
Are all these needed?  It kinda surprises me that we need image_view.h, for
example.

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
Huh.  Are we sure we want the magnifier to be the system "enabled link" color? 
That seems odd considering this isn't a link or anything.

I would think we'd want some starting color (blue or white or something) run
through GetReadableColor() over the background here, or the like.

https://codereview.chromium.org/1421683002/diff/20001/ui/gfx/vector_icons/sea...
File ui/gfx/vector_icons/search_chip_icon.icon (right):

https://codereview.chromium.org/1421683002/diff/20001/ui/gfx/vector_icons/sea...
ui/gfx/vector_icons/search_chip_icon.icon:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
Seems sort of weird to have "icon" in the name of this icon file in addition to
the ".icon" on the end.

How about just "keyword_search.icon"?  (That's marginally clearer to me than
"search_chip.icon", YMMV)

[Evan Stade, 2015-10-26 19:29:19]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:108: false :
is_extension_icon_;
On 2015/10/23 23:44:09, Peter Kasting wrote:
> Nit: Seems clearer?
> 
>   bool is_extension_icon =
>       is_extension_icon_ && !ui::MaterialDesignController::IsModeMaterial();

done

> 
> Although since this is the only place this variable is handled, maybe instead
we
> should put the conditional in set_extension_icon(), since that way when we
> switch to full-time MD, it will be obvious we don't need the member anymore?

not done because it feels wrong to say "this extension icon is not an extension
icon because it's MD"

> 
> Also since the only way we use this is by negating it below, maybe we should
> name it something else like is_default_icon or icon_needs_padding or
something?

done

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:24: #include
"ui/views/controls/image_view.h"
On 2015/10/23 23:44:09, Peter Kasting wrote:
> Are all these needed?  It kinda surprises me that we need image_view.h, for
> example.

a couple were not necessary

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/23 23:44:09, Peter Kasting wrote:
> Huh.  Are we sure we want the magnifier to be the system "enabled link" color?

> That seems odd considering this isn't a link or anything.
> 
> I would think we'd want some starting color (blue or white or something) run
> through GetReadableColor() over the background here, or the like.

The magnifier is set in a chip and the border of the chip is also the link color
(at least, it's the same blue as we're supposed to use for MD links).
Presumably, the link color should always be readable relative to a text entry bg
color? Sebastien did say it was intentionally the same blue as links, and not
the same blue as the "call to action" color used for active omnibox icons.

https://codereview.chromium.org/1421683002/diff/20001/ui/gfx/vector_icons/sea...
File ui/gfx/vector_icons/search_chip_icon.icon (right):

https://codereview.chromium.org/1421683002/diff/20001/ui/gfx/vector_icons/sea...
ui/gfx/vector_icons/search_chip_icon.icon:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/10/23 23:44:09, Peter Kasting wrote:
> Seems sort of weird to have "icon" in the name of this icon file in addition
to
> the ".icon" on the end.
> 
> How about just "keyword_search.icon"?  (That's marginally clearer to me than
> "search_chip.icon", YMMV)

Done.

[Peter Kasting, 2015-10-26 19:57:41]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/26 19:29:19, Evan Stade wrote:
> On 2015/10/23 23:44:09, Peter Kasting wrote:
> > Huh.  Are we sure we want the magnifier to be the system "enabled link"
color?
> 
> > That seems odd considering this isn't a link or anything.
> > 
> > I would think we'd want some starting color (blue or white or something) run
> > through GetReadableColor() over the background here, or the like.
> 
> The magnifier is set in a chip and the border of the chip is also the link
color
> (at least, it's the same blue as we're supposed to use for MD links).

GetSystemColor() probably isn't right, then, since it's supposed to mean "get
the native OS color for this" rather than just "get the color for this" (e.g. if
we hardcoded links to a particular blue in MD we would want to basically ignore
the system colors, rather than have the system color accessors return that
blue).  We'd want to make something like ThemeProvider::GetColor() return the
relevant blue here instead.

> Presumably, the link color should always be readable relative to a text entry
bg
> color?

What background color does this chip have?

If we were using system link and window BG colors, I might assume readability,
but if we're hardcoding a link color it's likely we'll need to force
readability.  But I'd know better if I knew what background this has.

[Evan Stade, 2015-10-26 20:19:36]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/26 19:57:41, Peter Kasting wrote:
> On 2015/10/26 19:29:19, Evan Stade wrote:
> > On 2015/10/23 23:44:09, Peter Kasting wrote:
> > > Huh.  Are we sure we want the magnifier to be the system "enabled link"
> color?
> > 
> > > That seems odd considering this isn't a link or anything.
> > > 
> > > I would think we'd want some starting color (blue or white or something)
run
> > > through GetReadableColor() over the background here, or the like.
> > 
> > The magnifier is set in a chip and the border of the chip is also the link
> color
> > (at least, it's the same blue as we're supposed to use for MD links).
> 
> GetSystemColor() probably isn't right, then, since it's supposed to mean "get
> the native OS color for this" rather than just "get the color for this" (e.g.
if
> we hardcoded links to a particular blue in MD we would want to basically
ignore
> the system colors, rather than have the system color accessors return that
> blue).  We'd want to make something like ThemeProvider::GetColor() return the
> relevant blue here instead.

NativeTheme does not necessarily draw from system colors except where it makes
sense to. ThemeProvider::GetColor only handles browser-specific colors (which
links are not). Link colors have to live in NativeTheme because stuff like
views::Link or other UI outside browser (maybe cros system UI?) needs access to
them. For Windows, I don't know or particularly care if we will choose MD link
colors or system link colors for links, but if we do hardcode the link color we
should at least hardcode it to something that looks good on the theme's bg color
(e.g. by returning a different color for links in inverted color schemes).
> 
> > Presumably, the link color should always be readable relative to a text
entry
> bg
> > color?
> 
> What background color does this chip have?
> 
> If we were using system link and window BG colors, I might assume readability,
> but if we're hardcoding a link color it's likely we'll need to force
> readability.  But I'd know better if I knew what background this has.

Same as link color with greatly reduced opacity (5%). So essentially, the
omnibox bg color.

[Peter Kasting, 2015-10-26 20:34:01]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/26 20:19:36, Evan Stade wrote:
> On 2015/10/26 19:57:41, Peter Kasting wrote:
> > On 2015/10/26 19:29:19, Evan Stade wrote:
> > > On 2015/10/23 23:44:09, Peter Kasting wrote:
> > > > Huh.  Are we sure we want the magnifier to be the system "enabled link"
> > color?
> > > 
> > > > That seems odd considering this isn't a link or anything.
> > > > 
> > > > I would think we'd want some starting color (blue or white or something)
> run
> > > > through GetReadableColor() over the background here, or the like.
> > > 
> > > The magnifier is set in a chip and the border of the chip is also the link
> > color
> > > (at least, it's the same blue as we're supposed to use for MD links).
> > 
> > GetSystemColor() probably isn't right, then, since it's supposed to mean
"get
> > the native OS color for this" rather than just "get the color for this"
(e.g.
> if
> > we hardcoded links to a particular blue in MD we would want to basically
> ignore
> > the system colors, rather than have the system color accessors return that
> > blue).  We'd want to make something like ThemeProvider::GetColor() return
the
> > relevant blue here instead.
> 
> NativeTheme does not necessarily draw from system colors except where it makes
> sense to.

At least on Windows, where there is a concept of an OS link color,
NativeTheme::GetSystemColor() should definitely return that.

> ThemeProvider::GetColor only handles browser-specific colors (which
> links are not). Link colors have to live in NativeTheme because stuff like
> views::Link or other UI outside browser (maybe cros system UI?) needs access
to
> them.

To me the MD colors are browser-specific.  If we were to write non-browser code
that had UI with links, it should probably be using the system link color,
whereas the browser should use the MD colors.

But if you don't want that, then we need some sort of layer that can be accessed
outside the browser that isn't NativeTheme.  NativeTheme should always be
returning the OS native colors when they exist, and should only hardcode other
colors in cases where there's no OS color.

Luckily, ThemeProvider is a concept that lives in ui/base/, and the ui/
implementation of it is DefaultThemeProvider.  So you could put your color there
and have ui/-level stuff call Widget::GetThemeProvider().  For widgets living in
the browser world this would return the browser-scope theme provider, and for
things living outside that it would return the DefaultThemeProvider.  Then I
think everything works automatically.

> For Windows, I don't know or particularly care if we will choose MD link
> colors or system link colors for links, but if we do hardcode the link color
we
> should at least hardcode it to something that looks good on the theme's bg
color
> (e.g. by returning a different color for links in inverted color schemes).

Yes; generally we do this by running things through GetReadableColor() at a low
level (inside the ThemeProvider or similar).

> > > Presumably, the link color should always be readable relative to a text
> entry
> > bg
> > > color?
> > 
> > What background color does this chip have?
> > 
> > If we were using system link and window BG colors, I might assume
readability,
> > but if we're hardcoding a link color it's likely we'll need to force
> > readability.  But I'd know better if I knew what background this has.
> 
> Same as link color with greatly reduced opacity (5%). So essentially, the
> omnibox bg color.

OK; as long as we ensure _that_ color follows the theme/system BG color, then if
we make the link color getter readable over that color at a low level, we should
be good to avoid doing another similar check here.

[Evan Stade, 2015-10-26 21:29:42]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/26 20:34:01, Peter Kasting wrote:
> On 2015/10/26 20:19:36, Evan Stade wrote:
> > On 2015/10/26 19:57:41, Peter Kasting wrote:
> > > On 2015/10/26 19:29:19, Evan Stade wrote:
> > > > On 2015/10/23 23:44:09, Peter Kasting wrote:
> > > > > Huh.  Are we sure we want the magnifier to be the system "enabled
link"
> > > color?
> > > > 
> > > > > That seems odd considering this isn't a link or anything.
> > > > > 
> > > > > I would think we'd want some starting color (blue or white or
something)
> > run
> > > > > through GetReadableColor() over the background here, or the like.
> > > > 
> > > > The magnifier is set in a chip and the border of the chip is also the
link
> > > color
> > > > (at least, it's the same blue as we're supposed to use for MD links).
> > > 
> > > GetSystemColor() probably isn't right, then, since it's supposed to mean
> "get
> > > the native OS color for this" rather than just "get the color for this"
> (e.g.
> > if
> > > we hardcoded links to a particular blue in MD we would want to basically
> > ignore
> > > the system colors, rather than have the system color accessors return that
> > > blue).  We'd want to make something like ThemeProvider::GetColor() return
> the
> > > relevant blue here instead.
> > 
> > NativeTheme does not necessarily draw from system colors except where it
makes
> > sense to.
> 
> At least on Windows, where there is a concept of an OS link color,
> NativeTheme::GetSystemColor() should definitely return that.
> 
> > ThemeProvider::GetColor only handles browser-specific colors (which
> > links are not). Link colors have to live in NativeTheme because stuff like
> > views::Link or other UI outside browser (maybe cros system UI?) needs access
> to
> > them.
> 
> To me the MD colors are browser-specific.  If we were to write non-browser
code
> that had UI with links, it should probably be using the system link color,
> whereas the browser should use the MD colors.
> 
> But if you don't want that, then we need some sort of layer that can be
accessed
> outside the browser that isn't NativeTheme.  NativeTheme should always be
> returning the OS native colors when they exist, and should only hardcode other
> colors in cases where there's no OS color
> Luckily, ThemeProvider is a concept that lives in ui/base/, and the ui/
> implementation of it is DefaultThemeProvider.  So you could put your color
there
> and have ui/-level stuff call Widget::GetThemeProvider().  For widgets living
in
> the browser world this would return the browser-scope theme provider, and for
> things living outside that it would return the DefaultThemeProvider.  Then I
> think everything works automatically.

Perhaps that was the original intent, but right now that simply isn't how
NativeTheme is used. For example, on Linux we use either NativeThemeAura or
NativeThemeGtk but only the latter actually cares about system colors. If we
move this to ThemeProvider, Linux has no way of making the link color match the
system link color. DefaultThemeProvider's concrete methods are all no-ops so I
don't think there's precedent for putting any kind of real implementation there.

This discussion is starting to reach outside the scope of this CL. If you would
like to campaign to change how ThemeProvider and NativeTheme are used, that
discussion is probably better off on a broader email list. I think we have
agreed that this icon's color should match the link color and currently this is
how you get the link color used throughout Chrome, so in isolation I believe
this CL to be correct.

> 
> > For Windows, I don't know or particularly care if we will choose MD link
> > colors or system link colors for links, but if we do hardcode the link color
> we
> > should at least hardcode it to something that looks good on the theme's bg
> color
> > (e.g. by returning a different color for links in inverted color schemes).
> 
> Yes; generally we do this by running things through GetReadableColor() at a
low
> level (inside the ThemeProvider or similar).

In my experience that doesn't always produce a satisfying color so it makes more
sense to me to handpick an inverted link color. But sure, we can take care of
this within whatever class is providing link colors.

> 
> > > > Presumably, the link color should always be readable relative to a text
> > entry
> > > bg
> > > > color?
> > > 
> > > What background color does this chip have?
> > > 
> > > If we were using system link and window BG colors, I might assume
> readability,
> > > but if we're hardcoding a link color it's likely we'll need to force
> > > readability.  But I'd know better if I knew what background this has.
> > 
> > Same as link color with greatly reduced opacity (5%). So essentially, the
> > omnibox bg color.
> 
> OK; as long as we ensure _that_ color follows the theme/system BG color, then
if
> we make the link color getter readable over that color at a low level, we
should
> be good to avoid doing another similar check here.

I believe the omnibox bg color does follow the theme/system. At least it does on
linux.

[Peter Kasting, 2015-10-26 22:21:19]

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/selected_keyword_view.cc (right):

https://codereview.chromium.org/1421683002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/selected_keyword_view.cc:60:
GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
On 2015/10/26 21:29:42, Evan Stade wrote:
> For example, on Linux we use either NativeThemeAura or
> NativeThemeGtk but only the latter actually cares about system colors.

I haven't studied Linux in detail, but last time I looked (when Dave implemented
AiS) there were a lot of things about its theming code that seemed broken.  I
would base your assumptions around whatever Windows does instead, which is also
not guaranteed to be correct, but is at least less likely to be incorrect.

> If we
> move this to ThemeProvider, Linux has no way of making the link color match
the
> system link color.

Why?  Why can't the ThemeProvider ask the NativeThemeProvider as needed?

> DefaultThemeProvider's concrete methods are all no-ops so I
> don't think there's precedent for putting any kind of real implementation
there.

Until recently the only colors we've needed outside chrome/ have been ones where
we can either use the system color, or there isn't a system color (or we choose
to ignore it entirely) and we can use a hardcoded color.  So there hasn't been a
need for any kind of above-chrome/-but-not-native-theme functionality.

The folks who started implementing MD initially started just putting overrides
into the NativeTheme code to return other colors, which wasn't how we should
have implemented this (but they didn't know and didn't ask, and also haven't yet
had any consideration for how system or custom themes interact with MD so as to
have encountered the reasons why this was the wrong implementation).  So they
didn't blaze a trail for you here, though they should have.

Anyway, it's not necessarily the case that you need to touch
DefaultThemeProvider.  That's only necessary if you want MD colors used in UI
objects that aren't used from chrome/ code.  As long as things are used from
chrome/ code, asking the widget for its theme provider should get you back the
non-native theme provider, which you can override to return the MD values.  Then
we can have any code that wants to respect those values ensure it's going that
route, and save the underlying NativeThemeProvider stuff for things that want to
be system-native regardless.

Or, if we want to change the meaning of what NativeThemeProvider is, so that
it's no longer related to "native" and is just the "default behavior" or
something, then we could probably combine a bunch of these classes, so all the
color selection code lived in one place in ui/.  It's not really obvious to me
why we need three or four classes here where one would do.  That would
_definitely_ be outside the scope of this CL, though.

> This discussion is starting to reach outside the scope of this CL. If you
would
> like to campaign to change how ThemeProvider and NativeTheme are used, that
> discussion is probably better off on a broader email list.

I don't think this is really a change in how most code uses them.  Various parts
of Chrome's UI (like omnibox-related stuff) try to respect system colors (or
used to as of when they were implemented) and assume the
NativeTheme::GetSystemColor() machinery provides them.  Perhaps on GTK that
assumption is wrong, leading to non-system-respecting behavior of parts of this
UI, but that seems like a bug.  It seems like what you want is to change such
code to not be asking for the system color directly, but instead to just ask the
theme machinery for the right color and let it do fallback as appropriate.  I
don't think such a change is outside the scope of this CL -- that seems like the
whole point of this CL?

You say "campaign" as if there are lots of skeptical stakeholders to win over
here.  Who else would you want involved in the discussion?  At this point, I
think I'm the closest thing the theme color code has to an owner, but I'm not
opposed to getting more input if there are other good people.

> I think we have
> agreed that this icon's color should match the link color and currently this
is
> how you get the link color used throughout Chrome, so in isolation I believe
> this CL to be correct.

Until now everyone has wanted the system-native link color, so it's no surprise
that code explicitly calls the NativeThemeProvider.  As I said above, it seems
like you want a different behavior than that.

[Peter Kasting, 2015-10-26 23:02:12]

BTW, if it turns out that what we want in the end would be a significant
departure from/imposition on this CL, I'm totally OK with doing it in stages.  I
just want to figure out what the right end state actually is first.

[Evan Stade, 2015-10-27 00:11:50]

> Until now everyone has wanted the system-native link color, so it's no
surprise
> that code explicitly calls the NativeThemeProvider.  As I said above, it seems
> like you want a different behavior than that.

Then I must have phrased something poorly to give you a false impression. All I
said was that the color of this icon should match the color of links. What the
color of links actually is a different matter.

[Peter Kasting, 2015-10-27 01:06:34]

On 2015/10/27 00:11:50, Evan Stade wrote:
> > Until now everyone has wanted the system-native link color, so it's no
> surprise
> > that code explicitly calls the NativeThemeProvider.  As I said above, it
seems
> > like you want a different behavior than that.
> 
> Then I must have phrased something poorly to give you a false impression. All
I
> said was that the color of this icon should match the color of links. What the
> color of links actually is a different matter.

Ugh, sorry, I think I got confused and derailed this all unnecessarily.

LGTM.  The existing set of classes and the implementation method for colors
kinda doesn't make sense, but there's nothing in this patch that makes anything
worse, so let's not hold this hostage to improving that somehow.

[Evan Stade, 2015-10-27 20:53:55]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-10-27 20:54:56]

estade@chromium.org changed reviewers:
+ sadrul@chromium.org

[Evan Stade, 2015-10-27 20:54:56]

+sadrul TBR for ui/gfx/BUILD.gn

[commit-bot: I haz the power, 2015-10-27 20:54:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421683002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421683002/40001

[Evan Stade, 2015-10-27 20:55:06]

Description was changed from

==========
Update search chip icon for MD, vectorize.

Also fixes the icons for the omnibox search extension API in MD mode.

BUG=505953
==========

to

==========
Update search chip icon for MD, vectorize.

Also fixes the icons for the omnibox search extension API in MD mode.

BUG=505953
TBR=sadrul@chromium.org
==========

[commit-bot: I haz the power, 2015-10-27 22:08:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-27 22:09:07]

Patchset 3 (id:??) landed as
https://crrev.com/81a623407a261c91e90b24266c12fdc8cf17c10b
Cr-Commit-Position: refs/heads/master@{#356409}