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

extensions/renderer/script_injection.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/renderer/script_injection.h"
 
 #include <map>
 #include <utility>
 
 #include "base/lazy_instance.h"
@@ skipping 195 matching lines @@
     ScriptInjector::InjectFailureReason reason) {
   complete_ = true;
   injector_->OnWillNotInject(reason, render_frame_);
 }
 
 ScriptInjection::InjectionResult ScriptInjection::Inject(
     ScriptsRunInfo* scripts_run_info) {
   DCHECK(injection_host_);
   DCHECK(scripts_run_info);
   DCHECK(!complete_);
+  bool should_inject_js = injector_->ShouldInjectJs(
+      run_location_, scripts_run_info->injected_scripts);
+  bool should_inject_css = injector_->ShouldInjectCss(
+      run_location_, scripts_run_info->injected_scripts);
 
-  bool should_inject_js = injector_->ShouldInjectJs(run_location_);
-  bool should_inject_css = injector_->ShouldInjectCss(run_location_);
-  DCHECK(should_inject_js || should_inject_css);
+  // Occurs when script is already injected.

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

I'd be a little more explicit here - maybe something like:
// This can legitimately happen if the extension specified a script to
// be run in multiple rules, and the script has already run.
// See crbug.com/nnn.

That way, it's clear that this is an edge case, rather than something that
happens normally.

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

That's clearer. I'll use yours.
Which crbug.com/nnn are you referring to?

[Devlin, 2016/08/25 16:52:26]

crbug.com/631247, which is the crash that resulted from assuming this couldn't
happen.

[catmullings, 2016/08/27 00:23:08]

Done.

+  if (!(should_inject_js || should_inject_css)) {

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

nit: I find this a little more readable after we De Morgan-ify it. :)
if (!should_inject_js && !should_inject_css)

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

Done.

+    return INJECTION_FINISHED;
+  }
 
   if (should_inject_js)
-    InjectJs();
+    InjectJs(scripts_run_info);
   if (should_inject_css)
-    InjectCss();
+    InjectCss(scripts_run_info);
 
   complete_ = did_inject_js_ || !should_inject_js;
 
   injector_->GetRunInfo(scripts_run_info, run_location_);
 
   if (complete_) {
     injector_->OnInjectionComplete(std::move(execution_result_), run_location_,
                                    render_frame_);
   } else {
     ++scripts_run_info->num_blocking_js;
   }
 
   return complete_ ? INJECTION_FINISHED : INJECTION_BLOCKED;
 }
 
-void ScriptInjection::InjectJs() {
+void ScriptInjection::InjectJs(ScriptsRunInfo* scripts_run_info) {
   DCHECK(!did_inject_js_);
   blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
   std::vector<blink::WebScriptSource> sources =
-      injector_->GetJsSources(run_location_);
+      injector_->GetJsSources(run_location_, scripts_run_info);
+  DCHECK(!sources.empty());
   bool in_main_world = injector_->ShouldExecuteInMainWorld();
   int world_id = in_main_world
                      ? DOMActivityLogger::kMainWorldId
                      : GetIsolatedWorldIdForInstance(injection_host_.get(),
                                                      web_frame);
   bool is_user_gesture = injector_->IsUserGesture();
 
   std::unique_ptr<blink::WebScriptExecutionCallback> callback(
       new ScriptInjectionCallback(
           base::Bind(&ScriptInjection::OnJsInjectionCompleted,
@@ skipping 49 matching lines @@
   // If |async_completion_callback_| is set, it means the script finished
   // asynchronously, and we should run it.
   if (!async_completion_callback_.is_null()) {
     injector_->OnInjectionComplete(std::move(execution_result_), run_location_,
                                    render_frame_);
     // Warning: this object can be destroyed after this line!
     async_completion_callback_.Run(this);
   }
 }
 
-void ScriptInjection::InjectCss() {
+void ScriptInjection::InjectCss(ScriptsRunInfo* scripts_run_info) {
   std::vector<std::string> css_sources =
-      injector_->GetCssSources(run_location_);
+      injector_->GetCssSources(run_location_, scripts_run_info);
   blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
   for (const std::string& css : css_sources)
     web_frame->document().insertStyleSheet(blink::WebString::fromUTF8(css));
 }
 
 }  // namespace extensions