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

extensions/renderer/script_injection_manager.cc

Index: extensions/renderer/script_injection_manager.cc
diff --git a/extensions/renderer/script_injection_manager.cc b/extensions/renderer/script_injection_manager.cc
index 2e1a747cb172673c8fc6d1e4a9bf97d58bdffe00..ebacdb998f7d84ed2d463dee6c571965331a991c 100644
--- a/extensions/renderer/script_injection_manager.cc
+++ b/extensions/renderer/script_injection_manager.cc
@@ -53,17 +53,12 @@ UserScript::RunLocation NextRunLocation(UserScript::RunLocation run_location) {
   return UserScript::RUN_LOCATION_LAST;
 }
 
-
-// TODO(hanxi): let ScriptInjection own an InjectionHost to avoid constructing
-// an ExtensionInjectionHost many times.
-// Note: the ScriptInjection should be able to know when the backing extension
-// is removed.
-scoped_ptr<ExtensionInjectionHost> GetExtensionInjectionHost(
+scoped_ptr<const ExtensionInjectionHost> GetExtensionInjectionHost(
     const std::string& extension_id, const ExtensionSet* extensions) {
   const Extension* extension = extensions->GetByID(extension_id);
   if (!extension)
-    return scoped_ptr<ExtensionInjectionHost>();
-  return scoped_ptr<ExtensionInjectionHost>(
+    return scoped_ptr<const ExtensionInjectionHost>();
+  return scoped_ptr<const ExtensionInjectionHost>(
       new ExtensionInjectionHost(extension));
 }
 
@@ -392,15 +387,13 @@ void ScriptInjectionManager::InjectScripts(
     if (!IsFrameValid(frame))
       break;
 
-    const std::string& extension_id = (*iter)->host_id().id();
-    scoped_ptr<ExtensionInjectionHost> extension_injection_host =
-        GetExtensionInjectionHost(extension_id, extensions_);
-    // Try to inject the script if the extension is not "dirty" (invalidated by
-    // an update). If the injection does not finish (i.e., it is waiting for
-    // permission), add it to the list of pending injections.
-    if (invalidated_while_injecting_.count(extension_id) == 0 &&
+    // Try to inject the script if the injection host is not "dirty"
+    // (invalidated by an update). If the injection does not finish
+    // (i.e., it is waiting for permission), add it to the list of pending
+    // injections.
+    if (invalidated_while_injecting_.count((*iter)->host_id().id()) == 0 &&
         !(*iter)->TryToInject(run_location,
-                              extension_injection_host.get(),
+                              (*iter)->GetInjectionHost(),

[Devlin, 2015/02/17 18:56:52]

This is a bit silly - the ScriptInjection owns the injection host; why does it
need to be passed in?

[Xi Han, 2015/02/18 21:11:46]

Removed, and also updated some other functions that take InjectionHost* as input
parameters.

                               &scripts_run_info)) {
       pending_injections_.insert(pending_injections_.begin(), *iter);
       iter = frame_injections.weak_erase(iter);
@@ -433,11 +426,17 @@ void ScriptInjectionManager::HandleExecuteCode(
     return;
   }
 
+  scoped_ptr<const ExtensionInjectionHost> extension_injection_host =
+      GetExtensionInjectionHost(params.extension_id, extensions_);
+
+  if (!extension_injection_host)
+    return;
+
   scoped_ptr<ScriptInjection> injection(new ScriptInjection(
       scoped_ptr<ScriptInjector>(
           new ProgrammaticScriptInjector(params, main_frame)),
       main_frame,
-      HostID(HostID::EXTENSIONS, params.extension_id),
+      extension_injection_host.Pass(),
       params.is_web_view ? UserScript::ConsumerInstanceType::WEBVIEW
                          : UserScript::ConsumerInstanceType::TAB,
       static_cast<UserScript::RunLocation>(params.run_at),
@@ -446,12 +445,9 @@ void ScriptInjectionManager::HandleExecuteCode(
   ScriptsRunInfo scripts_run_info;
   FrameStatusMap::const_iterator iter = frame_statuses_.find(main_frame);
 
-  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
-      GetExtensionInjectionHost(injection->host_id().id(), extensions_);
-
   if (!injection->TryToInject(
           iter == frame_statuses_.end() ? UserScript::UNDEFINED : iter->second,
-          extension_injection_host.get(),
+          injection->GetInjectionHost(),
           &scripts_run_info)) {
     pending_injections_.push_back(injection.release());
   }
@@ -463,9 +459,8 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
     const ExtensionId& extension_id,
     int script_id,
     const GURL& url) {
-  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
+  scoped_ptr<const ExtensionInjectionHost> extension_injection_host =
       GetExtensionInjectionHost(extension_id, extensions_);
-  const Extension* extension = extensions_->GetByID(extension_id);
   // TODO(dcheng): This function signature should really be a WebLocalFrame,
   // rather than trying to coerce it here.
   scoped_ptr<ScriptInjection> injection =
@@ -474,7 +469,7 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
           web_frame->toWebLocalFrame(),
           tab_id,
           url,
-          extension);
+          extension_injection_host.get());
   if (injection.get()) {
     ScriptsRunInfo scripts_run_info;
     // TODO(markdittmer): Use return value of TryToInject for error handling.
@@ -489,7 +484,8 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   ScopedVector<ScriptInjection>::iterator iter =
       pending_injections_.begin();
   for (; iter != pending_injections_.end(); ++iter) {
-    if ((*iter)->request_id() == request_id)
+    if ((*iter)->request_id() == request_id &&
+        (*iter)->host_id().type() == HostID::EXTENSIONS)
       break;
   }
   if (iter == pending_injections_.end())
@@ -504,7 +500,7 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   pending_injections_.weak_erase(iter);
 
   ScriptsRunInfo scripts_run_info;
-  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
+  scoped_ptr<const ExtensionInjectionHost> extension_injection_host =
       GetExtensionInjectionHost(injection->host_id().id(), extensions_);
   if (injection->OnPermissionGranted(extension_injection_host.get(),
                                      &scripts_run_info)) {