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

extensions/renderer/script_context.cc

Index: extensions/renderer/script_context.cc
diff --git a/extensions/renderer/script_context.cc b/extensions/renderer/script_context.cc
index f611ddebf7112e9d6fb6b63aa5d993fb77fc4c07..051a928a7ffecb1e60e2e9f1415efc8ea7369356 100644
--- a/extensions/renderer/script_context.cc
+++ b/extensions/renderer/script_context.cc
@@ -202,6 +202,23 @@ v8::Local<v8::Value> ScriptContext::CallFunction(
           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());