Extensions: run "custom bindings" v8-extensions in content scripts.

chrome/renderer/extensions/extension_dispatcher.cc

Index: chrome/renderer/extensions/extension_dispatcher.cc
diff --git a/chrome/renderer/extensions/extension_dispatcher.cc b/chrome/renderer/extensions/extension_dispatcher.cc
index 0d4169b122b8151664ef23175f5f8936c34056be..b7156c6c8b195442143a367edc4b19726f6c1bb2 100644
--- a/chrome/renderer/extensions/extension_dispatcher.cc
+++ b/chrome/renderer/extensions/extension_dispatcher.cc
@@ -272,11 +272,24 @@ bool ExtensionDispatcher::AllowScriptExtension(
   return AllowScriptExtension(frame, v8_extension_name, extension_group, 0);
 }
 
+namespace {
+
+// This is what the extension_group variable will be when DidCreateScriptContext
+// is called. We know because it's the same as what AllowScriptExtension gets
+// passed, and the two functions are called sequentially from WebKit.
+//
+// There is a patch in progress (koz) to properly plumb this through.

[koz (OOO until 15th September), 2012/02/15 23:43:11]

TODO(koz): Plumb extension_group through to AllowScriptExtension() from WebKit.

[not at google - send to devlin, 2012/02/16 00:06:27]

Done.

+static int hack_DidCreateScriptContext_extension_group = 0;
+
+}
+
 bool ExtensionDispatcher::AllowScriptExtension(
     WebFrame* frame,
     const std::string& v8_extension_name,
     int extension_group,
     int world_id) {
+  hack_DidCreateScriptContext_extension_group = extension_group;
+
   // NULL in unit tests.
   if (!RenderThread::Get())
     return true;
@@ -291,7 +304,8 @@ bool ExtensionDispatcher::AllowScriptExtension(
 
   // Extension-only bindings should be restricted to content scripts and
   // extension-blessed URLs.
-  if (extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS ||
+  bool is_content_script = extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS;
+  if (is_content_script ||
       extensions_.ExtensionBindingsAllowed(ExtensionURLInfo(
           frame->document().securityOrigin(),
           UserScriptSlave::GetDataSourceURLForFrame(frame)))) {
@@ -315,7 +329,7 @@ bool ExtensionDispatcher::AllowScriptExtension(
         return false;
       }
       return custom_bindings_util::AllowAPIInjection(
-          custom_binding_api_name, *extension);
+          custom_binding_api_name, *extension, is_content_script);

[koz (OOO until 15th September), 2012/02/15 23:43:11]

(aside) It'll be nice when we can factor this logic out so it becomes possible
to write tests like this:

CheckInjection(CONTENT_SCRIPT, NON_BLESSED_URL,
&apis_extension_has_permission_for, "storage", false);

Which would enable you to test the plugging up done by this change.

[not at google - send to devlin, 2012/02/16 00:06:27]

Yeah I'm hoping/expecting that once your require() change(s) go in we'll be able
to centralise the logic here and in SchemaGeneratedBindings (which are quite
simple, but I couldn't come up with a convenient/sensible way to share it with
the current structure of the code, and it's not worth refactoring since it's all
about to change soon anyway).

     }
 
     return true;
@@ -326,8 +340,15 @@ bool ExtensionDispatcher::AllowScriptExtension(
 
 void ExtensionDispatcher::DidCreateScriptContext(
     WebFrame* frame, v8::Handle<v8::Context> v8_context, int world_id) {
+  bool is_content_script = (hack_DidCreateScriptContext_extension_group ==
+      EXTENSION_GROUP_CONTENT_SCRIPTS);
+
   ChromeV8Context* context =
-      new ChromeV8Context(v8_context, frame, GetExtensionID(frame, world_id));
+      new ChromeV8Context(
+          v8_context,
+          frame,
+          GetExtensionID(frame, world_id),
+          is_content_script);
   v8_context_set_.Add(context);
 
   const Extension* extension = extensions_.GetByID(context->extension_id());