Prevent duplicate content script injection defined in manifest.json (reland)

extensions/renderer/user_script_injector.cc

Index: extensions/renderer/user_script_injector.cc
diff --git a/extensions/renderer/user_script_injector.cc b/extensions/renderer/user_script_injector.cc
index 21a59eac30fa4bd3e03c928090db5f13e6189ed9..612b9e09002b4378d6d5add4d6a0dfd3f6276c5f 100644
--- a/extensions/renderer/user_script_injector.cc
+++ b/extensions/renderer/user_script_injector.cc
@@ -135,16 +135,43 @@ bool UserScriptInjector::ExpectsResults() const {
   return false;
 }
 
-bool UserScriptInjector::ShouldInjectJs(
-    UserScript::RunLocation run_location) const {
+bool UserScriptInjector::ShouldInjectJs(UserScript::RunLocation run_location,
+                                        std::set<GURL> injected_scripts) const {
+  const UserScript::FileList& js_scripts = script_->js_scripts();

[Devlin, 2016/08/22 22:46:52]

if script_ is null here, this will crash.

[catmullings, 2016/08/24 01:13:19]

Done.

+
+  bool scripts_need_injecting = false;
+
+  for (UserScript::FileList::const_iterator iter = js_scripts.begin();

[Devlin, 2016/08/22 22:46:52]

prefer range-based for loops.

[catmullings, 2016/08/24 01:13:19]

Done.

+       iter != js_scripts.end(); ++iter) {
+    const GURL& script_url = iter->url();
+    // Check if the script is already injected.
+    if (injected_scripts.count(script_url) == 0) {
+      scripts_need_injecting = true;
+      break;
+    }
+  }
   return script_ && script_->run_location() == run_location &&
-         !script_->js_scripts().empty();
+         !script_->js_scripts().empty() && scripts_need_injecting;
 }
 
 bool UserScriptInjector::ShouldInjectCss(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    std::set<GURL> injected_scripts) const {
+  const UserScript::FileList& css_scripts = script_->css_scripts();
+
+  bool scripts_need_injecting = false;
+
+  for (UserScript::FileList::const_iterator iter = css_scripts.begin();

[Devlin, 2016/08/22 22:46:52]

nit: We could probably pull this into a helper function to make it more
readable.

[catmullings, 2016/08/24 01:13:19]

Done.

+       iter != css_scripts.end(); ++iter) {
+    const GURL& script_url = iter->url();
+    // Check if the stylesheet is already injected.
+    if (injected_scripts.count(script_url) == 0) {
+      scripts_need_injecting = true;
+      break;
+    }
+  }
   return script_ && run_location == UserScript::DOCUMENT_START &&
-         !script_->css_scripts().empty();
+         !script_->css_scripts().empty() && scripts_need_injecting;
 }
 
 PermissionsData::AccessType UserScriptInjector::CanExecuteOnFrame(
@@ -195,10 +222,10 @@ PermissionsData::AccessType UserScriptInjector::CanExecuteOnFrame(
 }
 
 std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    ScriptsRunInfo* scripts_run_info) const {
+  DCHECK(script_);
   std::vector<blink::WebScriptSource> sources;
-  if (!script_)
-    return sources;
 
   DCHECK_EQ(script_->run_location(), run_location);
 
@@ -207,6 +234,11 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
   for (UserScript::FileList::const_iterator iter = js_scripts.begin();
        iter != js_scripts.end();
        ++iter) {
+    const GURL& script_url = iter->url();
+    // Check if the script is already injected.
+    if (scripts_run_info->injected_scripts.count(script_url) != 0)
+      continue;
+
     std::string content = iter->GetContent().as_string();
 
     // We add this dumb function wrapper for user scripts to emulate what
@@ -216,7 +248,9 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
       content += kUserScriptTail;
     }
     sources.push_back(blink::WebScriptSource(
-        blink::WebString::fromUTF8(content), iter->url()));
+        blink::WebString::fromUTF8(content), script_url));
+
+    scripts_run_info->injected_scripts.insert(script_url);
   }
 
   // Emulate Greasemonkey API for scripts that were converted to extension
@@ -228,18 +262,25 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
 }
 
 std::vector<std::string> UserScriptInjector::GetCssSources(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    ScriptsRunInfo* scripts_run_info) const {
+  DCHECK(script_ != NULL);
   DCHECK_EQ(UserScript::DOCUMENT_START, run_location);
 
   std::vector<std::string> sources;
-  if (!script_)
-    return sources;
 
   const UserScript::FileList& css_scripts = script_->css_scripts();
+
   for (UserScript::FileList::const_iterator iter = css_scripts.begin();
        iter != css_scripts.end();
        ++iter) {
-    sources.push_back(iter->GetContent().as_string());
+    const GURL& script_url = iter->url();
+    // Check if the stylesheet is already injected.
+    if (scripts_run_info->injected_scripts.count(script_url) == 0) {
+      sources.push_back(iter->GetContent().as_string());
+
+      scripts_run_info->injected_scripts.insert(script_url);
+    }
   }
   return sources;
 }
@@ -250,7 +291,7 @@ void UserScriptInjector::GetRunInfo(
   if (!script_)
     return;
 
-  if (ShouldInjectJs(run_location)) {
+  if (ShouldInjectJs(run_location, scripts_run_info->injected_scripts)) {
     const UserScript::FileList& js_scripts = script_->js_scripts();
     scripts_run_info->num_js += js_scripts.size();
     for (UserScript::FileList::const_iterator iter = js_scripts.begin();
@@ -261,7 +302,7 @@ void UserScriptInjector::GetRunInfo(
     }
   }
 
-  if (ShouldInjectCss(run_location))
+  if (ShouldInjectCss(run_location, scripts_run_info->injected_scripts))
     scripts_run_info->num_css += script_->css_scripts().size();
 }