Refactoring: de-couple Extensions from "script injection System" [render side] : 1

extensions/renderer/script_injection.cc

Index: extensions/renderer/script_injection.cc
diff --git a/extensions/renderer/script_injection.cc b/extensions/renderer/script_injection.cc
index ea588fe90a47b20b2353411dc9ede8a463d6fc84..5ab43b8646e54b19ebd404d58e966ab0a2cdf743 100644
--- a/extensions/renderer/script_injection.cc
+++ b/extensions/renderer/script_injection.cc
@@ -17,6 +17,7 @@
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_handlers/csp_info.h"
 #include "extensions/renderer/dom_activity_logger.h"
+#include "extensions/renderer/extension_consumer.h"
 #include "extensions/renderer/extension_groups.h"
 #include "extensions/renderer/extensions_renderer_client.h"
 #include "third_party/WebKit/public/platform/WebString.h"
@@ -31,7 +32,8 @@ namespace extensions {
 
 namespace {
 
-typedef std::map<std::string, int> IsolatedWorldMap;
+using IsolatedWorldKey = std::pair<HostID, int>;
+using IsolatedWorldMap = std::map<IsolatedWorldKey, int>;
 base::LazyInstance<IsolatedWorldMap> g_isolated_worlds =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -55,38 +57,42 @@ void AppendAllChildFrames(blink::WebFrame* parent_frame,
   }
 }
 
-// Gets the isolated world ID to use for the given |extension| in the given
-// |frame|. If no isolated world has been created for that extension,
-// one will be created and initialized.
-int GetIsolatedWorldIdForExtension(const Extension* extension,
-                                   blink::WebLocalFrame* frame) {
+// Gets the isolated world ID to use for the given |host, instance_id|
+// in the given |frame|. If no isolated world has been created for that
+// |host, instance_id| one will be created and initialized.
+int GetIsolatedWorldIdForInstance(const Host* host,
+                                  int instance_id,
+                                  blink::WebLocalFrame* frame) {
   static int g_next_isolated_world_id =
       ExtensionsRendererClient::Get()->GetLowestIsolatedWorldId();
 
   IsolatedWorldMap& isolated_worlds = g_isolated_worlds.Get();
 
   int id = 0;
-  IsolatedWorldMap::iterator iter = isolated_worlds.find(extension->id());
+  const HostID& host_id = host->id();

[Devlin, 2015/02/09 17:40:25]

This cached variable is kinda silly.  Just call host->id() when you initialize
the key.

[Xi Han, 2015/02/09 23:28:11]

Done.

+  IsolatedWorldKey key(host_id, instance_id);
+  IsolatedWorldMap::iterator iter = isolated_worlds.find(key);
   if (iter != isolated_worlds.end()) {
     id = iter->second;
   } else {
     id = g_next_isolated_world_id++;
     // This map will tend to pile up over time, but realistically, you're never
-    // going to have enough extensions for it to matter.
-    isolated_worlds[extension->id()] = id;
+    // going to have enough hosts for it to matter.
+    isolated_worlds[key] = id;
   }
 
+  std::string name = host->name();
+  const GURL& origin = host->url();

[Devlin, 2015/02/09 17:40:25]

Same here - just use host->url() on line 91 and host->name() on line 95.

[Xi Han, 2015/02/09 23:28:11]

Legacy code, forgot to clean up:(

+
   // We need to set the isolated world origin and CSP even if it's not a new
   // world since these are stored per frame, and we might not have used this
   // isolated world in this frame before.
   frame->setIsolatedWorldSecurityOrigin(
-      id, blink::WebSecurityOrigin::create(extension->url()));
+      id, blink::WebSecurityOrigin::create(origin));
   frame->setIsolatedWorldContentSecurityPolicy(
-      id,
-      blink::WebString::fromUTF8(CSPInfo::GetContentSecurityPolicy(extension)));
+      id, blink::WebString::fromUTF8(host->GetContentSecurityPolicy()));
   frame->setIsolatedWorldHumanReadableName(
-      id,
-      blink::WebString::fromUTF8(extension->name()));
+      id, blink::WebString::fromUTF8(name));
 
   return id;
 }
