Reland of Inspector: Runtime.callFunctionOn to accept executionContextId

This patch:
- teaches Runtime.callFunctionOn to accept executionContextId instead of
  objectId.
- adds the optional objectGroup parameter to the Runtime.callFunctionOn.

R=kozy
TBR=pfeldman
BUG=chromium:760367

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I024654860f23a9e79fb57865ba5bd472692ea526
Reviewed-on: https://chromium-review.googlesource.com/641921
Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47725}

src/inspector/js_protocol.json

@@ -242,14 +242,16 @@
             {
                 "name": "callFunctionOn",
                 "parameters": [
-                    { "name": "objectId", "$ref": "RemoteObjectId", "description": "Identifier of the object to call function on." },
                     { "name": "functionDeclaration", "type": "string", "description": "Declaration of the function to call." },
+                    { "name": "objectId", "$ref": "RemoteObjectId", "optional": true, "description": "Identifier of the object to call function on. Either objectId or executionContextId should be specified." },
                     { "name": "arguments", "type": "array", "items": { "$ref": "CallArgument", "description": "Call argument." }, "optional": true, "description": "Call arguments. All call arguments must belong to the same JavaScript world as the target object." },
                     { "name": "silent", "type": "boolean", "optional": true, "description": "In silent mode exceptions thrown during evaluation are not reported and do not pause execution. Overrides <code>setPauseOnException</code> state." },
                     { "name": "returnByValue", "type": "boolean", "optional": true, "description": "Whether the result is expected to be a JSON object which should be sent by value." },
                     { "name": "generatePreview", "type": "boolean", "optional": true, "experimental": true, "description": "Whether preview should be generated for the result." },
                     { "name": "userGesture", "type": "boolean", "optional": true, "experimental": true, "description": "Whether execution should be treated as initiated by user in the UI." },
-                    { "name": "awaitPromise", "type": "boolean", "optional":true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." }
+                    { "name": "awaitPromise", "type": "boolean", "optional":true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." },
+                    { "name": "executionContextId", "$ref": "ExecutionContextId", "optional": true, "description": "Specifies execution context which global object will be used to call function on. Either executionContextId or objectId should be specified." },
+                    { "name": "objectGroup", "type": "string", "optional": true, "description": "Symbolic group name that can be used to release multiple objects. If objectGroup is not specified and objectId is, objectGroup will be inherited from object." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "RemoteObject", "description": "Call result." },

src/inspector/v8-runtime-agent-impl.cc

@@ -104,6 +104,97 @@ bool wrapEvaluateResultAsync(InjectedScript* injectedScript,
   return false;
 }
 
+void innerCallFunctionOn(
+    V8InspectorSessionImpl* session, InjectedScript::Scope& scope,
+    v8::Local<v8::Value> recv, const String16& expression,
+    Maybe<protocol::Array<protocol::Runtime::CallArgument>> optionalArguments,
+    bool silent, bool returnByValue, bool generatePreview, bool userGesture,
+    bool awaitPromise, const String16& objectGroup,
+    std::unique_ptr<V8RuntimeAgentImpl::CallFunctionOnCallback> callback) {
+  V8InspectorImpl* inspector = session->inspector();
+
+  std::unique_ptr<v8::Local<v8::Value>[]> argv = nullptr;
+  int argc = 0;
+  if (optionalArguments.isJust()) {
+    protocol::Array<protocol::Runtime::CallArgument>* arguments =
+        optionalArguments.fromJust();
+    argc = static_cast<int>(arguments->length());
+    argv.reset(new v8::Local<v8::Value>[argc]);
+    for (int i = 0; i < argc; ++i) {
+      v8::Local<v8::Value> argumentValue;
+      Response response = scope.injectedScript()->resolveCallArgument(
+          arguments->get(i), &argumentValue);
+      if (!response.isSuccess()) {
+        callback->sendFailure(response);
+        return;
+      }
+      argv[i] = argumentValue;
+    }
+  }
+
+  if (silent) scope.ignoreExceptionsAndMuteConsole();
+  if (userGesture) scope.pretendUserGesture();
+
+  v8::MaybeLocal<v8::Value> maybeFunctionValue;
+  v8::Local<v8::Script> functionScript;
+  if (inspector
+          ->compileScript(scope.context(), "(" + expression + ")", String16())
+          .ToLocal(&functionScript)) {
+    v8::MicrotasksScope microtasksScope(inspector->isolate(),
+                                        v8::MicrotasksScope::kRunMicrotasks);
+    maybeFunctionValue = functionScript->Run(scope.context());
+  }
+  // Re-initialize after running client's code, as it could have destroyed
+  // context or session.
+  Response response = scope.initialize();
+  if (!response.isSuccess()) {
+    callback->sendFailure(response);
+    return;
+  }
+
+  if (scope.tryCatch().HasCaught()) {
+    wrapEvaluateResultAsync(scope.injectedScript(), maybeFunctionValue,
+                            scope.tryCatch(), objectGroup, false, false,
+                            callback.get());
+    return;
+  }
+
+  v8::Local<v8::Value> functionValue;
+  if (!maybeFunctionValue.ToLocal(&functionValue) ||
+      !functionValue->IsFunction()) {
+    callback->sendFailure(
+        Response::Error("Given expression does not evaluate to a function"));
+    return;
+  }
+
+  v8::MaybeLocal<v8::Value> maybeResultValue;
+  {
+    v8::MicrotasksScope microtasksScope(inspector->isolate(),
+                                        v8::MicrotasksScope::kRunMicrotasks);
+    maybeResultValue = functionValue.As<v8::Function>()->Call(
+        scope.context(), recv, argc, argv.get());
+  }
+  // Re-initialize after running client's code, as it could have destroyed
+  // context or session.
+  response = scope.initialize();
+  if (!response.isSuccess()) {
+    callback->sendFailure(response);
+    return;
+  }
+
+  if (!awaitPromise || scope.tryCatch().HasCaught()) {
+    wrapEvaluateResultAsync(scope.injectedScript(), maybeResultValue,
+                            scope.tryCatch(), objectGroup, returnByValue,
+                            generatePreview, callback.get());
+    return;
+  }
+
+  scope.injectedScript()->addPromiseCallback(
+      session, maybeResultValue, objectGroup, returnByValue, generatePreview,
+      EvaluateCallbackWrapper<V8RuntimeAgentImpl::CallFunctionOnCallback>::wrap(
+          std::move(callback)));
+}
+
 Response ensureContext(V8InspectorImpl* inspector, int contextGroupId,
                        Maybe<int> executionContextId, int* contextId) {
   if (executionContextId.isJust()) {
@@ -218,100 +309,59 @@ void V8RuntimeAgentImpl::awaitPromise(
 }
 
 void V8RuntimeAgentImpl::callFunctionOn(
-    const String16& objectId, const String16& expression,
+    const String16& expression, Maybe<String16> objectId,
     Maybe<protocol::Array<protocol::Runtime::CallArgument>> optionalArguments,
     Maybe<bool> silent, Maybe<bool> returnByValue, Maybe<bool> generatePreview,
     Maybe<bool> userGesture, Maybe<bool> awaitPromise,
+    Maybe<int> executionContextId, Maybe<String16> objectGroup,
     std::unique_ptr<CallFunctionOnCallback> callback) {
-  InjectedScript::ObjectScope scope(m_session, objectId);
-  Response response = scope.initialize();
-  if (!response.isSuccess()) {
-    callback->sendFailure(response);
-    return;
-  }
-
-  std::unique_ptr<v8::Local<v8::Value>[]> argv = nullptr;
-  int argc = 0;
-  if (optionalArguments.isJust()) {
-    protocol::Array<protocol::Runtime::CallArgument>* arguments =
-        optionalArguments.fromJust();
-    argc = static_cast<int>(arguments->length());
-    argv.reset(new v8::Local<v8::Value>[argc]);
-    for (int i = 0; i < argc; ++i) {
-      v8::Local<v8::Value> argumentValue;
-      response = scope.injectedScript()->resolveCallArgument(arguments->get(i),
-                                                             &argumentValue);
-      if (!response.isSuccess()) {
-        callback->sendFailure(response);
-        return;
-      }
-      argv[i] = argumentValue;
-    }
-  }
-
-  if (silent.fromMaybe(false)) scope.ignoreExceptionsAndMuteConsole();
-  if (userGesture.fromMaybe(false)) scope.pretendUserGesture();
-
-  v8::MaybeLocal<v8::Value> maybeFunctionValue;
-  v8::Local<v8::Script> functionScript;
-  if (m_inspector
-          ->compileScript(scope.context(), "(" + expression + ")", String16())
-          .ToLocal(&functionScript)) {
-    v8::MicrotasksScope microtasksScope(m_inspector->isolate(),
-                                        v8::MicrotasksScope::kRunMicrotasks);
-    maybeFunctionValue = functionScript->Run(scope.context());
-  }
-  // Re-initialize after running client's code, as it could have destroyed
-  // context or session.
-  response = scope.initialize();
-  if (!response.isSuccess()) {
-    callback->sendFailure(response);
-    return;
-  }
-
-  if (scope.tryCatch().HasCaught()) {
-    wrapEvaluateResultAsync(scope.injectedScript(), maybeFunctionValue,
-                            scope.tryCatch(), scope.objectGroupName(), false,
-                            false, callback.get());
+  if (objectId.isJust() && executionContextId.isJust()) {
+    callback->sendFailure(Response::Error(
+        "ObjectId must not be specified together with executionContextId"));
     return;
   }
-
-  v8::Local<v8::Value> functionValue;
-  if (!maybeFunctionValue.ToLocal(&functionValue) ||
-      !functionValue->IsFunction()) {
-    callback->sendFailure(
-        Response::Error("Given expression does not evaluate to a function"));
+  if (!objectId.isJust() && !executionContextId.isJust()) {
+    callback->sendFailure(Response::Error(
+        "Either ObjectId or executionContextId must be specified"));
     return;
   }
-
-  v8::MaybeLocal<v8::Value> maybeResultValue;
-  {
-    v8::MicrotasksScope microtasksScope(m_inspector->isolate(),
-                                        v8::MicrotasksScope::kRunMicrotasks);
-    maybeResultValue = functionValue.As<v8::Function>()->Call(
-        scope.context(), scope.object(), argc, argv.get());
-  }
-  // Re-initialize after running client's code, as it could have destroyed
-  // context or session.
-  response = scope.initialize();
-  if (!response.isSuccess()) {
-    callback->sendFailure(response);
-    return;
-  }
-
-  if (!awaitPromise.fromMaybe(false) || scope.tryCatch().HasCaught()) {
-    wrapEvaluateResultAsync(scope.injectedScript(), maybeResultValue,
-                            scope.tryCatch(), scope.objectGroupName(),
-                            returnByValue.fromMaybe(false),
-                            generatePreview.fromMaybe(false), callback.get());
-    return;
+  if (objectId.isJust()) {
+    InjectedScript::ObjectScope scope(m_session, objectId.fromJust());
+    Response response = scope.initialize();
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
+    innerCallFunctionOn(
+        m_session, scope, scope.object(), expression,
+        std::move(optionalArguments), silent.fromMaybe(false),
+        returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
+        userGesture.fromMaybe(false), awaitPromise.fromMaybe(false),
+        objectGroup.isJust() ? objectGroup.fromMaybe(String16())
+                             : scope.objectGroupName(),
+        std::move(callback));
+  } else {
+    int contextId = 0;
+    Response response =
+        ensureContext(m_inspector, m_session->contextGroupId(),
+                      std::move(executionContextId.fromJust()), &contextId);
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
+    InjectedScript::ContextScope scope(m_session, contextId);
+    response = scope.initialize();
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
+    innerCallFunctionOn(
+        m_session, scope, scope.context()->Global(), expression,
+        std::move(optionalArguments), silent.fromMaybe(false),
+        returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
+        userGesture.fromMaybe(false), awaitPromise.fromMaybe(false),
+        objectGroup.fromMaybe(""), std::move(callback));
   }
-
-  scope.injectedScript()->addPromiseCallback(
-      m_session, maybeResultValue, scope.objectGroupName(),
-      returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
-      EvaluateCallbackWrapper<CallFunctionOnCallback>::wrap(
-          std::move(callback)));
 }
 
 Response V8RuntimeAgentImpl::getProperties(

src/inspector/v8-runtime-agent-impl.h

@@ -69,11 +69,12 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
                     Maybe<bool> generatePreview,
                     std::unique_ptr<AwaitPromiseCallback>) override;
   void callFunctionOn(
-      const String16& objectId, const String16& expression,
+      const String16& expression, Maybe<String16> objectId,
       Maybe<protocol::Array<protocol::Runtime::CallArgument>> optionalArguments,
       Maybe<bool> silent, Maybe<bool> returnByValue,
       Maybe<bool> generatePreview, Maybe<bool> userGesture,
-      Maybe<bool> awaitPromise,
+      Maybe<bool> awaitPromise, Maybe<int> executionContextId,
+      Maybe<String16> objectGroup,
       std::unique_ptr<CallFunctionOnCallback>) override;
   Response releaseObject(const String16& objectId) override;
   Response getProperties(

test/inspector/runtime/call-function-on-async-expected.txt

@@ -68,6 +68,24 @@ Running test: testExceptionInFunctionExpression
             exceptionId : <exceptionId>
             lineNumber : 0
             scriptId : <scriptId>
+            stackTrace : {
+                callFrames : [
+                    [0] : {
+                        columnNumber : 21
+                        functionName : 
+                        lineNumber : 0
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                    [1] : {
+                        columnNumber : 35
+                        functionName : 
+                        lineNumber : 0
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                ]
+            }
             text : Uncaught
         }
         result : {
@@ -154,3 +172,24 @@ Running test: testFunctionReturnRejectedPromise
         }
     }
 }
+
+Running test: testEvaluateOnExecutionContext
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 70
+            type : number
+            value : 70
+        }
+    }
+}
+
+Running test: testPassingBothObjectIdAndExecutionContextId
+{
+    error : {
+        code : -32000
+        message : ObjectId must not be specified together with executionContextId
+    }
+    id : <messageId>
+}

test/inspector/runtime/call-function-on-async.js

@@ -7,13 +7,22 @@ let callFunctionOn = Protocol.Runtime.callFunctionOn.bind(Protocol.Runtime);
 
 let remoteObject1;
 let remoteObject2;
+let executionContextId;
 
-InspectorTest.runAsyncTestSuite([
+Protocol.Runtime.enable();
+Protocol.Runtime.onExecutionContextCreated(messageObject => {
+  executionContextId = messageObject.params.context.id;
+  InspectorTest.runAsyncTestSuite(testSuite);
+});
+
+let testSuite = [
   async function prepareTestSuite() {
     let result = await Protocol.Runtime.evaluate({ expression: '({a : 1})' });
     remoteObject1 = result.result.result;
     result = await Protocol.Runtime.evaluate({ expression: '({a : 2})' });
     remoteObject2 = result.result.result;
+
+    await Protocol.Runtime.evaluate({ expression: 'globalObjectProperty = 42;' });
   },
 
   async function testArguments() {
@@ -102,8 +111,31 @@ InspectorTest.runAsyncTestSuite([
       generatePreview: false,
       awaitPromise: true
     }));
-  }
-]);
+  },
+
+  async function testEvaluateOnExecutionContext() {
+    InspectorTest.logMessage(await callFunctionOn({
+      executionContextId,
+      functionDeclaration: '(function(arg) { return this.globalObjectProperty + arg; })',
+      arguments: prepareArguments([ 28 ]),
+      returnByValue: true,
+      generatePreview: false,
+      awaitPromise: false
+    }));
+  },
+
+  async function testPassingBothObjectIdAndExecutionContextId() {
+    InspectorTest.logMessage(await callFunctionOn({
+      executionContextId,
+      objectId: remoteObject1.objectId,
+      functionDeclaration: '(function() { return 42; })',
+      arguments: prepareArguments([]),
+      returnByValue: true,
+      generatePreview: false,
+      awaitPromise: false
+    }));
+  },
+];
 
 function prepareArguments(args) {
   return args.map(arg => {