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

extensions/renderer/script_injection.h

Index: extensions/renderer/script_injection.h
diff --git a/extensions/renderer/script_injection.h b/extensions/renderer/script_injection.h
index 6a93a020a6805da626fc9de72c9556df97d067d6..fa83a3c30f1658ac35961dceccdfd962168831ef 100644
--- a/extensions/renderer/script_injection.h
+++ b/extensions/renderer/script_injection.h
@@ -8,9 +8,9 @@
 #include "base/basictypes.h"
 #include "base/macros.h"
 #include "extensions/common/user_script.h"
+#include "extensions/renderer/injection_host.h"
 #include "extensions/renderer/script_injector.h"
 
-class InjectionHost;
 struct HostID;
 
 namespace blink {
@@ -33,7 +33,7 @@ class ScriptInjection {
   ScriptInjection(
       scoped_ptr<ScriptInjector> injector,
       blink::WebLocalFrame* web_frame,
-      const HostID& host_id,
+      scoped_ptr<const InjectionHost> injection_host,
       const UserScript::ConsumerInstanceType& consumer_instance_type,
       UserScript::RunLocation run_location,
       int tab_id);
@@ -43,20 +43,20 @@ class ScriptInjection {
   // the script has either injected or will never inject (i.e., if the object
   // is done), and false if injection is delayed (either for permission purposes
   // or because |current_location| is not the designated |run_location_|).
-  // NOTE: |injection_host| may be NULL, if the injection_host is removed!
   bool TryToInject(UserScript::RunLocation current_location,
-                   const InjectionHost* injection_host,
                    ScriptsRunInfo* scripts_run_info);
 
   // Called when permission for the given injection has been granted.
   // Returns true if the injection ran.
-  bool OnPermissionGranted(const InjectionHost* injection_host,
-                           ScriptsRunInfo* scripts_run_info);
+  bool OnPermissionGranted(ScriptsRunInfo* scripts_run_info);
 
   // Accessors.
   blink::WebLocalFrame* web_frame() const { return web_frame_; }
-  const HostID& host_id() const { return host_id_; }
+  const HostID& host_id() const { return injection_host_->id(); }
   int64 request_id() const { return request_id_; }
+  void set_injection_host(InjectionHost* host) {

[Devlin, 2015/02/20 22:39:13]

two things:
- if this would actually reset the host to a new one, we should probably pass a
scoped_ptr, because it enforces the ownership scheme.
- it doesn't look like we ever use this for anything other than passing nullptr,
so why not just call it "ResetInjectionHost" or "OnHostRemoved" (and move it to
the .cc, please, to avoid the extra import).

[Xi Han, 2015/02/23 16:14:59]

You are right, it makes more sense to have "OnHostRemoved" and just reset the
pointer. Updated.

+    injection_host_.reset(host);
+  }
 
  private:
   // Sends a message to the browser, either that the script injection would
@@ -64,13 +64,11 @@ class ScriptInjection {
   void SendInjectionMessage(bool request_permission);
 
   // Injects the script, optionally populating |scripts_run_info|.
-  void Inject(const InjectionHost* injection_host,
-              ScriptsRunInfo* scripts_run_info);
+  void Inject(ScriptsRunInfo* scripts_run_info);
 
   // Inject any JS scripts into the |frame|, optionally populating
   // |execution_results|.
-  void InjectJs(const InjectionHost* injection_host,
-                blink::WebLocalFrame* frame,
+  void InjectJs(blink::WebLocalFrame* frame,
                 base::ListValue* execution_results);
 
   // Inject any CSS source into the |frame|.
@@ -85,8 +83,8 @@ class ScriptInjection {
   // The (main) WebFrame into which this should inject the script.
   blink::WebLocalFrame* web_frame_;
 
-  // The id of the associated injection_host.
-  HostID host_id_;
+  // The associated injection host.
+  scoped_ptr<const InjectionHost> injection_host_;
 
   // The type of the instance on which the host will inject the script.
   UserScript::ConsumerInstanceType consumer_instance_type_;