Generate a browser action for any extension which doesn't have a browser or

Generate a browser action for any extension which doesn't have a browser or
page action.

BUG=124605
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134657

[Aaron Boodman, 2012-04-30 02:37:04]

LGTM

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
File chrome/common/extensions/extension.cc (right):

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
chrome/common/extensions/extension.cc:2337:
browser_action_->SetTitle(ExtensionAction::kDefaultTabId, name());
If there's no icon, do we currently display the puzzle piece?

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
chrome/common/extensions/extension.cc:3098: !is_app()) {
Consider pulling these checks into GenerateBrowserAction.

[not at google - send to devlin, 2012-04-30 03:18:01]

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
File chrome/common/extensions/extension.cc (right):

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
chrome/common/extensions/extension.cc:2337:
browser_action_->SetTitle(ExtensionAction::kDefaultTabId, name());
On 2012/04/30 02:37:04, Aaron Boodman wrote:
> If there's no icon, do we currently display the puzzle piece?

We sure do! I was happy to find this.

https://chromiumcodereview.appspot.com/10264004/diff/2001/chrome/common/exten...
chrome/common/extensions/extension.cc:3098: !is_app()) {
On 2012/04/30 02:37:04, Aaron Boodman wrote:
> Consider pulling these checks into GenerateBrowserAction.

Yeah, nicer.

[Aaron Boodman, 2012-04-30 03:27:21]

still lgtm

https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
File chrome/common/extensions/extension.cc (right):

https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
chrome/common/extensions/extension.cc:2335: void
Extension::GenerateBrowserActionIfPossible() {
IfNecessary?

[not at google - send to devlin, 2012-04-30 04:48:22]

https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
File chrome/common/extensions/extension.cc (right):

https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
chrome/common/extensions/extension.cc:2335: void
Extension::GenerateBrowserActionIfPossible() {
On 2012/04/30 03:27:22, Aaron Boodman wrote:
> IfNecessary?

Well, IfPossible is I think more accurate. I like the

if (!browser_action)
  GenerateBrowserAction();

logic rather than just always calling "GenerateBrowserActionIfNecessary()".  And
yeah, the only reason why a browser action wouldn't be generated is if they
aren't supported by the extension type, hence "IfPossible".  Actually I could
have left that suffix off entirely, it's kind of an implementation detail.

[Aaron Boodman, 2012-04-30 17:27:34]

OK

On Sun, Apr 29, 2012 at 9:48 PM,  <kalman@chromium.org> wrote:
>
>
https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
> File chrome/common/extensions/extension.cc (right):
>
>
https://chromiumcodereview.appspot.com/10264004/diff/4002/chrome/common/exten...
> chrome/common/extensions/extension.cc:2335: void
> Extension::GenerateBrowserActionIfPossible() {
> On 2012/04/30 03:27:22, Aaron Boodman wrote:
>>
>> IfNecessary?
>
>
> Well, IfPossible is I think more accurate. I like the
>
> if (!browser_action)
>  GenerateBrowserAction();
>
> logic rather than just always calling
> "GenerateBrowserActionIfNecessary()".  And yeah, the only reason why a
> browser action wouldn't be generated is if they aren't supported by the
> extension type, hence "IfPossible".  Actually I could have left that
> suffix off entirely, it's kind of an implementation detail.
>
> https://chromiumcodereview.appspot.com/10264004/

[Evan Stade, 2012-04-30 18:06:23]

lgtm

[commit-bot: I haz the power, 2012-04-30 23:28:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10264004/10002

[commit-bot: I haz the power, 2012-05-01 01:22:00]

Change committed as 134657