Remove some more desktop infobar PNGs.

Remove some more desktop infobar PNGs.

BUG=648281
Committed: https://crrev.com/939f99090ea8725dd30ad88825933ee2ca47d0b0
Cr-Commit-Position: refs/heads/master@{#423212}

[Evan Stade, 2016-09-28 23:43:02]

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

[commit-bot: I haz the power, 2016-09-28 23:43:24]

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, 2016-09-29 00:20:51]

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

[commit-bot: I haz the power, 2016-09-29 00:20:52]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-09-29 01:01:45]

Description was changed from

==========
wip

BUG=
==========

to

==========
Remove some more desktop infobar PNGs.

BUG=648281
==========

[Evan Stade, 2016-09-29 01:01:45]

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

[Evan Stade, 2016-09-29 01:01:54]

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

[commit-bot: I haz the power, 2016-09-29 01:02:32]

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, 2016-09-29 01:12:52]

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

[commit-bot: I haz the power, 2016-09-29 01:12:53]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2016-09-29 17:49:53]

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

[commit-bot: I haz the power, 2016-09-29 17:50:13]

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, 2016-09-29 17:53:12]

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

[commit-bot: I haz the power, 2016-09-29 17:53:13]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Evan Stade, 2016-09-30 00:01:24]

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

[commit-bot: I haz the power, 2016-09-30 00:02:39]

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, 2016-09-30 00:19:41]

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

[commit-bot: I haz the power, 2016-09-30 00:19:42]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Evan Stade, 2016-09-30 18:05:49]

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

[commit-bot: I haz the power, 2016-09-30 18:06:28]

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

[Evan Stade, 2016-09-30 18:11:07]

estade@chromium.org changed reviewers:
+ groby@chromium.org, thestig@chromium.org

[Evan Stade, 2016-09-30 18:11:08]

+oshima for c/a/t
+groby for cocoa
+pkasting for everything else (a bunch of infobar delegates, mainly)
+thestig for OWNERS stamp (a bunch of different directories are touched here but
they're mostly fairly mechanical updates to infobar delegates, and infobars are
something Peter owns)

[commit-bot: I haz the power, 2016-09-30 18:17:08]

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

[commit-bot: I haz the power, 2016-09-30 18:17:09]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[groby-ooo-7-16, 2016-09-30 18:23:45]

LGTM % compile error

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_infobar_delegates.h (right):

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_infobar_delegates.h:53: gfx::VectorIconId
GetVectoIconId() const override;
GetVectorIconId :)

(missing 'r' - I wish codereview allowed me to just fix it)

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:50:
#include "ui/gfx/vector_icons_public.h"
Isn't that a transitive dependency of paint_vector_icon.h and never actually
used in here?

[Evan Stade, 2016-09-30 22:31:36]

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

[Evan Stade, 2016-09-30 22:31:39]

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_infobar_delegates.h (right):

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_infobar_delegates.h:53: gfx::VectorIconId
GetVectoIconId() const override;
On 2016/09/30 18:23:45, groby wrote:
> GetVectorIconId :)
> 
> (missing 'r' - I wish codereview allowed me to just fix it)

oops -- had fixed this but failed to reupload

[Evan Stade, 2016-09-30 22:31:55]

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2382563003/diff/70001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:50:
#include "ui/gfx/vector_icons_public.h"
On 2016/09/30 18:23:45, groby wrote:
> Isn't that a transitive dependency of paint_vector_icon.h and never actually
> used in here?

Done.

[commit-bot: I haz the power, 2016-09-30 22:32:16]

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

[oshima, 2016-09-30 23:04:34]

c/a/theme lgtm

[commit-bot: I haz the power, 2016-09-30 23:22:01]

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

[commit-bot: I haz the power, 2016-09-30 23:22:02]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Lei Zhang, 2016-09-30 23:56:11]

Still got red bots.

[Evan Stade, 2016-10-03 20:07:13]

ping pkasting

[Evan Stade, 2016-10-03 20:37:48]

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

[commit-bot: I haz the power, 2016-10-03 20:38:34]

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, 2016-10-03 21:04:28]

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

[commit-bot: I haz the power, 2016-10-03 21:04:29]

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, 2016-10-04 01:28:22]

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

[commit-bot: I haz the power, 2016-10-04 01:29:03]

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, 2016-10-04 02:00:38]

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

[commit-bot: I haz the power, 2016-10-04 02:00:39]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Peter Kasting, 2016-10-04 07:56:33]

On 2016/10/03 20:07:13, Evan Stade wrote:
> ping pkasting

Will try to get to this Tue 10/4

[Evan Stade, 2016-10-04 21:12:43]

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

[commit-bot: I haz the power, 2016-10-04 21:13:30]

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, 2016-10-04 22:12:21]

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

