Revert "Inspector: Runtime.callFunctionOn to accept executionContextId"

This reverts commit de839c56.

Reason for revert: Breaks chromium compilation, e.g.:
https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Debug%20Builder/builds/6010


Original change's description:
> 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
> 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: Ia29ee37f37a1e8cbe2d9f15ae75e841534ecf727
> Reviewed-on: https://chromium-review.googlesource.com/639751
> Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
> Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47659}

TBR=lushnikov@chromium.org,pfeldman@chromium.org,kozyatinskiy@chromium.org

Change-Id: I2586a6accde6c1f79d628b8999d90222b5714dc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/640590Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47661}

src/inspector/js_protocol.json

@@ -242,16 +242,14 @@
             {
                 "name": "callFunctionOn",
                 "parameters": [
-                    { "name": "objectId", "$ref": "RemoteObjectId", "optional": true, "description": "Identifier of the object to call function on. Either objectId or executionContextId should be specified." },
+                    { "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": "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": "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." }
+                    { "name": "awaitPromise", "type": "boolean", "optional":true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "RemoteObject", "description": "Call result." },

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

@@ -104,97 +104,6 @@ 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()) {
@@ -309,59 +218,100 @@ void V8RuntimeAgentImpl::awaitPromise(
 }
 
 void V8RuntimeAgentImpl::callFunctionOn(
-    Maybe<String16> objectId, const String16& expression,
+    const String16& objectId, const String16& expression,
     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) {
-  if (objectId.isJust() && executionContextId.isJust()) {
-    callback->sendFailure(Response::Error(
-        "ObjectId must not be specified together with executionContextId"));
+  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());
     return;
   }
-  if (!objectId.isJust() && !executionContextId.isJust()) {
-    callback->sendFailure(Response::Error(
-        "Either ObjectId or executionContextId must be specified"));
+
+  v8::Local<v8::Value> functionValue;
+  if (!maybeFunctionValue.ToLocal(&functionValue) ||
+      !functionValue->IsFunction()) {
+    callback->sendFailure(
+        Response::Error("Given expression does not evaluate to a function"));
     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));
+
+  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;
+  }
+
+  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,12 +69,11 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
                     Maybe<bool> generatePreview,
                     std::unique_ptr<AwaitPromiseCallback>) override;
   void callFunctionOn(
-      Maybe<String16> objectId, const String16& expression,
+      const String16& objectId, const String16& expression,
       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,
+      Maybe<bool> awaitPromise,
       std::unique_ptr<CallFunctionOnCallback>) override;
   Response releaseObject(const String16& objectId) override;
   Response getProperties(

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

@@ -68,24 +68,6 @@ 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 : {
@@ -172,24 +154,3 @@ 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,22 +7,13 @@ let callFunctionOn = Protocol.Runtime.callFunctionOn.bind(Protocol.Runtime);
 
 let remoteObject1;
 let remoteObject2;
-let executionContextId;
 
-Protocol.Runtime.enable();
-Protocol.Runtime.onExecutionContextCreated(messageObject => {
-  executionContextId = messageObject.params.context.id;
-  InspectorTest.runAsyncTestSuite(testSuite);
-});
-
-let testSuite = [
+InspectorTest.runAsyncTestSuite([
   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() {
@@ -111,31 +102,8 @@ let testSuite = [
       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 => {