Ensure that "Always Run" context menu item dissapears upon granting

Ensure that "Always Run" context menu item dissapears upon granting
script injection permissions

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

[gpdavis, 2014-08-20 20:16:01]

Looks like this simple change fixed the issue.  The extension's entry in
active_script_actions_ is never removed, but the entry in pending_requests_ is. 
This appears to behave as expected.

[not at google - send to devlin, 2014-08-20 20:27:07]

https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
File chrome/browser/extensions/active_script_controller.cc (right):

https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
chrome/browser/extensions/active_script_controller.cc:142: return enabled_ &&
pending_requests_.count(extension->id()) > 0;
active_script_actions_ seems like the right check to me? Is something else going
wrong?

I'm going to give this to Devlin, I need to finish something else off.

[gpdavis, 2014-08-21 21:32:07]

On 2014/08/20 20:27:07, kalman wrote:
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> File chrome/browser/extensions/active_script_controller.cc (right):
> 
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> chrome/browser/extensions/active_script_controller.cc:142: return enabled_ &&
> pending_requests_.count(extension->id()) > 0;
> active_script_actions_ seems like the right check to me? Is something else
going
> wrong?
> 
> I'm going to give this to Devlin, I need to finish something else off.

Ping! :)

[gpdavis, 2014-08-21 21:39:11]

https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
File chrome/browser/extensions/active_script_controller.cc (right):

https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
chrome/browser/extensions/active_script_controller.cc:160:
active_script_actions_[extension->id()] = action;
active_script_actions_ is only manipulated in this method.

Codesearch is not working properly, but the comment for active_script_actions_
says:
// Script badges that have been generated for extensions. This is both those
// with actions already declared that are copied and normalised, and actions
// that get generated for extensions that haven't declared anything.

Seems like we ought to be removing the extension's entry in
active_script_actions_ in OnClicked.  What do you think?

[Devlin, 2014-08-21 21:40:30]

lgtm

[Devlin, 2014-08-21 21:42:04]

On 2014/08/21 21:39:11, gpdavis wrote:
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> File chrome/browser/extensions/active_script_controller.cc (right):
> 
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> chrome/browser/extensions/active_script_controller.cc:160:
> active_script_actions_[extension->id()] = action;
> active_script_actions_ is only manipulated in this method.
> 
> Codesearch is not working properly, but the comment for active_script_actions_
> says:
> // Script badges that have been generated for extensions. This is both those
> // with actions already declared that are copied and normalised, and actions
> // that get generated for extensions that haven't declared anything.
> 
> Seems like we ought to be removing the extension's entry in
> active_script_actions_ in OnClicked.  What do you think?

No, we shouldn't remove the extension's entry in OnClicked(), since OnClicked()
may be for one page only.  The chances that an extension that requests all urls
only wants to execute once is pretty slim.

[gpdavis, 2014-08-21 21:43:51]

On 2014/08/21 21:42:04, Devlin wrote:
> On 2014/08/21 21:39:11, gpdavis wrote:
> >
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> > File chrome/browser/extensions/active_script_controller.cc (right):
> > 
> >
>
https://codereview.chromium.org/492803003/diff/1/chrome/browser/extensions/ac...
> > chrome/browser/extensions/active_script_controller.cc:160:
> > active_script_actions_[extension->id()] = action;
> > active_script_actions_ is only manipulated in this method.
> > 
> > Codesearch is not working properly, but the comment for
active_script_actions_
> > says:
> > // Script badges that have been generated for extensions. This is both those
> > // with actions already declared that are copied and normalised, and actions
> > // that get generated for extensions that haven't declared anything.
> > 
> > Seems like we ought to be removing the extension's entry in
> > active_script_actions_ in OnClicked.  What do you think?
> 
> No, we shouldn't remove the extension's entry in OnClicked(), since
OnClicked()
> may be for one page only.  The chances that an extension that requests all
urls
> only wants to execute once is pretty slim.

Ah, I see.  Alright.

[gpdavis, 2014-08-21 21:43:54]

The CQ bit was checked by gpdavis.chromium@gmail.com

[commit-bot: I haz the power, 2014-08-21 21:45:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gpdavis.chromium@gmail.com/492803003/1

[commit-bot: I haz the power, 2014-08-21 23:13:02]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[commit-bot: I haz the power, 2014-08-22 01:15:33]

Committed patchset #1 (1) as 291291