Update infobars to use VectorIcon struct instead of VectorIconId.

Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

TBR=rohitrao@chromium.org
BUG=626786
Review-Url: https://codereview.chromium.org/2711623003
Cr-Commit-Position: refs/heads/master@{#452940}
Committed: https://chromium.googlesource.com/chromium/src/+/bd85908c96b3ef0016091664a0dff37eaaced26b

[Evan Stade, 2017-02-22 03:05:14]

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

[commit-bot: I haz the power, 2017-02-22 03:06:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 03:15:47]

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

[commit-bot: I haz the power, 2017-02-22 03:15:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Evan Stade, 2017-02-22 17:09:26]

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

[commit-bot: I haz the power, 2017-02-22 17:10:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 17:19:31]

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

[commit-bot: I haz the power, 2017-02-22 17:19:31]

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

[Evan Stade, 2017-02-22 17:25:32]

Description was changed from

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

BUG=626786
==========

to

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

BUG=626786
==========

[Evan Stade, 2017-02-22 17:25:32]

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

[Evan Stade, 2017-02-22 17:26:39]

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

[Evan Stade, 2017-02-22 17:26:40]

+pkasting for *infobar*
+sky for the rest

[Evan Stade, 2017-02-22 17:27:01]

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

[commit-bot: I haz the power, 2017-02-22 17:27:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 17:38:17]

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

[commit-bot: I haz the power, 2017-02-22 17:38:18]

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

[sky, 2017-02-22 21:10:43]

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
File chrome/app/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
chrome/app/vector_icons/BUILD.gn:5: import("//build/util/branding.gni")
Can you add an OWNERS file here with your name in it?

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
chrome/app/vector_icons/BUILD.gn:12: "apps.icon",
Keep sorted?

https://codereview.chromium.org/2711623003/diff/40001/ui/vector_icons/BUILD.gn
File ui/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2711623003/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:18: "extension_crashed.icon",
How come you're putting extensioncrashed here? AFAICT it's only used in chrome?
I think extension.icon should be moved to chrome too.

[Evan Stade, 2017-02-22 21:30:11]

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
File chrome/app/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
chrome/app/vector_icons/BUILD.gn:5: import("//build/util/branding.gni")
On 2017/02/22 21:10:43, sky wrote:
> Can you add an OWNERS file here with your name in it?

Done.

https://codereview.chromium.org/2711623003/diff/40001/chrome/app/vector_icons...
chrome/app/vector_icons/BUILD.gn:12: "apps.icon",
On 2017/02/22 21:10:43, sky wrote:
> Keep sorted?

whoops. I thought gn format would do that for me.

https://codereview.chromium.org/2711623003/diff/40001/ui/vector_icons/BUILD.gn
File ui/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2711623003/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:18: "extension_crashed.icon",
On 2017/02/22 21:10:43, sky wrote:
> How come you're putting extensioncrashed here? AFAICT it's only used in
chrome?
> I think extension.icon should be moved to chrome too.

I just wanted to keep it next to extension.icon (the only difference being that
a slice is missing from the latter). But you're right, they can both go in
chrome/.

[Evan Stade, 2017-02-22 21:30:42]

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

[commit-bot: I haz the power, 2017-02-22 21:31:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sky, 2017-02-22 21:36:56]

LGTM

[commit-bot: I haz the power, 2017-02-22 22:49:15]

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

[commit-bot: I haz the power, 2017-02-22 22:49:15]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-02-23 02:07:17]

I will try to get to this tomorrow.

[Peter Kasting, 2017-02-24 02:12:27]

LGTM

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/chrom...
File chrome/browser/ui/chrome_select_file_policy.cc (right):

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/chrom...
chrome/browser/ui/chrome_select_file_policy.cc:36:
infobars::InfoBarDelegate::CHROME_SELECT_FILE_POLICY, nullptr,
Not for this CL (since this isn't a behavior change), but it seems like a bug
that we don't have an icon for this.

Do we need to file a bug against a UI designer to get one or is there an
appropriate one in the tree somewhere?

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/start...
File chrome/browser/ui/startup/bad_flags_prompt.cc (right):

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/start...
chrome/browser/ui/startup/bad_flags_prompt.cc:97:
infobars::InfoBarDelegate::BAD_FLAGS_PROMPT, nullptr,
Similarly, seems like this should have an icon (e.g. /!\)

https://codereview.chromium.org/2711623003/diff/60001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller_dependency_factory.mm
(right):

https://codereview.chromium.org/2711623003/diff/60001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller_dependency_factory.mm:55:
infobars::InfoBarDelegate::SHOW_PASSKIT_INFOBAR_ERROR_DELEGATE, nullptr,
Maybe this is another /!\ candidate.

[Evan Stade, 2017-02-24 17:18:17]

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/chrom...
File chrome/browser/ui/chrome_select_file_policy.cc (right):

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/chrom...
chrome/browser/ui/chrome_select_file_policy.cc:36:
infobars::InfoBarDelegate::CHROME_SELECT_FILE_POLICY, nullptr,
On 2017/02/24 02:12:26, Peter Kasting wrote:
> Not for this CL (since this isn't a behavior change), but it seems like a bug
> that we don't have an icon for this.
> 
> Do we need to file a bug against a UI designer to get one or is there an
> appropriate one in the tree somewhere?

they'd probably tell us to reuse the business icon (office building) since it's
something that's controlled by policy. I suspect this is just such a rare
infobar no one has noticed or really cared. bug filed.

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/start...
File chrome/browser/ui/startup/bad_flags_prompt.cc (right):

https://codereview.chromium.org/2711623003/diff/60001/chrome/browser/ui/start...
chrome/browser/ui/startup/bad_flags_prompt.cc:97:
infobars::InfoBarDelegate::BAD_FLAGS_PROMPT, nullptr,
On 2017/02/24 02:12:27, Peter Kasting wrote:
> Similarly, seems like this should have an icon (e.g. /!\)

maybe but I am even less enthusiastic about this one for the same reason we
don't try to make chrome://flags/ look good.

https://codereview.chromium.org/2711623003/diff/60001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller_dependency_factory.mm
(right):

https://codereview.chromium.org/2711623003/diff/60001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller_dependency_factory.mm:55:
infobars::InfoBarDelegate::SHOW_PASSKIT_INFOBAR_ERROR_DELEGATE, nullptr,
On 2017/02/24 02:12:27, Peter Kasting wrote:
> Maybe this is another /!\ candidate.

no idea what this even is. "Sorry, your Pass cannot be installed to Passbook at
this time." Would a generic warning icon add much to that? If so maybe we ought
to just fall back to a warning icon when none is provided. Or if we really want
good icons for every obscure infobar, disallow nullptr.

[Evan Stade, 2017-02-24 17:19:02]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-02-24 17:19:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 17:27:14]

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

[commit-bot: I haz the power, 2017-02-24 17:27:16]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Evan Stade, 2017-02-24 19:33:02]

