From d2af19f2da8348a7cf569e6be0636cff2addf33f Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Tue, 29 Aug 2017 06:37:47 +0000 Subject: [PATCH] Revert "Inspector: Runtime.callFunctionOn to accept executionContextId" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit de839c5671b56d0a1fd03b0a1f163af9332836ed. 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 > Reviewed-by: Aleksey Kozyatinskiy > Commit-Queue: Andrey Lushnikov > 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/640590 Reviewed-by: Michael Achenbach Commit-Queue: Michael Achenbach Cr-Commit-Position: refs/heads/master@{#47661} --- src/inspector/js_protocol.json | 6 +- src/inspector/v8-runtime-agent-impl.cc | 220 +++++++----------- src/inspector/v8-runtime-agent-impl.h | 5 +- .../call-function-on-async-expected.txt | 39 ---- .../runtime/call-function-on-async.js | 38 +-- 5 files changed, 92 insertions(+), 216 deletions(-) diff --git a/src/inspector/js_protocol.json b/src/inspector/js_protocol.json index af15ad9f52..439de72c10 100644 --- a/src/inspector/js_protocol.json +++ b/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 setPauseOnException 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 await 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 await for resulting value and return once awaited promise is resolved." } ], "returns": [ { "name": "result", "$ref": "RemoteObject", "description": "Call result." }, diff --git a/src/inspector/v8-runtime-agent-impl.cc b/src/inspector/v8-runtime-agent-impl.cc index 4f7881ca72..bdae0ef7a1 100644 --- a/src/inspector/v8-runtime-agent-impl.cc +++ b/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 recv, const String16& expression, - Maybe> optionalArguments, - bool silent, bool returnByValue, bool generatePreview, bool userGesture, - bool awaitPromise, const String16& objectGroup, - std::unique_ptr callback) { - V8InspectorImpl* inspector = session->inspector(); - - std::unique_ptr[]> argv = nullptr; - int argc = 0; - if (optionalArguments.isJust()) { - protocol::Array* arguments = - optionalArguments.fromJust(); - argc = static_cast(arguments->length()); - argv.reset(new v8::Local[argc]); - for (int i = 0; i < argc; ++i) { - v8::Local 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 maybeFunctionValue; - v8::Local 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 functionValue; - if (!maybeFunctionValue.ToLocal(&functionValue) || - !functionValue->IsFunction()) { - callback->sendFailure( - Response::Error("Given expression does not evaluate to a function")); - return; - } - - v8::MaybeLocal maybeResultValue; - { - v8::MicrotasksScope microtasksScope(inspector->isolate(), - v8::MicrotasksScope::kRunMicrotasks); - maybeResultValue = functionValue.As()->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::wrap( - std::move(callback))); -} - Response ensureContext(V8InspectorImpl* inspector, int contextGroupId, Maybe executionContextId, int* contextId) { if (executionContextId.isJust()) { @@ -309,59 +218,100 @@ void V8RuntimeAgentImpl::awaitPromise( } void V8RuntimeAgentImpl::callFunctionOn( - Maybe objectId, const String16& expression, + const String16& objectId, const String16& expression, Maybe> optionalArguments, Maybe silent, Maybe returnByValue, Maybe generatePreview, Maybe userGesture, Maybe awaitPromise, - Maybe executionContextId, Maybe objectGroup, std::unique_ptr 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; } - if (!objectId.isJust() && !executionContextId.isJust()) { - callback->sendFailure(Response::Error( - "Either ObjectId or executionContextId must be specified")); + + std::unique_ptr[]> argv = nullptr; + int argc = 0; + if (optionalArguments.isJust()) { + protocol::Array* arguments = + optionalArguments.fromJust(); + argc = static_cast(arguments->length()); + argv.reset(new v8::Local[argc]); + for (int i = 0; i < argc; ++i) { + v8::Local 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 maybeFunctionValue; + v8::Local 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 (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)); + + if (scope.tryCatch().HasCaught()) { + wrapEvaluateResultAsync(scope.injectedScript(), maybeFunctionValue, + scope.tryCatch(), scope.objectGroupName(), false, + false, callback.get()); + return; } + + v8::Local functionValue; + if (!maybeFunctionValue.ToLocal(&functionValue) || + !functionValue->IsFunction()) { + callback->sendFailure( + Response::Error("Given expression does not evaluate to a function")); + return; + } + + v8::MaybeLocal maybeResultValue; + { + v8::MicrotasksScope microtasksScope(m_inspector->isolate(), + v8::MicrotasksScope::kRunMicrotasks); + maybeResultValue = functionValue.As()->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::wrap( + std::move(callback))); } Response V8RuntimeAgentImpl::getProperties( diff --git a/src/inspector/v8-runtime-agent-impl.h b/src/inspector/v8-runtime-agent-impl.h index ba0cfc52d3..6f3b98cf44 100644 --- a/src/inspector/v8-runtime-agent-impl.h +++ b/src/inspector/v8-runtime-agent-impl.h @@ -69,12 +69,11 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend { Maybe generatePreview, std::unique_ptr) override; void callFunctionOn( - Maybe objectId, const String16& expression, + const String16& objectId, const String16& expression, Maybe> optionalArguments, Maybe silent, Maybe returnByValue, Maybe generatePreview, Maybe userGesture, - Maybe awaitPromise, Maybe executionContextId, - Maybe objectGroup, + Maybe awaitPromise, std::unique_ptr) override; Response releaseObject(const String16& objectId) override; Response getProperties( diff --git a/test/inspector/runtime/call-function-on-async-expected.txt b/test/inspector/runtime/call-function-on-async-expected.txt index 0cbf6f1c08..ed6342996a 100644 --- a/test/inspector/runtime/call-function-on-async-expected.txt +++ b/test/inspector/runtime/call-function-on-async-expected.txt @@ -68,24 +68,6 @@ Running test: testExceptionInFunctionExpression exceptionId : lineNumber : 0 scriptId : - stackTrace : { - callFrames : [ - [0] : { - columnNumber : 21 - functionName : - lineNumber : 0 - scriptId : - url : - } - [1] : { - columnNumber : 35 - functionName : - lineNumber : 0 - scriptId : - url : - } - ] - } text : Uncaught } result : { @@ -172,24 +154,3 @@ Running test: testFunctionReturnRejectedPromise } } } - -Running test: testEvaluateOnExecutionContext -{ - id : - result : { - result : { - description : 70 - type : number - value : 70 - } - } -} - -Running test: testPassingBothObjectIdAndExecutionContextId -{ - error : { - code : -32000 - message : ObjectId must not be specified together with executionContextId - } - id : -} diff --git a/test/inspector/runtime/call-function-on-async.js b/test/inspector/runtime/call-function-on-async.js index 277a01c468..a08b0777a6 100644 --- a/test/inspector/runtime/call-function-on-async.js +++ b/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 => {