Extensions: suspend extension's scripts when V8 is paused

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 93862dd04bdfe1109bf23c223ccef4cb734ec90e..5663b51f3ed6945d2f72938dd6bd690ea52c8c90 100644
--- a/extensions/renderer/script_injection_manager.cc
+++ b/extensions/renderer/script_injection_manager.cc
@@ -53,7 +53,6 @@ UserScript::RunLocation NextRunLocation(UserScript::RunLocation run_location) {
   return UserScript::RUN_LOCATION_LAST;
 }
 
-
 // TODO(hanxi): let ScriptInjection own an InjectionHost to avoid constructing
 // an ExtensionInjectionHost many times.
 // Note: the ScriptInjection should be able to know when the backing extension
@@ -242,7 +241,6 @@ ScriptInjectionManager::ScriptInjectionManager(
     const ExtensionSet* extensions,
     UserScriptSetManager* user_script_set_manager)
     : extensions_(extensions),
-      injecting_scripts_(false),
       user_script_set_manager_(user_script_set_manager),
       user_script_set_manager_observer_(this) {
   user_script_set_manager_observer_.Add(user_script_set_manager_);
@@ -256,6 +254,17 @@ void ScriptInjectionManager::OnRenderViewCreated(
   rvo_helpers_.push_back(new RVOHelper(render_view, this));
 }
 
+void ScriptInjectionManager::OnInjectionFinished(
+    ScriptInjection* injection) {
+  ScopedVector<ScriptInjection>::iterator it;
+  for (it = running_injections_.begin(); it != running_injections_.end(); ++it){

[Devlin, 2015/03/03 00:34:10]

Prefer:
ScopedVector<ScriptInjection>::iterator iter =
    running_injections_.find(injection);
if (iter != running_injections_.end())
  running_injections_.erase(iter);

[kozy, 2015/03/03 13:50:20]

Done.

+    if (*it == injection) {
+      running_injections_.erase(it);
+      break;
+    }
+  }
+}
+
 void ScriptInjectionManager::OnUserScriptsUpdated(
     const std::set<std::string>& changed_extensions,
     const std::vector<UserScript*>& scripts) {
@@ -267,13 +276,6 @@ void ScriptInjectionManager::OnUserScriptsUpdated(
     else
       ++iter;
   }
-
-  // If we are currently injecting scripts, we need to make a note that these
-  // extensions were updated.
-  if (injecting_scripts_) {
-    invalidated_while_injecting_.insert(changed_extensions.begin(),
-                                        changed_extensions.end());
-  }
 }
 
 void ScriptInjectionManager::RemoveObserver(RVOHelper* helper) {
@@ -300,10 +302,6 @@ void ScriptInjectionManager::InvalidateForFrame(blink::WebFrame* frame) {
   frame_statuses_.erase(frame);
 }
 
-bool ScriptInjectionManager::IsFrameValid(blink::WebFrame* frame) const {
-  return frame_statuses_.find(frame) != frame_statuses_.end();
-}
-
 void ScriptInjectionManager::StartInjectScripts(
     blink::WebFrame* frame, UserScript::RunLocation run_location) {
   FrameStatusMap::iterator iter = frame_statuses_.find(frame);
@@ -334,38 +332,14 @@ void ScriptInjectionManager::StartInjectScripts(
 
   // Otherwise, all is right in the world, and we can get on with the
   // injections!
-
   frame_statuses_[frame] = run_location;
-
-  // If a content script injects blocking code (such as a javascript alert()),
-  // then there is a chance that we are running in a nested message loop, and
-  // shouldn't inject scripts right now (to avoid conflicts).
-  if (!injecting_scripts_) {
-    InjectScripts(frame, run_location);
-    // As above, we might have been blocked, but that means that, in the mean
-    // time, it's possible the frame advanced. Inject any scripts for run
-    // locations that were registered, but never ran.
-    while ((iter = frame_statuses_.find(frame)) != frame_statuses_.end() &&
-           iter->second > run_location) {
-      run_location = NextRunLocation(run_location);
-      DCHECK_LE(run_location, UserScript::DOCUMENT_IDLE);
-      InjectScripts(frame, run_location);
-    }
-  }
+  InjectScripts(frame, run_location);
 }
 
 void ScriptInjectionManager::InjectScripts(
     blink::WebFrame* frame,
     UserScript::RunLocation run_location) {
-  DCHECK(!injecting_scripts_);
-  DCHECK(invalidated_while_injecting_.empty());
-  base::AutoReset<bool>(&injecting_scripts_, true);
-
   // Find any injections that want to run on the given frame.
-  // We create a separate vector for these because there is a chance that
-  // injected scripts can block, which can create a nested message loop. When
-  // this happens, other signals (like IPCs) can cause |pending_injections_| to
-  // be changed, so we don't want to risk that.
   ScopedVector<ScriptInjection> frame_injections;
   for (ScopedVector<ScriptInjection>::iterator iter =
            pending_injections_.begin();
@@ -385,34 +359,36 @@ void ScriptInjectionManager::InjectScripts(
       &frame_injections, frame, tab_id, run_location);
 
   ScriptsRunInfo scripts_run_info;
-  for (ScopedVector<ScriptInjection>::iterator iter = frame_injections.begin();
-       iter != frame_injections.end();) {
-    // If a blocking script was injected, there is potentially a possibility
-    // that the frame has been invalidated in the time since. Check.
-    if (!IsFrameValid(frame))
-      break;
-
-    const std::string& extension_id = (*iter)->host_id().id();
-    scoped_ptr<ExtensionInjectionHost> extension_injection_host =
-        GetExtensionInjectionHost(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(extension_id) == 0 &&
-        !(*iter)->TryToInject(run_location,
-                              extension_injection_host.get(),
-                              &scripts_run_info)) {
-      pending_injections_.insert(pending_injections_.begin(), *iter);
-      iter = frame_injections.weak_erase(iter);
-    } else {
-      ++iter;
-    }
-  }
+  std::vector<ScriptInjection*> released_injections;
+  frame_injections.release(&released_injections);
+  for (ScriptInjection* injection : released_injections)
+    TryToInject(make_scoped_ptr(injection), run_location, &scripts_run_info);
 
-  if (IsFrameValid(frame))
-    scripts_run_info.LogRun(frame, run_location);
+  scripts_run_info.LogRun(frame, run_location);
+}
 
-  invalidated_while_injecting_.clear();
+void ScriptInjectionManager::TryToInject(
+    scoped_ptr<ScriptInjection> injection,
+    UserScript::RunLocation run_location,
+    ScriptsRunInfo* scripts_run_info) {
+  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
+      GetExtensionInjectionHost(injection->host_id().id(), extensions_);
+  injection->SetScriptInjectionManager(this);
+  // Try to inject the script. If the injection is waiting (i.e., for
+  // permission), add it to the list of pending injections. If the injection
+  // has blocked, add it to the list of running injections.
+  switch (injection->TryToInject(
+      run_location,
+      extension_injection_host.get(),
+      scripts_run_info)) {
+    case ScriptInjection::INJECTION_WAITING:
+      pending_injections_.push_back(injection.release());
+    case ScriptInjection::INJECTION_BLOCKED:
+      running_injections_.push_back(injection.release());
+      break;
+    case ScriptInjection::INJECTION_FINISHED:
+      break;
+  }
 }
 
 void ScriptInjectionManager::HandleExecuteCode(
@@ -444,15 +420,10 @@ void ScriptInjectionManager::HandleExecuteCode(
   ScriptsRunInfo scripts_run_info;
   FrameStatusMap::const_iterator iter = frame_statuses_.find(main_frame);
 
-  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
-      GetExtensionInjectionHost(injection->host_id().id(), extensions_);
-
-  if (!injection->TryToInject(
-          iter == frame_statuses_.end() ? UserScript::UNDEFINED : iter->second,
-          extension_injection_host.get(),
-          &scripts_run_info)) {
-    pending_injections_.push_back(injection.release());
-  }
+  TryToInject(
+      injection.Pass(),
+      iter == frame_statuses_.end() ? UserScript::UNDEFINED : iter->second,
+      &scripts_run_info);
 }
 
 void ScriptInjectionManager::HandleExecuteDeclarativeScript(
@@ -461,8 +432,6 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
     const ExtensionId& extension_id,
     int script_id,
     const GURL& url) {
-  scoped_ptr<ExtensionInjectionHost> extension_injection_host =
-      GetExtensionInjectionHost(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.
@@ -475,10 +444,12 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
           extension);
   if (injection.get()) {
     ScriptsRunInfo scripts_run_info;
+
     // TODO(markdittmer): Use return value of TryToInject for error handling.
-    injection->TryToInject(UserScript::BROWSER_DRIVEN,
-                           extension_injection_host.get(),
-                           &scripts_run_info);
+    TryToInject(injection.Pass(),
+                UserScript::BROWSER_DRIVEN,
+                &scripts_run_info);
+
     scripts_run_info.LogRun(web_frame, UserScript::BROWSER_DRIVEN);
   }
 }
@@ -501,11 +472,17 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   scoped_ptr<ScriptInjection> injection(*iter);
   pending_injections_.weak_erase(iter);
 
-  ScriptsRunInfo scripts_run_info;
+  injection->SetScriptInjectionManager(this);

[Devlin, 2015/03/03 00:34:10]

Shouldn't this already be set?  (We can't have an injection waiting for
permission if it hasn't tried to inject)

[kozy, 2015/03/03 13:50:20]

Removed.

+
   scoped_ptr<ExtensionInjectionHost> extension_injection_host =
       GetExtensionInjectionHost(injection->host_id().id(), extensions_);
-  if (injection->OnPermissionGranted(extension_injection_host.get(),
-                                     &scripts_run_info)) {
+  ScriptsRunInfo scripts_run_info;
+  ScriptInjection::InjectionResult res = injection->OnPermissionGranted(
+      extension_injection_host.get(), &scripts_run_info);
+  if (res == ScriptInjection::INJECTION_BLOCKED) {

[Devlin, 2015/03/03 00:34:10]

nit: no brackets on single-line ifs.

[kozy, 2015/03/03 13:50:20]

Done.

+    running_injections_.push_back(injection.Pass());
+  }
+  if (extension_injection_host.get()) {
     scripts_run_info.LogRun(injection->web_frame(), UserScript::RUN_DEFERRED);
   }
 }