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..f3daf3aed1be8b82507407229143ac6d3cf527ce 100644
--- a/extensions/renderer/script_injection_manager.cc
+++ b/extensions/renderer/script_injection_manager.cc
@@ -14,6 +14,7 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_set.h"
+#include "extensions/renderer/extension_consumer.h"
 #include "extensions/renderer/extension_helper.h"
 #include "extensions/renderer/programmatic_script_injector.h"
 #include "extensions/renderer/script_injection.h"
@@ -52,6 +53,15 @@ UserScript::RunLocation NextRunLocation(UserScript::RunLocation run_location) {
   return UserScript::RUN_LOCATION_LAST;
 }
 
+scoped_ptr<ExtensionConsumer> GetExtensionConsumer(
+    const std::string& extension_id, const ExtensionSet* extensions) {
+  const Extension* extension = extensions->GetByID(extension_id);
+  if (!extension)
+    return scoped_ptr<ExtensionConsumer>();
+  return scoped_ptr<ExtensionConsumer>(new ExtensionConsumer(
+      extension, HostID(HostID::EXTENSIONS, extension_id)));
+}
+
 }  // namespace
 
 class ScriptInjectionManager::RVOHelper : public content::RenderViewObserver {
@@ -247,7 +257,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 +387,15 @@ void ScriptInjectionManager::InjectScripts(
     if (!IsFrameValid(frame))
       break;
 
+    const std::string extension_id = (*iter)->host_id().id();
+    scoped_ptr<ExtensionConsumer> extension_consumer = GetExtensionConsumer(

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

Not really a big deal, since constructing an ExtensionConsumer is super cheap,
but it'd be nice to avoid it at some point, and just have the injection own one
(and be able to know when the backing extension is removed).

Not in this patch, though.

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

Add a TODO here.

+        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((*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 +432,20 @@ void ScriptInjectionManager::HandleExecuteCode(
       scoped_ptr<ScriptInjector>(
           new ProgrammaticScriptInjector(params, main_frame)),
       main_frame,
-      params.extension_id,
+      HostID(HostID::EXTENSIONS, params.extension_id),

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

Hmm... should the type be HostID::Extensions if it's a webview?

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

The HostID::Extensions is a host type, WEBVIEW vs TAB is a instance type. I am
trying to keep the 1:1 relation between HostID and Host, so Host won't contain
any information of which kind of instance it will inject scripts on.

+      params.is_web_view ? params.instance_id : Host::kDefaultInstanceId,

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

didn't we ensure on the browser side that params.instance_id is always correct
(i.e., it will only be different from kDefaultInstanceId if it's a webview)?

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

Well, in browser side, if it is not <webview>, we didn't explicit set
param.instance_id when sending IPC ExtensionMsg_ExecuteCode(see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...).
So I add the check here to make sure a correct default instance is is used for
non-webview. Yes, for non-webview(regular-tab), the instance id is always
kDefaultInstanceId. The purpose of this id is for assigning isolated world in
teh render side. So for tabs from the same host, user scripts will always be
injected on the same isolated world, which remains the same behavior as it was.

       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);
+
+  scoped_ptr<ExtensionConsumer> extension_consumer = GetExtensionConsumer(
+      injection->host_id().id(), extensions_);
+
   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());
   }
@@ -439,7 +457,10 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
     const ExtensionId& extension_id,
     int script_id,
     const GURL& url) {
+  scoped_ptr<ExtensionConsumer> extension_consumer = GetExtensionConsumer(
+      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 =
@@ -453,7 +474,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 +499,9 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   pending_injections_.weak_erase(iter);
 
   ScriptsRunInfo scripts_run_info;
-  if (injection->OnPermissionGranted(extensions_->GetByID(
-                                         injection->extension_id()),
+  scoped_ptr<ExtensionConsumer> extension_consumer = GetExtensionConsumer(
+      injection->host_id().id(), extensions_);
+  if (injection->OnPermissionGranted(extension_consumer.get(),
                                      &scripts_run_info)) {
     scripts_run_info.LogRun(injection->web_frame(), UserScript::RUN_DEFERRED);
   }