Make the getAttention badge grey.

Make the getAttention badge grey.

The grey badge fades in using the existing animation, and when the
user clicks on it, it snaps to the full-color version, without an
animation.

Also avoided granting access on right clicks, which I noticed because
of the color change.

This change has the side-effect that pageaction.show() will cause the
badge to fade in.

This doesn't yet show the grey background, as that takes
platform-specific code.

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

[not at google - send to devlin, 2012-08-02 15:38:09]

lgtm

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/script_badge_controller.cc (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
chrome/browser/extensions/script_badge_controller.cc:71: // Don't grant access
on right clicks, so users can investigate
can you move this inside the switch below? That was the idea of it.

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
chrome/browser/extensions/script_badge_controller.cc:205: 
nit: 1 less \n (not yours I know)

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_action.h (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_action.h:52: GETTING_ATTENTION,
tbh I would have called this "WANTS_ATTENTION" because "GETTING_ATTENTION" could
mean like, what happens if somebody is giving the extension attention.

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_action_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_action_unittest.cc:146:
ASSERT_FALSE(action.GetIsVisible(1));
why are some of these ASSERT and others EXPECT? Seems like they could all be
EXPECT.

[Jeffrey Yasskin, 2012-08-02 15:55:56]

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/script_badge_controller.cc (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
chrome/browser/extensions/script_badge_controller.cc:71: // Don't grant access
on right clicks, so users can investigate
On 2012/08/02 15:38:09, kalman wrote:
> can you move this inside the switch below? That was the idea of it.

I would have done that, except it conflicts with the different behavior of left
vs middle click. That's a bug, of course, but I need to check whether we want to
throw out the scriptBadge.onClicked event entirely, and I probably need to fix
the popup bug before doing that...

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
chrome/browser/extensions/script_badge_controller.cc:205: 
On 2012/08/02 15:38:09, kalman wrote:
> nit: 1 less \n (not yours I know)

Done.

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_action.h (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_action.h:52: GETTING_ATTENTION,
On 2012/08/02 15:38:09, kalman wrote:
> tbh I would have called this "WANTS_ATTENTION" because "GETTING_ATTENTION"
could
> mean like, what happens if somebody is giving the extension attention.

Sure.

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_action_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_action_unittest.cc:146:
ASSERT_FALSE(action.GetIsVisible(1));
On 2012/08/02 15:38:09, kalman wrote:
> why are some of these ASSERT and others EXPECT? Seems like they could all be
> EXPECT.

Some were copied from the previous test. They're all EXPECTs now.

[Jeffrey Yasskin, 2012-08-03 11:11:30]

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/script_badge_controller.cc (right):

https://chromiumcodereview.appspot.com/10823143/diff/1/chrome/browser/extensi...
chrome/browser/extensions/script_badge_controller.cc:71: // Don't grant access
on right clicks, so users can investigate
On 2012/08/02 15:55:56, Jeffrey Yasskin wrote:
> On 2012/08/02 15:38:09, kalman wrote:
> > can you move this inside the switch below? That was the idea of it.
> 
> I would have done that, except it conflicts with the different behavior of
left
> vs middle click. That's a bug, of course, but I need to check whether we want
to
> throw out the scriptBadge.onClicked event entirely, and I probably need to fix
> the popup bug before doing that...

Done now, by having both mouse buttons send the onClicked and show the popup.
I'll need to fix the actual logic for onClicked vs popup in another CL.

[commit-bot: I haz the power, 2012-08-03 11:11:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jyasskin@chromium.org/10823143/10001

[commit-bot: I haz the power, 2012-08-03 12:53:01]

Change committed as 149846

[Timur Iskhodzhanov, 2012-08-03 16:31:48]

Jeffrey,
Can you please take a look at the Valgrind reports that appeared on the bots
after the commit ?