Vectorize website settings icons in omnibox

Vectorize website settings icons in omnibox

unlike other icon vectorization/MD work, this does respect the --top-chrome-md flag, because these icons live in the omnibox and look funky if you put them in the old omnibox.

BUG=505953, 514718
Committed: https://crrev.com/7ca51e26dc7316b50d5f96e4e1b4312584cb995f
Cr-Commit-Position: refs/heads/master@{#346432}

[Evan Stade, 2015-08-26 22:57:27]

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

[Evan Stade, 2015-08-26 22:57:53]

+sky for review
+tdanderson, shrike FYI

https://codereview.chromium.org/1313603003/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm (left):

https://codereview.chromium.org/1313603003/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm:181: int
old_icon = content_setting_image_model_->get_icon();
osx needs the icon id to determine if the image has changed (I guess it's a perf
optimization). I don't know how to compare two gfx::Images efficiently, which
we'll probably need to do eventually when mac switches to vectorization.

[sky, 2015-08-26 23:27:00]

Don't you need to add the new icons to build files? Assuming you do that, LGTM

https://codereview.chromium.org/1313603003/diff/60001/ui/gfx/paint_vector_icon.h
File ui/gfx/paint_vector_icon.h (right):

https://codereview.chromium.org/1313603003/diff/60001/ui/gfx/paint_vector_ico...
ui/gfx/paint_vector_icon.h:30: GFX_EXPORT ImageSkia
CreateVectorIconWithBadge(VectorIconId id,
Document what badge_id is.

[tdanderson, 2015-08-27 13:25:22]

tdanderson@chromium.org changed reviewers:
+ tdanderson@chromium.org

[tdanderson, 2015-08-27 13:25:23]

https://codereview.chromium.org/1313603003/diff/80001/chrome/browser/ui/conte...
File chrome/browser/ui/content_settings/content_setting_image_model.cc (right):

https://codereview.chromium.org/1313603003/diff/80001/chrome/browser/ui/conte...
chrome/browser/ui/content_settings/content_setting_image_model.cc:29: bool
IsMaterial() {
This function shouldn't be necessary. IsModeMaterial() will return false for
platforms which don't have the compile-time flag ENABLE_TOPCHROME_MD set (OSX
being one of them)

[Evan Stade, 2015-08-27 15:49:48]

https://codereview.chromium.org/1313603003/diff/80001/chrome/browser/ui/conte...
File chrome/browser/ui/content_settings/content_setting_image_model.cc (right):

https://codereview.chromium.org/1313603003/diff/80001/chrome/browser/ui/conte...
chrome/browser/ui/content_settings/content_setting_image_model.cc:29: bool
IsMaterial() {
On 2015/08/27 13:25:23, tdanderson wrote:
> This function shouldn't be necessary. IsModeMaterial() will return false for
> platforms which don't have the compile-time flag ENABLE_TOPCHROME_MD set (OSX
> being one of them)

tldr, this fn should be renamed to UseVectorGraphics()

I did this because someone could theoretically set that compile time flag on mac
and then the code below would break. We may not support that configuration today
but we might support it before vectorized rendering comes to mac (although
hopefully not).

[Evan Stade, 2015-08-27 17:41:27]

https://codereview.chromium.org/1313603003/diff/100001/ui/gfx/vector_icons/bl...
File ui/gfx/vector_icons/blocked_badge.icon (right):

https://codereview.chromium.org/1313603003/diff/100001/ui/gfx/vector_icons/bl...
ui/gfx/vector_icons/blocked_badge.icon:6: PATH_MODE_CLEAR,
I forgot I had to add a halo effect around the badge. PTAL this additional
change. Effect is: https://screenshot.googleplex.com/A0WXSNwhsWk.png

[Evan Stade, 2015-08-28 15:30:42]

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

[commit-bot: I haz the power, 2015-08-28 15:30:52]

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

[commit-bot: I haz the power, 2015-08-28 15:45:23]

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

[commit-bot: I haz the power, 2015-08-28 15:45:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2015-08-28 20:55:57]

https://codereview.chromium.org/1313603003/diff/100001/ui/gfx/vector_icons/bl...
File ui/gfx/vector_icons/blocked_badge.icon (right):

https://codereview.chromium.org/1313603003/diff/100001/ui/gfx/vector_icons/bl...
ui/gfx/vector_icons/blocked_badge.icon:6: PATH_MODE_CLEAR,
On 2015/08/27 17:41:27, Evan Stade wrote:
> I forgot I had to add a halo effect around the badge. PTAL this additional
> change. Effect is: https://screenshot.googleplex.com/A0WXSNwhsWk.png

ping sky for re-review of this new directive/usage

[sky, 2015-08-29 16:02:13]

LGTM

[Evan Stade, 2015-08-31 18:25:11]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-08-31 18:25:12]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1313603003/#ps140001
(title: "change DCHECK")

[commit-bot: I haz the power, 2015-08-31 18:25:23]

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

[commit-bot: I haz the power, 2015-08-31 19:03:41]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-08-31 19:04:13]

Patchset 8 (id:??) landed as
https://crrev.com/7ca51e26dc7316b50d5f96e4e1b4312584cb995f
Cr-Commit-Position: refs/heads/master@{#346432}