@@ -94,33 +100,40 @@ int GetIsolatedWorldIdForExtension(const Extension* extension,
 }  // namespace
 
 // static
-std::string ScriptInjection::GetExtensionIdForIsolatedWorld(
-    int isolated_world_id) {
-  IsolatedWorldMap& isolated_worlds = g_isolated_worlds.Get();
+HostID ScriptInjection::GetHostIdForIsolatedWorld(int isolated_world_id) {
+  const IsolatedWorldMap& isolated_worlds = g_isolated_worlds.Get();
 
-  for (IsolatedWorldMap::iterator iter = isolated_worlds.begin();
-       iter != isolated_worlds.end();
-       ++iter) {
-    if (iter->second == isolated_world_id)
-      return iter->first;
+  for (const auto& kv : isolated_worlds) {
+    if (kv.second == isolated_world_id)
+      return kv.first.first;
   }
-  return std::string();
+  return HostID();
 }
 
 // static
-void ScriptInjection::RemoveIsolatedWorld(const std::string& extension_id) {
-  g_isolated_worlds.Get().erase(extension_id);
+void ScriptInjection::RemoveIsolatedWorld(const HostID& host_id) {
+  std::set<IsolatedWorldKey> keys_to_delete;
+  IsolatedWorldMap& isolated_worlds = g_isolated_worlds.Get();
+  for (auto& kv: isolated_worlds) {

[Devlin, 2015/02/09 17:40:25]

const auto&

[Xi Han, 2015/02/09 23:28:11]

Done.

+    const IsolatedWorldKey& key = kv.first;
+    if (key.first == host_id)
+      keys_to_delete.insert(key);
+  }
+  for (auto& key : keys_to_delete)
+    isolated_worlds.erase(key);
 }
 
 ScriptInjection::ScriptInjection(
     scoped_ptr<ScriptInjector> injector,
     blink::WebLocalFrame* web_frame,
-    const std::string& extension_id,
+    const HostID& host_id,
+    int instance_id,
     UserScript::RunLocation run_location,
     int tab_id)
     : injector_(injector.Pass()),
       web_frame_(web_frame),
-      extension_id_(extension_id),
+      host_id_(host_id),
+      instance_id_(instance_id),
       run_location_(run_location),
       tab_id_(tab_id),
       request_id_(kInvalidRequestId),
@@ -133,7 +146,7 @@ ScriptInjection::~ScriptInjection() {
 }
 
 bool ScriptInjection::TryToInject(UserScript::RunLocation current_location,
-                                  const Extension* extension,
+                                  const Host* host,
                                   ScriptsRunInfo* scripts_run_info) {
   if (current_location < run_location_)
     return false;  // Wait for the right location.
@@ -141,13 +154,13 @@ bool ScriptInjection::TryToInject(UserScript::RunLocation current_location,
   if (request_id_ != kInvalidRequestId)
     return false;  // We're waiting for permission right now, try again later.
 
-  if (!extension) {
+  if (!host) {
     NotifyWillNotInject(ScriptInjector::EXTENSION_REMOVED);
     return true;  // We're done.
   }
 
   switch (injector_->CanExecuteOnFrame(
-      extension, web_frame_, tab_id_, web_frame_->top()->document().url())) {
+      host, web_frame_, tab_id_, web_frame_->top()->document().url())) {
     case PermissionsData::ACCESS_DENIED:
       NotifyWillNotInject(ScriptInjector::NOT_ALLOWED);
       return true;  // We're done.
@@ -155,7 +168,7 @@ bool ScriptInjection::TryToInject(UserScript::RunLocation current_location,
       RequestPermission();
       return false;  // Wait around for permission.
     case PermissionsData::ACCESS_ALLOWED:
-      Inject(extension, scripts_run_info);
+      Inject(host, scripts_run_info);
       return true;  // We're done!
   }
 
@@ -164,14 +177,14 @@ bool ScriptInjection::TryToInject(UserScript::RunLocation current_location,
   return false;
 }
 
-bool ScriptInjection::OnPermissionGranted(const Extension* extension,
+bool ScriptInjection::OnPermissionGranted(const Host* host,
                                           ScriptsRunInfo* scripts_run_info) {
-  if (!extension) {
+  if (!host) {
     NotifyWillNotInject(ScriptInjector::EXTENSION_REMOVED);
     return false;
   }
 
-  Inject(extension, scripts_run_info);
+  Inject(host, scripts_run_info);
   return true;
 }
 
@@ -185,7 +198,7 @@ void ScriptInjection::RequestPermission() {
                                                   : g_next_pending_id++;
   render_view->Send(new ExtensionHostMsg_RequestScriptInjectionPermission(
       render_view->GetRoutingID(),
-      extension_id_,
+      host_id_.id(),
       injector_->script_type(),
       request_id_));
 }
@@ -196,9 +209,9 @@ void ScriptInjection::NotifyWillNotInject(
   injector_->OnWillNotInject(reason);
 }
 
-void ScriptInjection::Inject(const Extension* extension,
+void ScriptInjection::Inject(const Host* host,
                              ScriptsRunInfo* scripts_run_info) {
-  DCHECK(extension);
+  DCHECK(host);
   DCHECK(scripts_run_info);
   DCHECK(!complete_);
 
@@ -229,19 +242,19 @@ void ScriptInjection::Inject(const Extension* extension,
 
     // We recheck access here in the renderer for extra safety against races
     // with navigation, but different frames can have different URLs, and the
-    // extension might only have access to a subset of them.
-    // For child frames, we just skip ones the extension doesn't have access
+    // host might only have access to a subset of them.
+    // For child frames, we just skip ones the host doesn't have access
     // to and carry on.
     // Note: we don't consider ACCESS_WITHHELD because there is nowhere to
     // surface a request for a child frame.
     // TODO(rdevlin.cronin): We should ask for permission somehow.
-    if (injector_->CanExecuteOnFrame(extension, frame, tab_id_, top_url) ==
+    if (injector_->CanExecuteOnFrame(host, frame, tab_id_, top_url) ==
         PermissionsData::ACCESS_DENIED) {
       DCHECK(frame->parent());
       continue;
     }
     if (inject_js)
-      InjectJs(extension, frame, execution_results.get());
+      InjectJs(host, frame, execution_results.get());
     if (inject_css)
       InjectCss(frame);
   }
@@ -252,7 +265,7 @@ void ScriptInjection::Inject(const Extension* extension,
                                  run_location_);
 }
 
-void ScriptInjection::InjectJs(const Extension* extension,
+void ScriptInjection::InjectJs(const Host* host,
                                blink::WebLocalFrame* frame,
                                base::ListValue* execution_results) {
   std::vector<blink::WebScriptSource> sources =
@@ -260,11 +273,11 @@ void ScriptInjection::InjectJs(const Extension* extension,
   bool in_main_world = injector_->ShouldExecuteInMainWorld();
   int world_id = in_main_world
                      ? DOMActivityLogger::kMainWorldId
-                     : GetIsolatedWorldIdForExtension(extension, frame);
+                     : GetIsolatedWorldIdForInstance(host, instance_id_, frame);
   bool expects_results = injector_->ExpectsResults();
 
   base::ElapsedTimer exec_timer;
-  DOMActivityLogger::AttachToWorld(world_id, extension->id());
+  DOMActivityLogger::AttachToWorld(world_id, host->id().id());

[Devlin, 2015/02/09 17:40:25]

We almost certainly don't want activity logger to have anything to do with
webuis' injections.

[Xi Han, 2015/02/09 23:28:11]

Add a check for host_type == EXTENSIONS.

   v8::HandleScope scope(v8::Isolate::GetCurrent());
   v8::Local<v8::Value> script_value;
   if (in_main_world) {