Move Icons out of Extension class

Move Icons out of Extension class

BUG=159265
TBR=davemoore@chromium.org, atwilson@chromium.org, dalecurtis@chromium.org
(davemoore - c/b/chromeos, atwilson - c/b/background, dalecurtis - c/b/media)

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

[Yoyo Zhou, 2013-01-08 01:26:12]

Nifty.

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/browser/ui/a...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/browser/ui/a...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:289:
item.image = extensions::IconsInfo::GetDefaultIcon(true);  // is app
This is silly. Can we have
GetDefaultAppIcon
GetDefaultExtensionIcon
instead?

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/common/exten...
File chrome/common/extensions/api/icons/icons_handler.cc (right):

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/common/exten...
chrome/common/extensions/api/icons/icons_handler.cc:25: struct EmptyIconSet {
Do you really need to wrap this in a struct?

[Devlin, 2013-01-16 23:32:28]

+ sky for c/b/ui

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/browser/ui/a...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/browser/ui/a...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:289:
item.image = extensions::IconsInfo::GetDefaultIcon(true);  // is app
On 2013/01/08 01:26:13, Yoyo Zhou wrote:
> This is silly. Can we have
> GetDefaultAppIcon
> GetDefaultExtensionIcon
> instead?

Done.

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/common/exten...
File chrome/common/extensions/api/icons/icons_handler.cc (right):

https://chromiumcodereview.appspot.com/11786003/diff/6001/chrome/common/exten...
chrome/common/extensions/api/icons/icons_handler.cc:25: struct EmptyIconSet {
On 2013/01/08 01:26:13, Yoyo Zhou wrote:
> Do you really need to wrap this in a struct?

Done.

[sky, 2013-01-17 00:47:10]

LGTM

https://chromiumcodereview.appspot.com/11786003/diff/35001/chrome/browser/ui/...
File chrome/browser/ui/app_list/extension_app_item.cc (right):

https://chromiumcodereview.appspot.com/11786003/diff/35001/chrome/browser/ui/...
chrome/browser/ui/app_list/extension_app_item.cc:1: // Copyright (c) 2013 The
Chromium Authors. All rights reserved.
New year, new rules. Don't update copyrights anymore like this. Applies to *all*
your files here (sorry).

[Yoyo Zhou, 2013-01-22 19:56:59]

LGTM

[Devlin, 2013-01-28 21:47:10]

+ nileshagrawal@ for android gypi changes
+ ben@ for gypi, chrome/common/localized_error.cc, and
chrome/utility/chrome_content_utility_client.cc
+ mirandac@ for c/b/profiles

[nilesh, 2013-01-28 22:42:56]

LGTM with nits.

https://chromiumcodereview.appspot.com/11786003/diff/85001/chrome/browser/ext...
File chrome/browser/extensions/api/icons/icons_api.h (right):

https://chromiumcodereview.appspot.com/11786003/diff/85001/chrome/browser/ext...
chrome/browser/extensions/api/icons/icons_api.h:30: }
DISALLOW_COPY_AND_ASSIGN(..)
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_C...

https://chromiumcodereview.appspot.com/11786003/diff/85001/chrome/common/exte...
File chrome/common/extensions/api/icons/icons_handler.h (right):

https://chromiumcodereview.appspot.com/11786003/diff/85001/chrome/common/exte...
chrome/common/extensions/api/icons/icons_handler.h:76: string16* error)
OVERRIDE;
DISALLOW_COPY_AND_ASSIGN here too?
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_C...

[Yoyo Zhou, 2013-01-29 19:49:41]

FYI, I just found out that script badges depends on icons.

[Ben Goodger (Google), 2013-01-30 18:40:43]

lgtm

[Yoyo Zhou, 2013-02-05 23:55:48]

You can add icons to the PrerequisiteKeys for script badges. This change LGTM
with that one change.

[Miranda Callahan, 2013-02-06 13:10:58]

On 2013/02/05 23:55:48, Yoyo Zhou wrote:
> You can add icons to the PrerequisiteKeys for script badges. This change LGTM
> with that one change.

profile_dependency_manager LGTM

[commit-bot: I haz the power, 2013-02-26 06:24:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/11786003/1...

[commit-bot: I haz the power, 2013-02-26 07:34:59]

Retried try job too often on mac_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-02-27 18:48:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/11786003/1...

[Yoyo Zhou, 2013-02-27 20:09:14]

LGTM

https://chromiumcodereview.appspot.com/11786003/diff/116004/chrome/common/ext...
File chrome/common/extensions/api/extension_action/script_badge_handler.cc
(right):

https://chromiumcodereview.appspot.com/11786003/diff/116004/chrome/common/ext...
chrome/common/extensions/api/extension_action/script_badge_handler.cc:30: const
std::vector<std::string>& ScriptBadgeHandler::PrerequisiteKeys() {
By the way, I later realized, it doesn't make sense to return by reference here.
If you want to change it so that it doesn't, feel free; otherwise, I'll do it in
my pending CL.

[commit-bot: I haz the power, 2013-02-27 20:55:53]

Change committed as 185030