Extensions: suspend extension's scripts when V8 is paused

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 "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/timer/elapsed_timer.h"
 #include "base/values.h"
 #include "content/public/child/v8_value_converter.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/host_id.h"
 #include "extensions/common/manifest_handlers/csp_info.h"
 #include "extensions/renderer/dom_activity_logger.h"
 #include "extensions/renderer/extension_groups.h"
 #include "extensions/renderer/extension_injection_host.h"
 #include "extensions/renderer/extensions_renderer_client.h"
+#include "extensions/renderer/script_injection_callback.h"
+#include "extensions/renderer/script_injection_manager.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebScopedUserGesture.h"
 #include "third_party/WebKit/public/web/WebScriptSource.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "url/gurl.h"
 
 namespace extensions {
 
@@ skipping 78 matching lines @@
     blink::WebLocalFrame* web_frame,
     const HostID& host_id,
     UserScript::RunLocation run_location,
     int tab_id)
     : injector_(injector.Pass()),
       web_frame_(web_frame),
       host_id_(host_id),
       run_location_(run_location),
       tab_id_(tab_id),
       request_id_(kInvalidRequestId),
-      complete_(false) {
+      complete_(false),
+      running_frames_(0),
+      execution_results_(new base::ListValue()),
+      all_injection_started_(false),
+      script_injection_manager_(nullptr) {
 }
 
 ScriptInjection::~ScriptInjection() {
   if (!complete_)
     injector_->OnWillNotInject(ScriptInjector::WONT_INJECT);
 }
 
-bool ScriptInjection::TryToInject(UserScript::RunLocation current_location,
-                                  const InjectionHost* injection_host,
-                                  ScriptsRunInfo* scripts_run_info) {
+ScriptInjection::InjectionResult ScriptInjection::TryToInject(
+    UserScript::RunLocation current_location,
+    const InjectionHost* injection_host,
+    ScriptsRunInfo* scripts_run_info) {
   if (current_location < run_location_)
-    return false;  // Wait for the right location.
+    return INJECTION_WAITING;  // Wait for the right location.
 
-  if (request_id_ != kInvalidRequestId)
-    return false;  // We're waiting for permission right now, try again later.
+  if (request_id_ != kInvalidRequestId) {
+    // We're waiting for permission right now, try again later.
+    return INJECTION_WAITING;
+  }
 
   if (!injection_host) {
     NotifyWillNotInject(ScriptInjector::EXTENSION_REMOVED);
-    return true;  // We're done.
+    return INJECTION_FINISHED;  // We're done.
   }
 
   switch (injector_->CanExecuteOnFrame(injection_host, web_frame_, tab_id_,
                                        web_frame_->top()->document().url())) {
     case PermissionsData::ACCESS_DENIED:
       NotifyWillNotInject(ScriptInjector::NOT_ALLOWED);
-      return true;  // We're done.
+      return INJECTION_FINISHED;  // We're done.
     case PermissionsData::ACCESS_WITHHELD:
       SendInjectionMessage(true /* request permission */);
-      return false;  // Wait around for permission.
+      return INJECTION_WAITING;  // Wait around for permission.
     case PermissionsData::ACCESS_ALLOWED:
-      Inject(injection_host, scripts_run_info);
-      return true;  // We're done!
+      return Inject(injection_host, scripts_run_info);
   }
 
   NOTREACHED();
-  return false;
+  return INJECTION_FINISHED;
 }
 
-bool ScriptInjection::OnPermissionGranted(const InjectionHost* injection_host,
-                                          ScriptsRunInfo* scripts_run_info) {
+ScriptInjection::InjectionResult ScriptInjection::OnPermissionGranted(
+    const InjectionHost* injection_host,
+    ScriptsRunInfo* scripts_run_info) {
   if (!injection_host) {
     NotifyWillNotInject(ScriptInjector::EXTENSION_REMOVED);
-    return false;
+    return INJECTION_FINISHED;
   }
 
-  Inject(injection_host, scripts_run_info);
-  return true;
+  return Inject(injection_host, scripts_run_info);
 }
 
 void ScriptInjection::SendInjectionMessage(bool request_permission) {
   content::RenderView* render_view =
       content::RenderView::FromWebView(web_frame()->top()->view());
 
   // If we are just notifying the browser of the injection, then send an
   // invalid request (which is treated like a notification).
   request_id_ = request_permission ? g_next_pending_id++ : kInvalidRequestId;
   render_view->Send(new ExtensionHostMsg_RequestScriptInjectionPermission(
       render_view->GetRoutingID(),
       host_id_.id(),
       injector_->script_type(),
       request_id_));
 }
 
 void ScriptInjection::NotifyWillNotInject(
     ScriptInjector::InjectFailureReason reason) {
   complete_ = true;
   injector_->OnWillNotInject(reason);
 }
 
-void ScriptInjection::Inject(const InjectionHost* injection_host,
-                             ScriptsRunInfo* scripts_run_info) {
+ScriptInjection::InjectionResult ScriptInjection::Inject(
+    const InjectionHost* injection_host,
+    ScriptsRunInfo* scripts_run_info) {
   DCHECK(injection_host);
   DCHECK(scripts_run_info);
   DCHECK(!complete_);
 
   if (injection_host->ShouldNotifyBrowserOfInjection())
     SendInjectionMessage(false /* don't request permission */);
 
   std::vector<blink::WebFrame*> frame_vector;
   frame_vector.push_back(web_frame_);
   if (injector_->ShouldExecuteInChildFrames())
     AppendAllChildFrames(web_frame_, &frame_vector);
 
-  scoped_ptr<blink::WebScopedUserGesture> gesture;
-  if (injector_->IsUserGesture())
-    gesture.reset(new blink::WebScopedUserGesture());
-
   bool inject_js = injector_->ShouldInjectJs(run_location_);
   bool inject_css = injector_->ShouldInjectCss(run_location_);
   DCHECK(inject_js || inject_css);
 
-  scoped_ptr<base::ListValue> execution_results(new base::ListValue());
   GURL top_url = web_frame_->top()->document().url();
   for (std::vector<blink::WebFrame*>::iterator iter = frame_vector.begin();
        iter != frame_vector.end();
        ++iter) {
     // TODO(dcheng): Unfortunately, the code as written won't work in an OOPI
     // world. This is just a temporary hack to make things compile.
     blink::WebLocalFrame* frame = (*iter)->toWebLocalFrame();
 
     // We recheck access here in the renderer for extra safety against races
     // with navigation, but different frames can have different URLs, and the
     // injection host might only have access to a subset of them.
     // For child frames, we just skip ones the injection host doesn't have
     // access to and carry on.
     // Note: we don't consider ACCESS_WITHHELD because there is nowhere to
     // surface a request for a child frame.
     // TODO(rdevlin.cronin): We should ask for permission somehow.
     if (injector_->CanExecuteOnFrame(injection_host, frame, tab_id_, top_url) ==
         PermissionsData::ACCESS_DENIED) {
       DCHECK(frame->parent());
       continue;
     }
     if (inject_js)
-      InjectJs(injection_host, frame, execution_results.get());
+      InjectJs(injection_host, frame);
     if (inject_css)
       InjectCss(frame);
   }
 
-  complete_ = true;
-
-  // TODO(hanxi): don't log these metrics for webUIs' injections.
-  injector_->OnInjectionComplete(execution_results.Pass(),
-                                 scripts_run_info,
-                                 run_location_);
+  all_injection_started_ = true;
+  injector_->GetRunInfo(scripts_run_info, run_location_);
+  TryToFinish();

[Devlin, 2015/02/27 18:25:32]

This TryToFinish seems wrong, since it will notify the ScriptInjectionManager. 
Instead, I think this should just say:
if (complete_)
  injector_->OnInjectionComplete(...)

[kozy, 2015/02/28 15:23:21]

If we have synchronous injections in two frames then first injection will call
TryToFinish in OnJSInjectionCompleted. This first call will change nothing
because running_frames = 0 but all_injection_started is false. Then we start
second injection, it call TryToFinish and change nothing too with the same
reason. And we need to call TryToFinish one more time in this case.
We can change complete_ only in TryToFinish methods and I suppose we can't just
check if(complete_) without TryToFinish call.

+  return complete_ ? INJECTION_FINISHED : INJECTION_BLOCKED;
 }
 
 void ScriptInjection::InjectJs(const InjectionHost* injection_host,
-                               blink::WebLocalFrame* frame,
-                               base::ListValue* execution_results) {
+                               blink::WebLocalFrame* frame) {
+  running_frames_++;

[Devlin, 2015/02/27 18:25:32]

nit: chrome style prefers pre-increment.

[kozy, 2015/02/28 15:23:21]

Done.

   std::vector<blink::WebScriptSource> sources =
       injector_->GetJsSources(run_location_);
   bool in_main_world = injector_->ShouldExecuteInMainWorld();
   int world_id = in_main_world
                      ? DOMActivityLogger::kMainWorldId
                      : GetIsolatedWorldIdForInstance(injection_host, frame);
-  bool expects_results = injector_->ExpectsResults();
+  bool is_user_gesture = injector_->IsUserGesture();
+
+  scoped_ptr<blink::WebScriptExecutionCallback> callback(
+    new ScriptInjectionCallback(this, frame));

[Devlin, 2015/02/27 18:25:32]

nit: indentation off.

[kozy, 2015/02/28 15:23:21]

Done.

 
   base::ElapsedTimer exec_timer;
   if (injection_host->id().type() == HostID::EXTENSIONS)
     DOMActivityLogger::AttachToWorld(world_id, injection_host->id().id());
-  v8::HandleScope scope(v8::Isolate::GetCurrent());
-  v8::Local<v8::Value> script_value;
+
   if (in_main_world) {
     // We only inject in the main world for javascript: urls.
     DCHECK_EQ(1u, sources.size());
 
-    const blink::WebScriptSource& source = sources.front();
-    if (expects_results)
-      script_value = frame->executeScriptAndReturnValue(source);
-    else
-      frame->executeScript(source);
+    frame->requestExecuteScriptAndReturnValue(sources.front(),
+                                              is_user_gesture,
+                                              callback.release());
   } else {  // in isolated world
-    scoped_ptr<blink::WebVector<v8::Local<v8::Value> > > results;
-    if (expects_results)
-      results.reset(new blink::WebVector<v8::Local<v8::Value> >());
-    frame->executeScriptInIsolatedWorld(world_id,
-                                        &sources.front(),
-                                        sources.size(),
-                                        EXTENSION_GROUP_CONTENT_SCRIPTS,
-                                        results.get());
-    if (expects_results && !results->isEmpty())
-      script_value = (*results)[0];
+    frame->requestExecuteScriptInIsolatedWorld(world_id,
+                                               &sources.front(),
+                                               sources.size(),
+                                               EXTENSION_GROUP_CONTENT_SCRIPTS,
+                                               is_user_gesture,
+                                               callback.release());
   }
 
   if (injection_host->id().type() == HostID::EXTENSIONS)
     UMA_HISTOGRAM_TIMES("Extensions.InjectScriptTime", exec_timer.Elapsed());
+}
 
+void ScriptInjection::OnJSInjectionCompleted(
+    blink::WebLocalFrame* frame,
+    const blink::WebVector<v8::Local<v8::Value> >& results) {
+  DCHECK(running_frames_ > 0);
+  --running_frames_;
+
+  bool expects_results = injector_->ExpectsResults();
   if (expects_results) {
+    v8::Local<v8::Value> script_value;
+    if (!results.isEmpty())
+      script_value = results[0];
     // Right now, we only support returning single results (per frame).
     scoped_ptr<content::V8ValueConverter> v8_converter(
         content::V8ValueConverter::create());
     // It's safe to always use the main world context when converting
     // here. V8ValueConverterImpl shouldn't actually care about the
     // context scope, and it switches to v8::Object's creation context
     // when encountered.
     v8::Local<v8::Context> context = frame->mainWorldScriptContext();
     scoped_ptr<base::Value> result(
         v8_converter->FromV8Value(script_value, context));
-    // Always append an execution result (i.e. no result == null result)
-    // so that |execution_results| lines up with the frames.
-    execution_results->Append(result.get() ? result.release()
-                                           : base::Value::CreateNullValue());
+    // if current frame is main frame in injection

[Devlin, 2015/02/27 18:25:32]

This comment isn't really helpful (it just describes the if-statement). 
Instead, let's put a comment like:
// We guarantee that the main frame's result is at the first index, but
// any sub frames results do not have guaranteed order.

[kozy, 2015/02/28 15:23:21]

Done.

+    if (frame == web_frame_) {
+      execution_results_->Insert(0, result.get()
+                                      ? result.release()
+                                      : base::Value::CreateNullValue());

[Devlin, 2015/02/27 18:25:32]

instead of having this result.get() ? logic twice, above the if-statement for
the frames, do:
if (!result)
  result.reset(new base::Value::CreateNullValue());

and then when appending you can just do result.release().

Also, let's take out the frame if, and just do:
execution_results_->Insert(
    frame == web_frame_ ? 0 : execution_results_->GetSize(), result.get());

[kozy, 2015/02/28 15:23:21]

Done.

+    } else  {
+      execution_results_->Append(result.get() ? result.release()
+                                              : base::Value::CreateNullValue());
+    }
+  }
+  TryToFinish();
+}
+
+void ScriptInjection::SetScriptInjectionManager(
+  ScriptInjectionManager* manager){

[Devlin, 2015/02/27 18:25:32]

nit: indenation off.

[kozy, 2015/02/28 15:23:21]

Done.

+  script_injection_manager_ = manager;
+}
+
+void ScriptInjection::TryToFinish() {
+  if (all_injection_started_ && running_frames_ == 0) {
+    complete_ = true;
+    injector_->OnInjectionComplete(execution_results_.Pass(),
+                                   run_location_);
+
+    CHECK(script_injection_manager_);
+    // this object can be destroyed after next line
+    script_injection_manager_->OnInjectionFinished(this);
   }
 }
 
 void ScriptInjection::InjectCss(blink::WebLocalFrame* frame) {
   std::vector<std::string> css_sources =
       injector_->GetCssSources(run_location_);
   for (std::vector<std::string>::const_iterator iter = css_sources.begin();
        iter != css_sources.end();
        ++iter) {
     frame->document().insertStyleSheet(blink::WebString::fromUTF8(*iter));
   }
 }
 
 }  // namespace extensions