Description was changed from

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

BUG=626786
==========

to

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

TBR=rohitrao@chromium.org
BUG=626786
==========

[Evan Stade, 2017-02-24 19:33:03]

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

[Evan Stade, 2017-02-24 19:33:17]

tbr rohitrao for ios change

[Evan Stade, 2017-02-24 19:33:23]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-02-24 19:34:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 19:39:50]

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

[commit-bot: I haz the power, 2017-02-24 19:39:52]

Failed to apply patch for ui/gfx/vector_icons/BUILD.gn:
While running git apply --index -p1;
  error: patch failed: ui/gfx/vector_icons/BUILD.gn:61
  error: ui/gfx/vector_icons/BUILD.gn: patch does not apply

Patch:       ui/gfx/vector_icons/BUILD.gn
Index: ui/gfx/vector_icons/BUILD.gn
diff --git a/ui/gfx/vector_icons/BUILD.gn b/ui/gfx/vector_icons/BUILD.gn
index
1bd790f19c190a480ec95ddc1486270fe4c223c5..92d60bbf56777f3bad030e2ebbeec2c4d3ef7565
100644
--- a/ui/gfx/vector_icons/BUILD.gn
+++ b/ui/gfx/vector_icons/BUILD.gn
@@ -14,7 +14,6 @@ action("aggregate_vector_icons") {
     "account_child.icon",
     "account_child_circle.icon",
     "account_circle.icon",
-    "apps.icon",
     "blocked_badge.icon",
     "business.icon",
     "check_circle.1x.icon",
@@ -25,10 +24,8 @@ action("aggregate_vector_icons") {
     "default_favicon.icon",
     "eol.icon",
     "extension.icon",
-    "extension_crashed.icon",
     "file_download.icon",
     "fullscreen.icon",
-    "globe.icon",
     "help_outline.icon",
     "image.icon",
     "info_outline.icon",
@@ -43,14 +40,12 @@ action("aggregate_vector_icons") {
     # TODO(estade): this is the same as the one in components/omnibox, but it's
     # referenced from Ash. De-dupe this soon.
     "omnibox_search.icon",
-    "paintbrush.icon",
     "pdf.1x.icon",
     "pdf.icon",
     "photo_camera.icon",
     "protocol_handler.icon",
     "remove_box.icon",
     "remove_circle.icon",
-    "sad_tab.icon",
     "settings.icon",
     "subresource_filter_active.icon",
     "supervisor_account.icon",
@@ -61,7 +56,6 @@ action("aggregate_vector_icons") {
     "warning.icon",
     "warning_badge.icon",
     "web.icon",
-    "${branding_path_component}/product.icon",
   ]
 
   output_cc = "$target_gen_dir/vector_icons.cc"

[Evan Stade, 2017-02-24 19:58:28]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-02-24 19:58:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2711623003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-24 19:59:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 21:53:51]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487966308155460,
"parent_rev": "c07578e81c74f2a7dc371699fbf1c7a9d8389e79", "commit_rev":
"bd85908c96b3ef0016091664a0dff37eaaced26b"}

[commit-bot: I haz the power, 2017-02-24 21:54:25]

Description was changed from

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

TBR=rohitrao@chromium.org
BUG=626786
==========

to

==========
Update infobars to use VectorIcon struct instead of VectorIconId.

Once again, some duped icons until we can update more clients.

TBR=rohitrao@chromium.org
BUG=626786

Review-Url: https://codereview.chromium.org/2711623003
Cr-Commit-Position: refs/heads/master@{#452940}
Committed:
https://chromium.googlesource.com/chromium/src/+/bd85908c96b3ef0016091664a0df...
==========

[commit-bot: I haz the power, 2017-02-24 21:54:30]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/bd85908c96b3ef0016091664a0df...