Generalize ExtensionIconSet to store icon paths for custom size sets

Generalize ExtensionIconSet to store icon paths for custom size sets.

This is needed to make it easier to handle sets of extension icons that are not
defined in extension manifest (e.g. default icons for browser_actions)

BUG=138025, 135271
TEST=existing tests

For mechanical changes caused by moving ExtensionIcons from ExtensionIconSet to extension_misc namespace (patchset 14):
TBR=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151992

[tbarzic, 2012-08-07 21:29:29]

Hey, Aaron, can you take a look at this?

[Aaron Boodman, 2012-08-10 20:53:16]

Hard to evaluate without a sample usage. Can you add some call sites so we can
see it in action?

[Aaron Boodman, 2012-08-10 20:54:48]

In general it seems weird to combine such different responsibilities in one
class. Also, now, ExtensionIconSet seems in some ways to do what ImageSkia does.

[tbarzic, 2012-08-10 22:20:18]

On 2012/08/10 20:54:48, Aaron Boodman wrote:
> In general it seems weird to combine such different responsibilities in one
> class. Also, now, ExtensionIconSet seems in some ways to do what ImageSkia
does.

Yeah, bitmaps are a bit overkill.. removed them from icon set (the use case I
planned to solve with them can be solved differently)

PTAL

[Aaron Boodman, 2012-08-10 23:01:57]

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.cc (right):

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.cc:11: const int
kExtensionIconSizes[] = {
Now that this class is more general purpose, move these constants (which refer
to a particular set of icons) out to someplace like extension_constants.h.

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.cc:36: allowed_sizes_ =
kExtensionIconSizes;
I do not think it is necessary to validate that the icons added are within the
set of allowed sizes. You can remove IconSetType.

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.h (right):

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.h:20: // the cc file Icons {
s/ file Icons {/./

[tbarzic, 2012-08-11 00:58:45]

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.cc (right):

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.cc:11: const int
kExtensionIconSizes[] = {
On 2012/08/10 23:01:57, Aaron Boodman wrote:
> Now that this class is more general purpose, move these constants (which refer
> to a particular set of icons) out to someplace like extension_constants.h.

Done.

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.cc:36: allowed_sizes_ =
kExtensionIconSizes;
On 2012/08/10 23:01:57, Aaron Boodman wrote:
> I do not think it is necessary to validate that the icons added are within the
> set of allowed sizes. You can remove IconSetType.

Done.

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.h (right):

http://codereview.chromium.org/10843014/diff/4035/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.h:20: // the cc file Icons {
On 2012/08/10 23:01:57, Aaron Boodman wrote:
> s/ file Icons {/./

Done.

[Aaron Boodman, 2012-08-13 23:33:25]

lgtm w/ nit

http://codereview.chromium.org/10843014/diff/9012/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.h (right):

http://codereview.chromium.org/10843014/diff/9012/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.h:16: enum ExtensionIcons {
Can you move this enum to extension_constants, too? (It has little to do with
this file now).

[tbarzic, 2012-08-14 18:06:57]

http://codereview.chromium.org/10843014/diff/9012/chrome/common/extensions/ex...
File chrome/common/extensions/extension_icon_set.h (right):

http://codereview.chromium.org/10843014/diff/9012/chrome/common/extensions/ex...
chrome/common/extensions/extension_icon_set.h:16: enum ExtensionIcons {
On 2012/08/13 23:33:25, Aaron Boodman wrote:
> Can you move this enum to extension_constants, too? (It has little to do with
> this file now).

Done.

[tbarzic, 2012-08-14 18:14:13]

added TBR=ben

[commit-bot: I haz the power, 2012-08-15 15:05:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/10843014/8024

[commit-bot: I haz the power, 2012-08-15 16:13:22]

Try job failure for 10843014-8024 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-08-15 20:05:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/10843014/8024

[commit-bot: I haz the power, 2012-08-15 22:39:15]

Try job failure for 10843014-8024 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, steps "interactive_ui_tests, browser_tests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-16 18:01:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/10843014/8024

[commit-bot: I haz the power, 2012-08-16 19:34:47]

Try job failure for 10843014-8024 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...