[commit-bot: I haz the power, 2016-10-04 22:12:22]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2016-10-05 01:13:13]

I assume that places which previously had no vector icon ID, for which you
replaced the raster ID with a vector one with a different name, are in fact not
seeing a behavioral change, just a different naming scheme for the vector icons?

LGTM if so.

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/android...
File chrome/browser/android/resource_id.h (right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/android...
chrome/browser/android/resource_id.h:45: R.drawable.infobar_multiple_downloads)
Nit: Can these be alphabetized or ordered in some other way?

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
File chrome/browser/permissions/permission_request_impl.cc (right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
chrome/browser/permissions/permission_request_impl.cc:44: #if
!defined(OS_ANDROID)
Nit: Reverse to avoid double-negative in "else":

#if defined(OS_ANDROID)
  return gfx::VectorIconId::VECTOR_ICON_NONE;
#else
  ...
#endif

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
chrome/browser/permissions/permission_request_impl.cc:73: #if
defined(OS_ANDROID)
Nit: Maybe put the entire function (and declaration in the .h) in an #if
OS_ANDROID, then eliminate |icon_id| entirely and just "return IDR_XXX" from
each switch case.

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:513:
gfx::CreateVectorIcon(request->GetVectorIconId(), 18,
Nit: Does this 18 need to be in sync with other values elsewhere (i.e. deserve a
shared constant somewhere)?

[Lei Zhang, 2016-10-05 01:29:02]

lgtm

[Evan Stade, 2016-10-05 16:55:32]

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

[commit-bot: I haz the power, 2016-10-05 16:56:02]

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, 2016-10-05 17:41:45]

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

[commit-bot: I haz the power, 2016-10-05 17:41:46]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-10-05 17:42:29]

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/android...
File chrome/browser/android/resource_id.h (right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/android...
chrome/browser/android/resource_id.h:45: R.drawable.infobar_multiple_downloads)
On 2016/10/05 01:13:13, Peter Kasting wrote:
> Nit: Can these be alphabetized or ordered in some other way?

I don't see why not --- done

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
File chrome/browser/permissions/permission_request_impl.cc (right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
chrome/browser/permissions/permission_request_impl.cc:44: #if
!defined(OS_ANDROID)
On 2016/10/05 01:13:13, Peter Kasting wrote:
> Nit: Reverse to avoid double-negative in "else":
> 
> #if defined(OS_ANDROID)
>   return gfx::VectorIconId::VECTOR_ICON_NONE;
> #else
>   ...
> #endif

Done.

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/permiss...
chrome/browser/permissions/permission_request_impl.cc:73: #if
defined(OS_ANDROID)
On 2016/10/05 01:13:13, Peter Kasting wrote:
> Nit: Maybe put the entire function (and declaration in the .h) in an #if
> OS_ANDROID, then eliminate |icon_id| entirely and just "return IDR_XXX" from
> each switch case.

done

I'm not entirely sure this is even necessary on android but I have another patch
coming to remove it if not.

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2382563003/diff/130001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:513:
gfx::CreateVectorIcon(request->GetVectorIconId(), 18,
On 2016/10/05 01:13:13, Peter Kasting wrote:
> Nit: Does this 18 need to be in sync with other values elsewhere (i.e. deserve
a
> shared constant somewhere)?

it is the same as a constant in Views, but it's not used anywhere else in cocoa.
The top of this file is full of cocoa-specific constants so I think it makes
more sense to add this here as another cocoa-specific constant that
coincidentally matches the Views one.

[Evan Stade, 2016-10-05 17:42:36]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-10-05 17:42:37]

The patchset sent to the CQ was uploaded after l-g-t-m from groby@chromium.org,
oshima@chromium.org, thestig@chromium.org, pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2382563003/#ps170001
(title: "pkasting review")

[commit-bot: I haz the power, 2016-10-05 17:43:05]

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, 2016-10-05 17:48:53]

Description was changed from

==========
Remove some more desktop infobar PNGs.

BUG=648281
==========

to

==========
Remove some more desktop infobar PNGs.

BUG=648281
==========

[commit-bot: I haz the power, 2016-10-05 17:48:54]

Committed patchset #10 (id:170001)

[commit-bot: I haz the power, 2016-10-05 17:50:01]

Description was changed from

==========
Remove some more desktop infobar PNGs.

BUG=648281
==========

to

==========
Remove some more desktop infobar PNGs.

BUG=648281

Committed: https://crrev.com/939f99090ea8725dd30ad88825933ee2ca47d0b0
Cr-Commit-Position: refs/heads/master@{#423212}
==========

[commit-bot: I haz the power, 2016-10-05 17:50:02]

Patchset 10 (id:??) landed as
https://crrev.com/939f99090ea8725dd30ad88825933ee2ca47d0b0
Cr-Commit-Position: refs/heads/master@{#423212}