Add an API to manipulate the browser action badge.

Add an API to manipulate the browser action badge.

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

[Matt Perry, 2009-10-01 23:17:07]

http://codereview.chromium.org/256032/diff/1/2
File chrome/browser/extensions/extension_browser_actions_api.cc (right):

http://codereview.chromium.org/256032/diff/1/2#newcode65
Line 65: bool BrowserActionSetBadgeTextFunction::RunImpl() {
Is it time to extract a common base class?
  setup;
  ExecuteFunction(extension);
  notify;

http://codereview.chromium.org/256032/diff/1/5
File chrome/browser/views/browser_actions_container.cc (right):

http://codereview.chromium.org/256032/diff/1/5#newcode258
Line 258: std::string text = view->browser_action_state()->badge_text();
nit: const std::string&

http://codereview.chromium.org/256032/diff/1/6
File chrome/browser/views/browser_actions_container.h (right):

http://codereview.chromium.org/256032/diff/1/6#newcode18
Line 18: class TextButton;
nit: this can go away

http://codereview.chromium.org/256032/diff/1/6#newcode21
Line 21: class BrowserActionImageView;
alphabetize

http://codereview.chromium.org/256032/diff/1/8
File chrome/common/extensions/extension_action.h (right):

http://codereview.chromium.org/256032/diff/1/8#newcode14
Line 14: #include "third_party/skia/include/core/SkColor.h"
is it OK to be using skia types here? do we have a color abstraction we should
use instead?

[Aaron Boodman, 2009-10-01 23:35:59]

http://codereview.chromium.org/256032/diff/1/2
File chrome/browser/extensions/extension_browser_actions_api.cc (right):

http://codereview.chromium.org/256032/diff/1/2#newcode65
Line 65: bool BrowserActionSetBadgeTextFunction::RunImpl() {
On 2009/10/01 23:17:07, Matt Perry wrote:
> Is it time to extract a common base class?
>   setup;
>   ExecuteFunction(extension);
>   notify;

meh

http://codereview.chromium.org/256032/diff/1/5
File chrome/browser/views/browser_actions_container.cc (right):

http://codereview.chromium.org/256032/diff/1/5#newcode258
Line 258: std::string text = view->browser_action_state()->badge_text();
On 2009/10/01 23:17:07, Matt Perry wrote:
> nit: const std::string&

Really? The function returns a copy, not a ref, so declaring this reference
wouldn't do anything different.

http://codereview.chromium.org/256032/diff/1/6
File chrome/browser/views/browser_actions_container.h (right):

http://codereview.chromium.org/256032/diff/1/6#newcode18
Line 18: class TextButton;
On 2009/10/01 23:17:07, Matt Perry wrote:
> nit: this can go away

Done.

http://codereview.chromium.org/256032/diff/1/6#newcode21
Line 21: class BrowserActionImageView;
On 2009/10/01 23:17:07, Matt Perry wrote:
> alphabetize

Done.

http://codereview.chromium.org/256032/diff/1/8
File chrome/common/extensions/extension_action.h (right):

http://codereview.chromium.org/256032/diff/1/8#newcode14
Line 14: #include "third_party/skia/include/core/SkColor.h"
On 2009/10/01 23:17:07, Matt Perry wrote:
> is it OK to be using skia types here? do we have a color abstraction we should
> use instead?

I do not see any color abstraction in base/gfx. I will try it and see if deps
complains.

[Matt Perry, 2009-10-01 23:38:21]

http://codereview.chromium.org/256032/diff/1/2
File chrome/browser/extensions/extension_browser_actions_api.cc (right):

http://codereview.chromium.org/256032/diff/1/2#newcode65
Line 65: bool BrowserActionSetBadgeTextFunction::RunImpl() {
On 2009/10/01 23:35:59, Aaron Boodman wrote:
> On 2009/10/01 23:17:07, Matt Perry wrote:
> > Is it time to extract a common base class?
> >   setup;
> >   ExecuteFunction(extension);
> >   notify;
> 
> meh

:'(

http://codereview.chromium.org/256032/diff/1/5
File chrome/browser/views/browser_actions_container.cc (right):

http://codereview.chromium.org/256032/diff/1/5#newcode258
Line 258: std::string text = view->browser_action_state()->badge_text();
On 2009/10/01 23:35:59, Aaron Boodman wrote:
> On 2009/10/01 23:17:07, Matt Perry wrote:
> > nit: const std::string&
> 
> Really? The function returns a copy, not a ref, so declaring this reference
> wouldn't do anything different.

Well, have it return a ref then :)

http://codereview.chromium.org/256032/diff/18/4013
File chrome/common/extensions/extension_action.h (right):

http://codereview.chromium.org/256032/diff/18/4013#newcode86
Line 86: std::string badge_text() const { return badge_text_; }
return const ref

[Aaron Boodman, 2009-10-02 00:57:44]

Ok, I'm meeting you halfway. const ref, but the silly class that abstracts two
lines I could not abide.

[Matt Perry, 2009-10-02 00:59:04]

LGTM