[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..9e1817ca2a444e55392eb59d6685252e78cec5a9 100644
--- a/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
+++ b/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
@@ -81,7 +81,8 @@ class V8FunctionExecutor : public SuspendableScriptExecutor::Executor {
                      v8::Local<v8::Function>,
                      v8::Local<v8::Value> receiver,
                      int argc,
-                     v8::Local<v8::Value> argv[]);
+                     v8::Local<v8::Value> argv[],
+                     bool userGesture);
 
   Vector<v8::Local<v8::Value>> execute(LocalFrame*) override;
 
@@ -90,6 +91,7 @@ class V8FunctionExecutor : public SuspendableScriptExecutor::Executor {
   ScopedPersistent<v8::Value> m_receiver;
   V8PersistentValueVector<v8::Value> m_args;
   RefPtr<ScriptState> m_scriptState;
+  bool m_userGesture;
 };
 
 V8FunctionExecutor::V8FunctionExecutor(v8::Isolate* isolate,
@@ -97,11 +99,13 @@ V8FunctionExecutor::V8FunctionExecutor(v8::Isolate* isolate,
                                        v8::Local<v8::Function> function,
                                        v8::Local<v8::Value> receiver,
                                        int argc,
-                                       v8::Local<v8::Value> argv[])
+                                       v8::Local<v8::Value> argv[],
+                                       bool userGesture)
     : m_function(isolate, function),
       m_receiver(isolate, receiver),
       m_args(isolate),
-      m_scriptState(scriptState) {
+      m_scriptState(scriptState),
+      m_userGesture(userGesture) {
   m_args.ReserveCapacity(argc);
   for (int i = 0; i < argc; ++i)
     m_args.Append(argv[i]);
@@ -118,12 +122,22 @@ Vector<v8::Local<v8::Value>> V8FunctionExecutor::execute(LocalFrame* frame) {
   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> indicator;
+    if (m_userGesture && !UserGestureIndicator::processingUserGesture()) {

[Devlin, 2016/10/26 22:45:22]

I'm very unsure about this.

I originally started by always creating a new gesture, as
WebScriptExecutor::execute() does, but that resulted in too many new gestures on
the stack.  As a result, if we entered this code multiple times (which can
happen easily during extension API calls), something that normally would consume
the gesture (like window.open()) now only consumed a single gesture, leaving
more on the stack.  (You can see this test failure in patch set 1.)

Is there something better to be doing here?  The fact that gestures are counted,
rather than a binary yes-no, is something that we don't have much in chrome
code, so I'm not sure how best to handle it...

[dcheng, 2016/10/26 23:16:49]

I believe we should be saving the original UserGestureToken and using it here.
Here's an example where DOMTimer does it:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/DOM...

[Devlin, 2016/10/27 23:33:28]

Tried it out; ptal.

+      indicator =
+          wrapUnique(new UserGestureIndicator(DocumentUserGestureToken::create(
+              frame->document(), UserGestureToken::NewGesture)));
+    }
+
+    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;
 }
 
@@ -158,8 +172,9 @@ void SuspendableScriptExecutor::createAndRun(
     return;
   }
   SuspendableScriptExecutor* executor = new SuspendableScriptExecutor(
-      frame, callback, new V8FunctionExecutor(isolate, scriptState, function,
-                                              receiver, argc, argv));
+      frame, callback, new V8FunctionExecutor(
+                           isolate, scriptState, function, receiver, argc, argv,
+                           UserGestureIndicator::processingUserGesture()));
   executor->run();
 }