Enable extension APIs for content scripts.

Enable extension APIs for content scripts.

This only enables extension functions. I will do events in a
separate change.

BUG=81760, 80308
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109812

[Matt Perry, 2011-11-11 20:07:19]

yay

http://codereview.chromium.org/8540012/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

http://codereview.chromium.org/8540012/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_function_dispatcher.cc:656: if
(!process_map.Contains(extension->id(), requesting_process_id))
did you mean to remove this?

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.cc (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.cc:29:
base::JSONReader::ReadAndReturnError(
since this is used in the renderer, doesn't this mean we're now loading/parsing
extension_api.json twice for each process (at least twice, since it's done in JS
for each context)? the file is about 100K so it's not insignificant. do you plan
on getting rid of the parsing from JS contexts somehow?

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.h (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:22: class ExtensionAPI {
please add a class comment

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:26: bool
IsFullNameUnprivileged(const std::string& name) const;
some method comments too

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:26: bool
IsFullNameUnprivileged(const std::string& name) const;
this name leads to double negatives (i.e. !IsUnprivileged). i think it would
read better as IsPrivileged.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:31: ExtensionAPI();
declare a destructor and implement it in the .cc file:
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/extens...
File chrome/common/extensions/extension_messages.h (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/extens...
chrome/common/extensions/extension_messages.h:34: // extension url. Extension
requests can also original from content scripts,
originate*

http://codereview.chromium.org/8540012/diff/1/chrome/renderer/resources/exten...
File chrome/renderer/resources/extensions/extension_process_bindings.js (right):

http://codereview.chromium.org/8540012/diff/1/chrome/renderer/resources/exten...
chrome/renderer/resources/extensions/extension_process_bindings.js:1: //
Copyright (c) 2011 The Chromium Authors. All rights reserved.
this file needs a rename now!

[Aaron Boodman, 2011-11-11 23:12:02]

http://codereview.chromium.org/8540012/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

http://codereview.chromium.org/8540012/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_function_dispatcher.cc:656: if
(!process_map.Contains(extension->id(), requesting_process_id))
On 2011/11/11 20:07:19, Matt Perry wrote:
> did you mean to remove this?

Yes, merge failure. Thanks.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.cc (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.cc:29:
base::JSONReader::ReadAndReturnError(
On 2011/11/11 20:07:19, Matt Perry wrote:
> since this is used in the renderer, doesn't this mean we're now
loading/parsing
> extension_api.json twice for each process (at least twice, since it's done in
JS
> for each context)? the file is about 100K so it's not insignificant. do you
plan
> on getting rid of the parsing from JS contexts somehow?

In the latest patch, I've changed the place where we use extension_api.json in
JS to cache a single JavaScript object across an entire renderer process.

I'd like to also split apart these gigantic files, but that is a separate task.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.h (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:22: class ExtensionAPI {
On 2011/11/11 20:07:19, Matt Perry wrote:
> please add a class comment

Done.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:26: bool
IsFullNameUnprivileged(const std::string& name) const;
On 2011/11/11 20:07:19, Matt Perry wrote:
> some method comments too

Done.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:26: bool
IsFullNameUnprivileged(const std::string& name) const;
On 2011/11/11 20:07:19, Matt Perry wrote:
> this name leads to double negatives (i.e. !IsUnprivileged). i think it would
> read better as IsPrivileged.

Yeah, agree, but the default is privileged, so I think it makes the
implementation clearer to leave it as-is.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:31: ExtensionAPI();
On 2011/11/11 20:07:19, Matt Perry wrote:
> declare a destructor and implement it in the .cc file:
>
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

Done.

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/extens...
File chrome/common/extensions/extension_messages.h (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/extens...
chrome/common/extensions/extension_messages.h:34: // extension url. Extension
requests can also original from content scripts,
On 2011/11/11 20:07:19, Matt Perry wrote:
> originate*

Done.

http://codereview.chromium.org/8540012/diff/1/chrome/renderer/resources/exten...
File chrome/renderer/resources/extensions/extension_process_bindings.js (right):

http://codereview.chromium.org/8540012/diff/1/chrome/renderer/resources/exten...
chrome/renderer/resources/extensions/extension_process_bindings.js:1: //
Copyright (c) 2011 The Chromium Authors. All rights reserved.
On 2011/11/11 20:07:19, Matt Perry wrote:
> this file needs a rename now!

Done.

[Matt Perry, 2011-11-11 23:35:23]

lgtm

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.cc (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.cc:29:
base::JSONReader::ReadAndReturnError(
On 2011/11/11 23:12:02, Aaron Boodman wrote:
> On 2011/11/11 20:07:19, Matt Perry wrote:
> > since this is used in the renderer, doesn't this mean we're now
> loading/parsing
> > extension_api.json twice for each process (at least twice, since it's done
in
> JS
> > for each context)? the file is about 100K so it's not insignificant. do you
> plan
> > on getting rid of the parsing from JS contexts somehow?
> 
> In the latest patch, I've changed the place where we use extension_api.json in
> JS to cache a single JavaScript object across an entire renderer process.
> 
> I'd like to also split apart these gigantic files, but that is a separate
task.

awesome

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.h (right):

http://codereview.chromium.org/8540012/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.h:26: bool
IsFullNameUnprivileged(const std::string& name) const;
On 2011/11/11 23:12:02, Aaron Boodman wrote:
> On 2011/11/11 20:07:19, Matt Perry wrote:
> > this name leads to double negatives (i.e. !IsUnprivileged). i think it would
> > read better as IsPrivileged.
> 
> Yeah, agree, but the default is privileged, so I think it makes the
> implementation clearer to leave it as-is.

meh, i don't think the default really matters to consumers. if the default for
View is visible, should it have an is_invisible() accessor?

[Aaron Boodman, 2011-11-11 23:55:03]

On 2011/11/11 23:35:23, Matt Perry wrote:
> meh, i don't think the default really matters to consumers. if the default for
> View is visible, should it have an is_invisible() accessor?

You're a hard taskmaster, Mr Perry. Fixed.

[Matt Perry, 2011-11-12 00:24:15]

On 2011/11/11 23:55:03, Aaron Boodman wrote:
> On 2011/11/11 23:35:23, Matt Perry wrote:
> > meh, i don't think the default really matters to consumers. if the default
for
> > View is visible, should it have an is_invisible() accessor?
> 
> You're a hard taskmaster, Mr Perry. Fixed.

I'd probably change IsChildNameUnprivileged as well, but whatever :)

still LGTM

[Aaron Boodman, 2011-11-12 00:44:06]

On 2011/11/12 00:24:15, Matt Perry wrote:
> On 2011/11/11 23:55:03, Aaron Boodman wrote:
> > On 2011/11/11 23:35:23, Matt Perry wrote:
> > > meh, i don't think the default really matters to consumers. if the default
> for
> > > View is visible, should it have an is_invisible() accessor?
> > 
> > You're a hard taskmaster, Mr Perry. Fixed.
> 
> I'd probably change IsChildNameUnprivileged as well, but whatever :)
> 
> still LGTM

Yeah, not sure what I was thinking there.

[Matt Perry, 2011-11-12 01:26:10]

lgtm

[commit-bot: I haz the power, 2011-11-12 17:34:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/8540012/5007

[commit-bot: I haz the power, 2011-11-12 17:35:58]

Can't apply patch for file chrome/browser/autocomplete/autocomplete_match.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/autocomplete/autocomplete_match.cc
Hunk #1 FAILED at 160.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/autocomplete/autocomplete_match.cc.rej

[commit-bot: I haz the power, 2011-11-12 22:01:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/8540012/9002

[commit-bot: I haz the power, 2011-11-12 22:02:06]

Presubmit check for 8540012-9002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2011 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.\n
Found a bad license header in these files:
 
chrome/test/data/extensions/api_test/content_scripts/extension_api/functions.js
\
  chrome/test/data/extensions/api_test/stubs/content_script.js

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
content/renderer/v8_value_converter_impl.cc,content/renderer/v8_value_converter_impl.h,content/public/renderer/v8_value_converter.h

Presubmit checks took 4.3s to calculate.