Hook the activeTab permission up to the omnibox, page action, and context menu APIs.

Hook the activeTab permission up to the omnibox, page action, and context menu APIs.


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

[not at google - send to devlin, 2012-06-13 20:56:09]

This is everything proposed, apart from keyboard shortcuts.

@finnur I've CC'ed you for the keyboard shortcut stuff, which I haven't done yet
because it will be slightly more involved, and since you're in a wildly
different timezone I'll do that in a separate patch. Basically I think that
those ExtensionKeybindingRegistry* classes will need to take a Browser* not a
Profile* and pass that through to ExtensionBrowserEventRouter so that it can
call GrantIfRequested (phew). ExtensionBrowserEventRouter kinda feels like the
wrong place to do that, but it's less code to duplicate on each platform.

[Finnur, 2012-06-14 10:53:00]

> @finnur I've CC'ed you for the keyboard shortcut stuff, 
> which I haven't done yet because it will be slightly 
> more involved, and since you're in a wildly different 
> timezone I'll do that in a separate patch.

Thanks.

One nit...

https://chromiumcodereview.appspot.com/10535155/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/active_tab_permission_manager.h (right):

https://chromiumcodereview.appspot.com/10535155/diff/2001/chrome/browser/exte...
chrome/browser/extensions/active_tab_permission_manager.h:37: void
GrantIfRequested(const std::string& extension_id);
I think the style guide doesn't like two functions using the same name when it
takes different params (exceptions made for simple cases like char* and
std::string). Maybe GrantByIdIfRequested?

[Aaron Boodman, 2012-06-14 20:50:44]

lgtm

http://codereview.chromium.org/10535155/diff/2001/chrome/browser/extensions/a...
File chrome/browser/extensions/active_tab_permission_manager.h (right):

http://codereview.chromium.org/10535155/diff/2001/chrome/browser/extensions/a...
chrome/browser/extensions/active_tab_permission_manager.h:37: void
GrantIfRequested(const std::string& extension_id);
On 2012/06/14 10:53:00, Finnur wrote:
> I think the style guide doesn't like two functions using the same name when it
> takes different params (exceptions made for simple cases like char* and
> std::string). Maybe GrantByIdIfRequested?

I think the styleguide allows for overloading in this way when the different
types are for convenience and the methods mean the same thing.

[commit-bot: I haz the power, 2012-06-14 20:57:41]

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

[commit-bot: I haz the power, 2012-06-14 20:57:46]

Failed to apply patch for chrome/browser/extensions/extension_menu_manager.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/extensions/extension_menu_manager.cc
Hunk #1 FAILED at 17.
Hunk #2 succeeded at 568 (offset 35 lines).
Hunk #3 succeeded at 618 (offset 35 lines).
Hunk #4 FAILED at 602.
2 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/extensions/extension_menu_manager.cc.rej

[commit-bot: I haz the power, 2012-06-14 21:24:43]

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

[commit-bot: I haz the power, 2012-06-14 21:46:54]

Try job failure for 10535155-7001 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-14 22:29:37]

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

[commit-bot: I haz the power, 2012-06-14 22:59:26]

Try job failure for 10535155-8014 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-19 18:39:02]

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

[commit-bot: I haz the power, 2012-06-19 19:32:04]

Try job failure for 10535155-7004 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-19 22:10:36]

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

[commit-bot: I haz the power, 2012-06-19 22:30:03]

Try job failure for 10535155-20003 (retry) on mac_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-20 00:38:39]

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

[commit-bot: I haz the power, 2012-06-20 03:20:21]

Change committed as 143134