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 d017a6742eb906abebfe9c9e0835994418c2de3d..c0996932e3d9715537a40e1b4cb8c3044d938286 100644
--- a/extensions/renderer/script_injection_manager.cc
+++ b/extensions/renderer/script_injection_manager.cc
@@ -227,7 +227,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_);
@@ -241,6 +240,19 @@ void ScriptInjectionManager::OnRenderViewCreated(
   rvo_helpers_.push_back(new RVOHelper(render_view, this));
 }
 
+void ScriptInjectionManager::OnInjectionFinished(
+    ScriptInjection* injection,
+    ScriptsRunInfo* scripts_run_info) {
+  if (IsFrameValid(injection->web_frame()))
+    scripts_run_info->LogRun(injection->web_frame(), injection->run_location());
+
+  ScopedVector<ScriptInjection>::iterator it;
+  for (it = running_injections_.begin(); it != running_injections_.end(); ++it){
+    running_injections_.erase(it);
+    break;
+  }
+}
+
 void ScriptInjectionManager::OnUserScriptsUpdated(
     const std::set<std::string>& changed_extensions,
     const std::vector<UserScript*>& scripts) {
@@ -252,13 +264,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) {
@@ -322,30 +327,21 @@ void ScriptInjectionManager::StartInjectScripts(
 
   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);
-    // 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);
-    }
   }
 }
 
 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
@@ -369,7 +365,6 @@ void ScriptInjectionManager::InjectScripts(
   user_script_set_manager_->GetAllInjections(
       &frame_injections, frame, tab_id, run_location);
 
-  ScriptsRunInfo scripts_run_info;

[Devlin, 2015/02/11 17:49:58]

Having each injection own a ScriptsRunInfo makes these metrics mean something
completely different. The goal is to have a measurement of how long it took to
run all extension scripts at each stage.

[Devlin, 2015/02/19 17:13:00]

Still waiting on this?

   for (ScopedVector<ScriptInjection>::iterator iter = frame_injections.begin();
        iter != frame_injections.end();) {
     // If a blocking script was injected, there is potentially a possibility
@@ -377,24 +372,23 @@ void ScriptInjectionManager::InjectScripts(
     if (!IsFrameValid(frame))
       break;
 
-    // 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((*iter)->extension_id()) == 0 &&

[Devlin, 2015/02/11 17:49:58]

I don't think it's safe to remove this - can't scripts still be invalidated,
since they execute asynchronously?

-        !(*iter)->TryToInject(run_location,
-                              extensions_->GetByID((*iter)->extension_id()),
-                              &scripts_run_info)) {
+    // Try to inject the script. If the injection does not started
+    // (i.e., it is waiting for permission), add it to the list of pending
+    // injections. If the injection does not finished, add it to the list of
+    // running injections.
+    ScriptInjection* injection = *iter;
+    injection->SetScriptInjectionManager(this);
+    if (!injection->TryToInject(run_location,
+                            extensions_->GetByID((*iter)->extension_id()))) {
       pending_injections_.insert(pending_injections_.begin(), *iter);
       iter = frame_injections.weak_erase(iter);
+    } else if (!injection->is_complete()) {
+      running_injections_.insert(running_injections_.begin(), *iter);
+      iter = frame_injections.weak_erase(iter);
     } else {
       ++iter;
     }
   }
-
-  if (IsFrameValid(frame))
-    scripts_run_info.LogRun(frame, run_location);
-
-  invalidated_while_injecting_.clear();
 }
 
 void ScriptInjectionManager::HandleExecuteCode(
@@ -423,13 +417,14 @@ void ScriptInjectionManager::HandleExecuteCode(
       static_cast<UserScript::RunLocation>(params.run_at),
       ExtensionHelper::Get(render_view)->tab_id()));
 
-  ScriptsRunInfo scripts_run_info;
   FrameStatusMap::const_iterator iter = frame_statuses_.find(main_frame);
+  injection->SetScriptInjectionManager(this);
   if (!injection->TryToInject(
           iter == frame_statuses_.end() ? UserScript::UNDEFINED : iter->second,
-          extensions_->GetByID(injection->extension_id()),
-          &scripts_run_info)) {
+          extensions_->GetByID(injection->extension_id()))) {
     pending_injections_.push_back(injection.release());
+  } else if (!injection->is_complete()) {
+    running_injections_.push_back(injection.release());
   }
 }
 
@@ -450,12 +445,13 @@ void ScriptInjectionManager::HandleExecuteDeclarativeScript(
           url,
           extension);
   if (injection.get()) {
-    ScriptsRunInfo scripts_run_info;
+    injection->SetScriptInjectionManager(this);
     // TODO(markdittmer): Use return value of TryToInject for error handling.
-    injection->TryToInject(UserScript::BROWSER_DRIVEN,
-                           extension,
-                           &scripts_run_info);
-    scripts_run_info.LogRun(web_frame, UserScript::BROWSER_DRIVEN);
+    if (injection->TryToInject(UserScript::BROWSER_DRIVEN,
+                               extension)) {
+      if (!injection->is_complete())
+        running_injections_.push_back(injection.release());
+    }
   }
 }
 
@@ -477,11 +473,11 @@ void ScriptInjectionManager::HandlePermitScriptInjection(int64 request_id) {
   scoped_ptr<ScriptInjection> injection(*iter);
   pending_injections_.weak_erase(iter);
 
-  ScriptsRunInfo scripts_run_info;
+  injection->SetScriptInjectionManager(this);
   if (injection->OnPermissionGranted(extensions_->GetByID(
-                                         injection->extension_id()),
-                                     &scripts_run_info)) {
-    scripts_run_info.LogRun(injection->web_frame(), UserScript::RUN_DEFERRED);
+                                         injection->extension_id()))) {
+    if (!injection->is_complete())
+      running_injections_.push_back(injection.Pass());
   }
 }