Expose Extension Bindings to Component Applications

Expose Extension Bindings to Component Applications

This patch allows component (built-in) extension apps to have extension api bindings.

Note that this patch adds browser-side api permission checking for extension requests.
 
This is step two along the path to exposing an extension management api to the gallery (webstore).

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

[Matt Perry, 2010-08-26 22:10:43]

LGTM with a few small bits

http://codereview.chromium.org/3163044/diff/2001/3004
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

http://codereview.chromium.org/3163044/diff/2001/3004#newcode341
chrome/browser/extensions/extension_function_dispatcher.cc:341: extension =
service->GetExtensionByWebExtent(url);
I'm running into this problem in my own patch. I wonder if GetByURL should just
handle it. (no need to do anything, just thinking out loud)

http://codereview.chromium.org/3163044/diff/2001/3017
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/3163044/diff/2001/3017#newcode129
chrome/common/extensions/extension.h:129: static const size_t
kNumNonPermissionModuleNames;
looks like none of these are actually referenced outside extension.cc, which
means you can just make them non-class statics in the .cc file.

http://codereview.chromium.org/3163044/diff/2001/3021
File chrome/renderer/extensions/extension_process_bindings.cc (right):

http://codereview.chromium.org/3163044/diff/2001/3021#newcode1
chrome/renderer/extensions/extension_process_bindings.cc:1: // Copyright (c)
2010 The Chromium Authors. All rights reserved.
Nice cleanup here.

http://codereview.chromium.org/3163044/diff/2001/3024
File chrome/renderer/render_thread.cc (right):

http://codereview.chromium.org/3163044/diff/2001/3024#newcode867
chrome/renderer/render_thread.cc:867: // TODO(rafaelw). Note that
extension-related v8 extensions are being
can you file a bug and link it in the comment?

[rafaelw, 2010-08-27 18:30:38]

Note that I decided to just DISABLE ContentScriptExtensionIframe until we can
implement the v8proxy delegate. enabling bindings for is_extension_process_ ||
scheme = "chrome-extension" didn't work and I suspect that tracking down why may
be more work than just implementing the v8proxy delegate.

http://codereview.chromium.org/3163044/diff/2001/3004
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

http://codereview.chromium.org/3163044/diff/2001/3004#newcode341
chrome/browser/extensions/extension_function_dispatcher.cc:341: extension =
service->GetExtensionByWebExtent(url);
Yeah. I agree. I actually looked at this. The majority of the cases, we now have
this basic pattern. There are a few stragglers, though.

On 2010/08/26 22:10:44, Matt Perry wrote:
> I'm running into this problem in my own patch. I wonder if GetByURL should
just
> handle it. (no need to do anything, just thinking out loud)

http://codereview.chromium.org/3163044/diff/2001/3017
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/3163044/diff/2001/3017#newcode129
chrome/common/extensions/extension.h:129: static const size_t
kNumNonPermissionModuleNames;
On 2010/08/26 22:10:44, Matt Perry wrote:
> looks like none of these are actually referenced outside extension.cc, which
> means you can just make them non-class statics in the .cc file.

Done.

http://codereview.chromium.org/3163044/diff/2001/3024
File chrome/renderer/render_thread.cc (right):

http://codereview.chromium.org/3163044/diff/2001/3024#newcode867
chrome/renderer/render_thread.cc:867: // TODO(rafaelw). Note that
extension-related v8 extensions are being
Done. crbug.com/53610

On 2010/08/26 22:10:44, Matt Perry wrote:
> can you file a bug and link it in the comment?

[Matt Perry, 2010-08-27 20:50:46]

On Fri, Aug 27, 2010 at 11:30 AM, <[email protected]> wrote:

> Note that I decided to just DISABLE ContentScriptExtensionIframe until we
> can
> implement the v8proxy delegate. enabling bindings for is_extension_process_
> ||
> scheme = "chrome-extension" didn't work and I suspect that tracking down
> why may
> be more work than just implementing the v8proxy delegate.


Does that mean http://code.google.com/p/chromium/issues/detail?id=37920 will
regress?

[rafaelw1, 2010-08-27 21:01:54]

I think we can fix it post branch before it goes out.

On Fri, Aug 27, 2010 at 1:50 PM, Matt Perry <[email protected]> wrote:
> On Fri, Aug 27, 2010 at 11:30 AM, <[email protected]> wrote:
>>
>> Note that I decided to just DISABLE ContentScriptExtensionIframe until we
>> can
>> implement the v8proxy delegate. enabling bindings for
>> is_extension_process_ ||
>> scheme = "chrome-extension" didn't work and I suspect that tracking down
>> why may
>> be more work than just implementing the v8proxy delegate.
>
> Does that mean http://code.google.com/p/chromium/issues/detail?id=37920 will
> regress?