Allow arbitrary sizes for extension icons (and bookmark app icons)

Allow arbitrary sizes for extension icons (and bookmark app icons)

BUG=564926
Committed: https://crrev.com/fd2986d06d57ad04fc6f8db79ddb7a42d6445e03
Cr-Commit-Position: refs/heads/master@{#363669}

[Evan Stade, 2015-12-04 22:33:20]

estade@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Devlin, 2015-12-07 20:37:19]

Looks like a few tests need to be updated, but generally lg.

https://codereview.chromium.org/1504453002/diff/1/chrome/browser/extensions/b...
File chrome/browser/extensions/bookmark_app_helper.cc (right):

https://codereview.chromium.org/1504453002/diff/1/chrome/browser/extensions/b...
chrome/browser/extensions/bookmark_app_helper.cc:785: extension, size,
ExtensionIconSet::MATCH_EXACTLY);
I'm pretty sure this is the same as
resource = extension->GetResource(iter->second);

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants.cc
File extensions/common/constants.cc (left):

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants...
extensions/common/constants.cc:64: const int kExtensionIconSizes[] =
{EXTENSION_ICON_GIGANTOR,     // 512
Wow, awesome that we don't need this anymore.  The slight downside is that,
without this, I'm not sure I'd know how to find the "general" size for an icon
purpose.  Can you rephrase this enum in the form of a comment and put it on,
say, IconsHandler?

[Evan Stade, 2015-12-07 20:52:48]

tests updated

https://codereview.chromium.org/1504453002/diff/1/chrome/browser/extensions/b...
File chrome/browser/extensions/bookmark_app_helper.cc (right):

https://codereview.chromium.org/1504453002/diff/1/chrome/browser/extensions/b...
chrome/browser/extensions/bookmark_app_helper.cc:785: extension, size,
ExtensionIconSet::MATCH_EXACTLY);
On 2015/12/07 20:37:19, Devlin wrote:
> I'm pretty sure this is the same as
> resource = extension->GetResource(iter->second);

seems true, changed.

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants.cc
File extensions/common/constants.cc (left):

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants...
extensions/common/constants.cc:64: const int kExtensionIconSizes[] =
{EXTENSION_ICON_GIGANTOR,     // 512
On 2015/12/07 20:37:19, Devlin wrote:
> Wow, awesome that we don't need this anymore.  The slight downside is that,
> without this, I'm not sure I'd know how to find the "general" size for an icon
> purpose.  Can you rephrase this enum in the form of a comment and put it on,
> say, IconsHandler?

Wouldn't you look at the enum definition:
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/common/...

I'm not sure what more info this array provided

[Evan Stade, 2015-12-07 20:53:14]

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

[Evan Stade, 2015-12-07 20:53:30]

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

[Evan Stade, 2015-12-07 20:53:30]

+tapted for chrome/browser/apps/drive/

[commit-bot: I haz the power, 2015-12-07 20:56:06]

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

[Devlin, 2015-12-07 21:02:39]

extensions lgtm

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants.cc
File extensions/common/constants.cc (left):

https://codereview.chromium.org/1504453002/diff/1/extensions/common/constants...
extensions/common/constants.cc:64: const int kExtensionIconSizes[] =
{EXTENSION_ICON_GIGANTOR,     // 512
On 2015/12/07 20:52:48, Evan Stade wrote:
> On 2015/12/07 20:37:19, Devlin wrote:
> > Wow, awesome that we don't need this anymore.  The slight downside is that,
> > without this, I'm not sure I'd know how to find the "general" size for an
icon
> > purpose.  Can you rephrase this enum in the form of a comment and put it on,
> > say, IconsHandler?
> 
> Wouldn't you look at the enum definition:
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/common/...
> 
> I'm not sure what more info this array provided

Gah.  Mondays.  Ignore.

[tapted, 2015-12-08 00:11:48]

chrome/browser/apps/drive lgtm

[commit-bot: I haz the power, 2015-12-08 00:41:41]

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

[commit-bot: I haz the power, 2015-12-08 00:41:41]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2015-12-08 00:44:29]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2015-12-08 00:47:38]

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

[commit-bot: I haz the power, 2015-12-08 01:14:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-08 01:15:41]

Description was changed from

==========
Allow arbitrary sizes for extension icons (and bookmark app icons)

BUG=564926
==========

to

==========
Allow arbitrary sizes for extension icons (and bookmark app icons)

BUG=564926

Committed: https://crrev.com/fd2986d06d57ad04fc6f8db79ddb7a42d6445e03
Cr-Commit-Position: refs/heads/master@{#363669}
==========

[commit-bot: I haz the power, 2015-12-08 01:15:41]

Patchset 2 (id:??) landed as
https://crrev.com/fd2986d06d57ad04fc6f8db79ddb7a42d6445e03
Cr-Commit-Position: refs/heads/master@{#363669}