[Extensions + Blink] Account for user gesture in v8 function calls

third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp

Index: third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
diff --git a/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp b/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
index 20aba329814e771e03d166318aecd919a4079194..ebbdeca71d7ea644688e7715a6d518e38ca02e67 100644
--- a/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
+++ b/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
@@ -7,6 +7,7 @@
 #include "bindings/core/v8/ScriptController.h"
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "bindings/core/v8/V8PersistentValueVector.h"
+#include "bindings/core/v8/WindowProxy.h"
 #include "core/dom/Document.h"
 #include "core/dom/DocumentUserGestureToken.h"
 #include "core/frame/LocalFrame.h"
@@ -77,7 +78,6 @@ Vector<v8::Local<v8::Value>> WebScriptExecutor::execute(LocalFrame* frame) {
 class V8FunctionExecutor : public SuspendableScriptExecutor::Executor {
  public:
   V8FunctionExecutor(v8::Isolate*,
-                     ScriptState*,
                      v8::Local<v8::Function>,
                      v8::Local<v8::Value> receiver,
                      int argc,
@@ -89,11 +89,10 @@ class V8FunctionExecutor : public SuspendableScriptExecutor::Executor {
   ScopedPersistent<v8::Function> m_function;
   ScopedPersistent<v8::Value> m_receiver;
   V8PersistentValueVector<v8::Value> m_args;
-  RefPtr<ScriptState> m_scriptState;
+  RefPtr<UserGestureToken> m_gestureToken;
 };
 
 V8FunctionExecutor::V8FunctionExecutor(v8::Isolate* isolate,
-                                       ScriptState* scriptState,
                                        v8::Local<v8::Function> function,
                                        v8::Local<v8::Value> receiver,
                                        int argc,
@@ -101,7 +100,7 @@ V8FunctionExecutor::V8FunctionExecutor(v8::Isolate* isolate,
     : m_function(isolate, function),
       m_receiver(isolate, receiver),
       m_args(isolate),
-      m_scriptState(scriptState) {
+      m_gestureToken(UserGestureIndicator::currentToken()) {
   m_args.ReserveCapacity(argc);
   for (int i = 0; i < argc; ++i)
     m_args.Append(argv[i]);
@@ -110,20 +109,24 @@ V8FunctionExecutor::V8FunctionExecutor(v8::Isolate* isolate,
 Vector<v8::Local<v8::Value>> V8FunctionExecutor::execute(LocalFrame* frame) {
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
   Vector<v8::Local<v8::Value>> results;
-  if (!m_scriptState->contextIsValid())
-    return results;
-  ScriptState::Scope scope(m_scriptState.get());
   v8::Local<v8::Value> singleResult;
   Vector<v8::Local<v8::Value>> args;
   args.reserveCapacity(m_args.Size());
   for (size_t i = 0; i < m_args.Size(); ++i)
     args.append(m_args.Get(i));
-  if (V8ScriptRunner::callFunction(m_function.newLocal(isolate),
-                                   frame->document(),
-                                   m_receiver.newLocal(isolate), args.size(),
-                                   args.data(), toIsolate(frame))
-          .ToLocal(&singleResult))
-    results.append(singleResult);
+  {
+    std::unique_ptr<UserGestureIndicator> gestureIndicator;
+    if (m_gestureToken) {
+      gestureIndicator =
+          wrapUnique(new UserGestureIndicator(m_gestureToken.release()));
+    }
+    if (V8ScriptRunner::callFunction(m_function.newLocal(isolate),
+                                     frame->document(),
+                                     m_receiver.newLocal(isolate), args.size(),
+                                     args.data(), toIsolate(frame))
+            .ToLocal(&singleResult))
+      results.append(singleResult);
+  }
   return results;
 }
 
@@ -136,8 +139,11 @@ void SuspendableScriptExecutor::createAndRun(
     int extensionGroup,
     bool userGesture,
     WebScriptExecutionCallback* callback) {
+  ScriptState* scriptState = ScriptState::forWorld(
+      frame, *DOMWrapperWorld::fromWorldId(v8::Isolate::GetCurrent(), worldID,

[haraken, 2016/11/03 14:37:04]

Use toIsolate(frame), or explicitly pass in v8::Isolate* to this method (which
is better).

[Devlin, 2016/11/04 05:40:32]

Went with toIsolate(frame) for now, but added a TODO to update.  I'm trying to
keep the churn low in this CL (though that ship may have already sailed).

+                                           extensionGroup));
   SuspendableScriptExecutor* executor = new SuspendableScriptExecutor(
-      frame, callback,
+      frame, scriptState, callback,
       new WebScriptExecutor(sources, worldID, extensionGroup, userGesture));
   executor->run();
 }
@@ -158,8 +164,8 @@ void SuspendableScriptExecutor::createAndRun(
     return;
   }
   SuspendableScriptExecutor* executor = new SuspendableScriptExecutor(
-      frame, callback, new V8FunctionExecutor(isolate, scriptState, function,
-                                              receiver, argc, argv));
+      frame, scriptState, callback,
+      new V8FunctionExecutor(isolate, function, receiver, argc, argv));
   executor->run();
 }
 
@@ -172,10 +178,12 @@ void SuspendableScriptExecutor::contextDestroyed() {
 
 SuspendableScriptExecutor::SuspendableScriptExecutor(
     LocalFrame* frame,
+    ScriptState* scriptState,
     WebScriptExecutionCallback* callback,
     Executor* executor)
     : SuspendableTimer(frame->document()),
       m_frame(frame),
+      m_scriptState(scriptState),
       m_callback(callback),
       m_keepAlive(this),
       m_executor(executor) {}
@@ -199,16 +207,20 @@ void SuspendableScriptExecutor::run() {
 }
 
 void SuspendableScriptExecutor::executeAndDestroySelf() {
-  v8::HandleScope scope(v8::Isolate::GetCurrent());
+  CHECK(m_scriptState->contextIsValid());
+
+  ScriptState::Scope scriptScope(m_scriptState.get());
   Vector<v8::Local<v8::Value>> results = m_executor->execute(m_frame);
 
   // The script may have removed the frame, in which case contextDestroyed()
   // will have handled the disposal/callback.
-  if (!m_frame->client())
+  if (!lifecycleContext())

[haraken, 2016/11/03 14:37:04]

This will work but I'd prefer checking m_scriptState->contextIsValid() instead.

[Devlin, 2016/11/04 05:40:32]

Done.

     return;
 
+  // We need to call the callback before scriptScope goes out of scope.

[haraken, 2016/11/03 14:37:04]

This comment doesn't make much sense now.

[Devlin, 2016/11/04 05:40:32]

Whoops, done.

   if (m_callback)
     m_callback->completed(results);
+
   dispose();
 }