[Blink] Modify SuspendableScriptExecutor to take a v8::Function

extensions/renderer/script_context.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_context.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_split.h"
@@ skipping 184 matching lines @@
   }
 
   v8::Local<v8::Object> global = v8_context()->Global();
   if (!web_frame_)
     return handle_scope.Escape(function->Call(global, argc, argv));
   return handle_scope.Escape(
       v8::Local<v8::Value>(web_frame_->callFunctionEvenIfScriptDisabled(
           function, global, argc, argv)));
 }
 
+void ScriptContext::SafeCallFunction(
+    const v8::Local<v8::Function>& function,
+    int argc,
+    v8::Local<v8::Value> argv[]) {
+  v8::HandleScope handle_scope(isolate());
+  v8::Context::Scope scope(v8_context());
+  v8::MicrotasksScope microtasks(
+      isolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
+  v8::Local<v8::Object> global = v8_context()->Global();
+  if (web_frame_) {
+    web_frame_->requestExecuteV8Function(function, global, argc, argv, nullptr);
+  } else {

[Devlin, 2016/09/16 18:16:31]

This can happen in the case of e.g. ServiceWorker contexts.  What should we do
here?

[dcheng, 2016/09/27 08:30:57]

Handwaving a bit... maybe this is OK? I don't think service workers can go into
nested run loops or do things that would cause script execution to be
suspended... so in theory, we shouldn't up doing work in nested message loops
that surprises code above it in the stack.

+falken to confirm

[falken, 2016/09/28 00:04:14]

+nhiroki, +yhirano

I probably don't understand enough to give an authoritative answer. dcheng tells
me sync APIs are the key to nested run loops. Service worker in principle
doesn't have sync APIs except for importScripts(). During the importScripts call
we wouldn't be dispatching an event to the SW, since the browser process waits
for the SW to finish starting up before it sends messages to the renderer to
dispatch events. Also currently  FileReaderSync is available but that is a bug
<https://github.com/w3c/ServiceWorker/issues/735>.

If it's relevant, SW also can't terminate itself synchronously: the browser
process orders the renderer to terminate the worker. This used to result in a
terminateExecution but it changed a bit recently in https://crbug.com/487050 to
have a more graceful shutdown.

[yhirano, 2016/09/28 05:55:03]

This topic is about sync loading on a worker thread, right? My understanding is
it's not related with a nested message loop. Sync loading on a worker thread is
implemented in WorkerThreadableLoader[1] and it blocks the worker thread
completely. On the other hand, the main thread is working normally.

1:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/Wo...

+    // TODO(devlin): This probably isn't safe.
+    function->Call(global, argc, argv);
+  }
+}
+
 v8::Local<v8::Value> ScriptContext::CallFunction(
     const v8::Local<v8::Function>& function) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return CallFunction(function, 0, nullptr);
 }
 
 Feature::Availability ScriptContext::GetAvailability(
     const std::string& api_name) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (base::StartsWith(api_name, "test", base::CompareCase::SENSITIVE)) {
@@ skipping 286 matching lines @@
     v8::Local<v8::Value> argv[]) {
   return context_->CallFunction(function, argc, argv);
 }
 
 gin::ContextHolder* ScriptContext::Runner::GetContextHolder() {
   v8::HandleScope handle_scope(context_->isolate());
   return gin::PerContextData::From(context_->v8_context())->context_holder();
 }
 
 }  // namespace extensions