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 da0c760e54425c7c329438150ed892501b18c6d5..ba835f48389fa4d8fb1bf5f3a4af54858fe66def 100644
--- a/extensions/renderer/user_script_injector.cc
+++ b/extensions/renderer/user_script_injector.cc
@@ -85,6 +85,19 @@ blink::WebScriptSource GreasemonkeyApiJsString::GetSource() const {
 base::LazyInstance<GreasemonkeyApiJsString> g_greasemonkey_api =
     LAZY_INSTANCE_INITIALIZER;
 
+bool ShouldInjectScripts(
+    const UserScript::FileList& scripts,
+    std::map<std::string, std::set<std::string>>& executing_scripts,
+    const HostID& host_id) {
+  for (const std::unique_ptr<UserScript::File>& file : scripts) {
+    // Check if the script is already injected.
+    if (executing_scripts[host_id.id()].count(file->url().path()) == 0) {

[Devlin, 2016/09/06 17:21:13]

We're always checking the same set in the map, so we could just pass in that set
(and also save ourselves the repeated lookup).  Then we also don't need to pass
in the host_id.

[catmullings, 2016/09/07 01:00:36]

Done.

+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 UserScriptInjector::UserScriptInjector(const UserScript* script,
@@ -138,15 +151,21 @@ bool UserScriptInjector::ExpectsResults() const {
 }
 
 bool UserScriptInjector::ShouldInjectJs(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    std::map<std::string, std::set<std::string>>& executing_scripts) const {
   return script_ && script_->run_location() == run_location &&
-         !script_->js_scripts().empty();
+         !script_->js_scripts().empty() &&
+         ShouldInjectScripts(script_->js_scripts(), executing_scripts,
+                             host_id_);
 }
 
 bool UserScriptInjector::ShouldInjectCss(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    std::map<std::string, std::set<std::string>>& executing_scripts) const {
   return script_ && run_location == UserScript::DOCUMENT_START &&
-         !script_->css_scripts().empty();
+         !script_->css_scripts().empty() &&
+         ShouldInjectScripts(script_->css_scripts(), executing_scripts,
+                             host_id_);
 }
 
 PermissionsData::AccessType UserScriptInjector::CanExecuteOnFrame(
@@ -197,16 +216,21 @@ PermissionsData::AccessType UserScriptInjector::CanExecuteOnFrame(
 }
 
 std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    std::map<std::string, std::set<std::string>>* executing_scripts,
+    size_t* num_injected_js_scripts) const {
+  DCHECK(script_);
   std::vector<blink::WebScriptSource> sources;
-  if (!script_)
-    return sources;
 
   DCHECK_EQ(script_->run_location(), run_location);
 
   const UserScript::FileList& js_scripts = script_->js_scripts();
   sources.reserve(js_scripts.size());
   for (const std::unique_ptr<UserScript::File>& file : js_scripts) {
+    const GURL& script_url = file->url();
+    // Check if the script is already injected.
+    if ((*executing_scripts)[host_id_.id()].count(script_url.path()) != 0)

[Devlin, 2016/09/06 17:21:13]

Let's cache the set of injecting scripts so that we don't need to repeat the
lookup (also in InjectCss)

[catmullings, 2016/09/07 01:00:36]

Done.

+      continue;
     base::StringPiece script_content = file->GetContent();
     blink::WebString source;
     if (script_->emulate_greasemonkey()) {
@@ -227,9 +251,10 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
       source = blink::WebString::fromUTF8(script_content.data(),
                                           script_content.length());
     }
-    sources.push_back(blink::WebScriptSource(source, file->url()));
+    sources.push_back(blink::WebScriptSource(source, script_url));
+    (*num_injected_js_scripts) += 1;
+    (*executing_scripts)[host_id_.id()].insert(script_url.path());
   }
-
   // Emulate Greasemonkey API for scripts that were converted to extension
   // user scripts.
   if (script_->emulate_greasemonkey())
@@ -239,45 +264,34 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
 }
 
 std::vector<blink::WebString> UserScriptInjector::GetCssSources(
-    UserScript::RunLocation run_location) const {
+    UserScript::RunLocation run_location,
+    std::map<std::string, std::set<std::string>>* executing_scripts,
+    size_t* num_injected_css_scripts) const {
+  DCHECK(script_);
   DCHECK_EQ(UserScript::DOCUMENT_START, run_location);
 
   std::vector<blink::WebString> sources;
-  if (!script_)
-    return sources;
 
   const UserScript::FileList& css_scripts = script_->css_scripts();
   sources.reserve(css_scripts.size());
   for (const std::unique_ptr<UserScript::File>& file : script_->css_scripts()) {
+    const GURL& script_url = file->url();
+    // Check if the stylesheet is already injected.
+    if ((*executing_scripts)[host_id_.id()].count(script_url.path()) != 0)
+      continue;
+
     // TODO(lazyboy): |css_content| string is copied into blink::WebString for
     // every frame in the current renderer process. Avoid the copy, possibly by
     // only performing the copy once.
     base::StringPiece css_content = file->GetContent();
     sources.push_back(
         blink::WebString::fromUTF8(css_content.data(), css_content.length()));
+    (*num_injected_css_scripts) += 1;
+    (*executing_scripts)[host_id_.id()].insert(script_url.path());
   }
   return sources;
 }
 
-void UserScriptInjector::GetRunInfo(
-    ScriptsRunInfo* scripts_run_info,
-    UserScript::RunLocation run_location) const {
-  if (!script_)
-    return;
-
-  if (ShouldInjectJs(run_location)) {
-    const UserScript::FileList& js_scripts = script_->js_scripts();
-    scripts_run_info->num_js += js_scripts.size();
-    for (const std::unique_ptr<UserScript::File>& iter : js_scripts) {
-      scripts_run_info->executing_scripts[host_id_.id()].insert(
-          iter->url().path());
-    }
-  }
-
-  if (ShouldInjectCss(run_location))
-    scripts_run_info->num_css += script_->css_scripts().size();
-}
-
 void UserScriptInjector::OnInjectionComplete(
     std::unique_ptr<base::Value> execution_result,
     UserScript::RunLocation run_location,