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

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 d017a6742eb906abebfe9c9e0835994418c2de3d..1d2246cf2ea270c273fdb889332e8735bb4e577f 100644
--- a/extensions/renderer/script_injection_manager.cc
+++ b/extensions/renderer/script_injection_manager.cc
@@ -12,6 +12,7 @@
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/render_view_observer.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_consumer.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/renderer/extension_helper.h"
@@ -247,7 +248,7 @@ void ScriptInjectionManager::OnUserScriptsUpdated(
   for (ScopedVector<ScriptInjection>::iterator iter =
            pending_injections_.begin();
        iter != pending_injections_.end();) {
-    if (changed_extensions.count((*iter)->extension_id()) > 0)
+    if (changed_extensions.count((*iter)->host_id().id()) > 0)
       iter = pending_injections_.erase(iter);
     else
       ++iter;
@@ -377,12 +378,17 @@ void ScriptInjectionManager::InjectScripts(
     if (!IsFrameValid(frame))
       break;
 
+    const std::string extension_id = (*iter)->host_id().id();
+    scoped_ptr<ExtensionConsumer> extension_consumer(
+        new ExtensionConsumer(

[Devlin, 2015/02/04 17:01:14]

This breaks.

ScriptInjection checks that |extension| is non-null, which you have updated to
check that |host| is non-null.  But you never pass in a null host here.

[Xi Han, 2015/02/05 16:06:21]

You are absolutely right. In ScriptInjection class, what I need to check is
whether the pointer of Extension within the host is empty.

+            const_cast<Extension*>(extensions_->GetByID(extension_id)),
+            HostID(HostID::EXTENSIONS, extension_id)));
     // 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((*iter)->extension_id()) == 0 &&
+    if (invalidated_while_injecting_.count(extension_id) == 0 &&
         !(*iter)->TryToInject(run_location,
-                              extensions_->GetByID((*iter)->extension_id()),
+                              extension_consumer.get(),
                               &scripts_run_info)) {
       pending_injections_.insert(pending_injections_.begin(), *iter);
       iter = frame_injections.weak_erase(iter);
@@ -419,15 +425,22 @@ void ScriptInjectionManager::HandleExecuteCode(
       scoped_ptr<ScriptInjector>(
           new ProgrammaticScriptInjector(params, main_frame)),
       main_frame,
-      params.extension_id,
+      HostID(HostID::EXTENSIONS, params.extension_id),
+      params.is_web_view ? params.instance_id : Host::kDefaultInstanceId,
       static_cast<UserScript::RunLocation>(params.run_at),
       ExtensionHelper::Get(render_view)->tab_id()));
 
   ScriptsRunInfo scripts_run_info;
   FrameStatusMap::const_iterator iter = frame_statuses_.find(main_frame);
+
+  const std::string& extension_id = injection->host_id().id();
+  scoped_ptr<ExtensionConsumer> extension_consumer(new ExtensionConsumer(
+      const_cast<Extension*>(extensions_->GetByID(extension_id)),
+      HostID(HostID::EXTENSIONS, extension_id)));
+
   if (!injection->TryToInject(
           iter == frame_statuses_.end() ? UserScript::UNDEFINED : iter->second,
-          extensions_->GetByID(injection->extension_id()),
+          extension_consumer.get(),
           &scripts_run_info)) {
     pending_injections_.push_back(injection.release());
   }
@@ -440,6 +453,9 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
     int script_id,
     const GURL& url) {
   const Extension* extension = extensions_->GetByID(extension_id);
+  scoped_ptr<ExtensionConsumer> extension_consumer(
+      new ExtensionConsumer(const_cast<Extension*>(extension),
+                        HostID(HostID::EXTENSIONS, extension_id)));
   // TODO(dcheng): This function signature should really be a WebLocalFrame,
   // rather than trying to coerce it here.
   scoped_ptr<ScriptInjection> injection =
@@ -453,7 +469,7 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
     ScriptsRunInfo scripts_run_info;
     // TODO(markdittmer): Use return value of TryToInject for error handling.
     injection->TryToInject(UserScript::BROWSER_DRIVEN,
-                           extension,
+                           extension_consumer.get(),
                            &scripts_run_info);
     scripts_run_info.LogRun(web_frame, UserScript::BROWSER_DRIVEN);
   }
@@ -478,8 +494,11 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   pending_injections_.weak_erase(iter);
 
   ScriptsRunInfo scripts_run_info;
-  if (injection->OnPermissionGranted(extensions_->GetByID(
-                                         injection->extension_id()),
+  const std::string& extension_id = injection->host_id().id();
+  scoped_ptr<ExtensionConsumer> extension_consumer(new ExtensionConsumer(
+      const_cast<Extension*>(extensions_->GetByID(extension_id)),
+      HostID(HostID::EXTENSIONS, extension_id)));
+  if (injection->OnPermissionGranted(extension_consumer.get(),
                                      &scripts_run_info)) {
     scripts_run_info.LogRun(injection->web_frame(), UserScript::RUN_DEFERRED);
   }