From af667f934e228565b5f6268483bce64ba18fd37b Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Wed, 14 Mar 2018 07:54:37 +0000 Subject: [PATCH] Revert "[inspector] added Runtime.terminateExecution" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 98dec8f24026df57dd1b3d42833cb84c79a8250e. Reason for revert: Speculative revert as win32/64 debug seems to hang after this: https://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20debug/builds/13691 Original change's description: > [inspector] added Runtime.terminateExecution > > Runtime.terminateExecution terminates current or next JavaScript > call. Termination flag is automatically reset as soon as v8 call > or microtasks are completed. > > R=​pfeldman@chromium.org > > Bug: chromium:820640 > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel > Change-Id: Ie21c123be3a61fe25cf6e04c38a8b6c664622ed7 > Reviewed-on: https://chromium-review.googlesource.com/957386 > Commit-Queue: Aleksey Kozyatinskiy > Reviewed-by: Dmitry Gozman > Cr-Commit-Position: refs/heads/master@{#51912} TBR=dgozman@chromium.org,pfeldman@chromium.org,kozyatinskiy@chromium.org Change-Id: I25258ca5e9a2c2c514f0834da0ef0f5e75421d52 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:820640 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/962002 Reviewed-by: Michael Achenbach Commit-Queue: Michael Achenbach Cr-Commit-Position: refs/heads/master@{#51914} --- src/inspector/injected-script.cc | 3 - src/inspector/inspector_protocol_config.json | 2 +- src/inspector/js_protocol.json | 5 -- src/inspector/js_protocol.pdl | 4 -- src/inspector/v8-debugger.cc | 47 +------------ src/inspector/v8-debugger.h | 7 -- src/inspector/v8-inspector-session-impl.cc | 5 +- src/inspector/v8-runtime-agent-impl.cc | 5 -- src/inspector/v8-runtime-agent-impl.h | 2 - .../runtime/terminate-execution-expected.txt | 66 ------------------- test/inspector/runtime/terminate-execution.js | 60 ----------------- 11 files changed, 4 insertions(+), 202 deletions(-) delete mode 100644 test/inspector/runtime/terminate-execution-expected.txt delete mode 100644 test/inspector/runtime/terminate-execution.js diff --git a/src/inspector/injected-script.cc b/src/inspector/injected-script.cc index d51b201bce..741e092ce9 100644 --- a/src/inspector/injected-script.cc +++ b/src/inspector/injected-script.cc @@ -618,9 +618,6 @@ Response InjectedScript::wrapEvaluateResult( m_lastEvaluationResult.AnnotateStrongRetainer(kGlobalHandleLabel); } } else { - if (tryCatch.HasTerminated() || !tryCatch.CanContinue()) { - return Response::Error("Execution was terminated"); - } v8::Local exception = tryCatch.Exception(); Response response = wrapObject(exception, objectGroup, false, diff --git a/src/inspector/inspector_protocol_config.json b/src/inspector/inspector_protocol_config.json index fa073128b3..fdb2b64b90 100644 --- a/src/inspector/inspector_protocol_config.json +++ b/src/inspector/inspector_protocol_config.json @@ -11,7 +11,7 @@ }, { "domain": "Runtime", - "async": ["evaluate", "awaitPromise", "callFunctionOn", "runScript", "terminateExecution"], + "async": ["evaluate", "awaitPromise", "callFunctionOn", "runScript"], "exported": ["StackTrace", "StackTraceId", "RemoteObject", "ExecutionContextId"] }, { diff --git a/src/inspector/js_protocol.json b/src/inspector/js_protocol.json index 2ad7deff7c..069bfe43c2 100644 --- a/src/inspector/js_protocol.json +++ b/src/inspector/js_protocol.json @@ -2788,11 +2788,6 @@ "type": "boolean" } ] - }, - { - "name": "terminateExecution", - "description": "Terminate current or next JavaScript execution.\nWill cancel the termination when the outer-most script execution ends.", - "experimental": true } ], "events": [ diff --git a/src/inspector/js_protocol.pdl b/src/inspector/js_protocol.pdl index 9380a17f92..bab5f6c645 100644 --- a/src/inspector/js_protocol.pdl +++ b/src/inspector/js_protocol.pdl @@ -1280,10 +1280,6 @@ domain Runtime parameters boolean enabled - # Terminate current or next JavaScript execution. - # Will cancel the termination when the outer-most script execution ends. - experimental command terminateExecution - # Issued when console API was called. event consoleAPICalled parameters diff --git a/src/inspector/v8-debugger.cc b/src/inspector/v8-debugger.cc index 29bb1218db..9b0ca38018 100644 --- a/src/inspector/v8-debugger.cc +++ b/src/inspector/v8-debugger.cc @@ -155,12 +155,6 @@ class MatchPrototypePredicate : public v8::debug::QueryObjectPredicate { v8::Local m_prototype; }; -// TODO(kozyatinskiy): add V8Inspector* field on i::Isolate instead. -std::unordered_map& IsolateToDebuggerMap() { - static std::unordered_map map; - return map; -} - } // namespace V8Debugger::V8Debugger(v8::Isolate* isolate, V8InspectorImpl* inspector) @@ -172,19 +166,9 @@ V8Debugger::V8Debugger(v8::Isolate* isolate, V8InspectorImpl* inspector) m_maxAsyncCallStacks(kMaxAsyncTaskStacks), m_maxAsyncCallStackDepth(0), m_pauseOnExceptionsState(v8::debug::NoBreakOnException), - m_wasmTranslation(isolate) { - IsolateToDebuggerMap()[isolate] = this; -} + m_wasmTranslation(isolate) {} -V8Debugger::~V8Debugger() { - IsolateToDebuggerMap().erase(m_isolate); - if (m_terminateExecutionCallback) { - m_isolate->RemoveCallCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - m_isolate->RemoveMicrotasksCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - } -} +V8Debugger::~V8Debugger() {} void V8Debugger::enable() { if (m_enableCount++) return; @@ -350,33 +334,6 @@ void V8Debugger::pauseOnAsyncCall(int targetContextGroupId, uintptr_t task, m_taskWithScheduledBreakDebuggerId = debuggerId; } -void V8Debugger::terminateExecution( - std::unique_ptr callback) { - if (m_terminateExecutionCallback) { - callback->sendFailure( - Response::Error("There is current termination request in progress")); - return; - } - m_terminateExecutionCallback = std::move(callback); - m_isolate->AddCallCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - m_isolate->AddMicrotasksCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - m_isolate->TerminateExecution(); -} - -void V8Debugger::terminateExecutionCompletedCallback(v8::Isolate* isolate) { - isolate->RemoveCallCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - isolate->RemoveMicrotasksCompletedCallback( - &V8Debugger::terminateExecutionCompletedCallback); - V8Debugger* debugger = IsolateToDebuggerMap()[isolate]; - DCHECK(debugger); - debugger->m_isolate->CancelTerminateExecution(); - debugger->m_terminateExecutionCallback->sendSuccess(); - debugger->m_terminateExecutionCallback.reset(); -} - Response V8Debugger::continueToLocation( int targetContextGroupId, V8DebuggerScript* script, std::unique_ptr location, diff --git a/src/inspector/v8-debugger.h b/src/inspector/v8-debugger.h index dd4f784df7..a710726581 100644 --- a/src/inspector/v8-debugger.h +++ b/src/inspector/v8-debugger.h @@ -32,8 +32,6 @@ struct V8StackTraceId; using protocol::Response; using ScheduleStepIntoAsyncCallback = protocol::Debugger::Backend::ScheduleStepIntoAsyncCallback; -using TerminateExecutionCallback = - protocol::Runtime::Backend::TerminateExecutionCallback; class V8Debugger : public v8::debug::DebugDelegate { public: @@ -62,8 +60,6 @@ class V8Debugger : public v8::debug::DebugDelegate { void pauseOnAsyncCall(int targetContextGroupId, uintptr_t task, const String16& debuggerId); - void terminateExecution(std::unique_ptr callback); - Response continueToLocation(int targetContextGroupId, V8DebuggerScript* script, std::unique_ptr, @@ -136,7 +132,6 @@ class V8Debugger : public v8::debug::DebugDelegate { bool shouldContinueToCurrentLocation(); static void v8OOMCallback(void* data); - static void terminateExecutionCompletedCallback(v8::Isolate* isolate); void handleProgramBreak( v8::Local pausedContext, v8::Local exception, @@ -237,8 +232,6 @@ class V8Debugger : public v8::debug::DebugDelegate { protocol::HashMap> m_serializedDebuggerIdToDebuggerId; - std::unique_ptr m_terminateExecutionCallback; - WasmTranslation m_wasmTranslation; DISALLOW_COPY_AND_ASSIGN(V8Debugger); diff --git a/src/inspector/v8-inspector-session-impl.cc b/src/inspector/v8-inspector-session-impl.cc index 1d8d12ac0d..d580c41e30 100644 --- a/src/inspector/v8-inspector-session-impl.cc +++ b/src/inspector/v8-inspector-session-impl.cc @@ -197,11 +197,8 @@ Response V8InspectorSessionImpl::findInjectedScript( if (!context) return Response::Error("Cannot find context with specified id"); injectedScript = context->getInjectedScript(m_sessionId); if (!injectedScript) { - if (!context->createInjectedScript(m_sessionId)) { - if (m_inspector->isolate()->IsExecutionTerminating()) - return Response::Error("Execution was terminated"); + if (!context->createInjectedScript(m_sessionId)) return Response::Error("Cannot access specified execution context"); - } injectedScript = context->getInjectedScript(m_sessionId); if (m_customObjectFormatterEnabled) injectedScript->setCustomObjectFormatterEnabled(true); diff --git a/src/inspector/v8-runtime-agent-impl.cc b/src/inspector/v8-runtime-agent-impl.cc index 6b97dda6ce..54e39db56c 100644 --- a/src/inspector/v8-runtime-agent-impl.cc +++ b/src/inspector/v8-runtime-agent-impl.cc @@ -608,11 +608,6 @@ Response V8RuntimeAgentImpl::globalLexicalScopeNames( return Response::OK(); } -void V8RuntimeAgentImpl::terminateExecution( - std::unique_ptr callback) { - m_inspector->debugger()->terminateExecution(std::move(callback)); -} - void V8RuntimeAgentImpl::restore() { if (!m_state->booleanProperty(V8RuntimeAgentImplState::runtimeEnabled, false)) return; diff --git a/src/inspector/v8-runtime-agent-impl.h b/src/inspector/v8-runtime-agent-impl.h index 460ddcbe7a..43016a6efe 100644 --- a/src/inspector/v8-runtime-agent-impl.h +++ b/src/inspector/v8-runtime-agent-impl.h @@ -104,8 +104,6 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend { Response globalLexicalScopeNames( Maybe executionContextId, std::unique_ptr>* outNames) override; - void terminateExecution( - std::unique_ptr callback) override; void reset(); void reportExecutionContextCreated(InspectedContext*); diff --git a/test/inspector/runtime/terminate-execution-expected.txt b/test/inspector/runtime/terminate-execution-expected.txt deleted file mode 100644 index b64854cf80..0000000000 --- a/test/inspector/runtime/terminate-execution-expected.txt +++ /dev/null @@ -1,66 +0,0 @@ -Tests Runtime.terminateExecution -Terminate first evaluation (it forces injected-script-source compilation) -{ - id : - result : { - } -} -{ - error : { - code : -32000 - message : Cannot access specified execution context - } - id : -} -Checks that we reset termination after evaluation -{ - description : 42 - type : number - value : 42 -} -{ - id : - result : { - result : { - type : undefined - } - } -} -Terminate evaluation when injected-script-source already compiled -{ - id : - result : { - } -} -{ - error : { - code : -32000 - message : Execution was terminated - } - id : -} -Terminate script evaluated with v8 API -{ - id : - result : { - } -} -Terminate chained callback -Pause inside microtask and terminate execution -{ - id : - result : { - } -} -{ - type : string - value : separate eval after while(true) -} -{ - id : - result : { - result : { - type : undefined - } - } -} diff --git a/test/inspector/runtime/terminate-execution.js b/test/inspector/runtime/terminate-execution.js deleted file mode 100644 index feaf52eb2c..0000000000 --- a/test/inspector/runtime/terminate-execution.js +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2018 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -let {session, contextGroup, Protocol} = - InspectorTest.start('Tests Runtime.terminateExecution'); - -(async function test() { - Protocol.Runtime.enable(); - Protocol.Runtime.onConsoleAPICalled( - message => InspectorTest.logMessage(message.params.args[0])); - InspectorTest.log( - 'Terminate first evaluation (it forces injected-script-source compilation)'); - await Promise.all([ - Protocol.Runtime.terminateExecution().then(InspectorTest.logMessage), - Protocol.Runtime.evaluate({expression: 'console.log(42)'}) - .then(InspectorTest.logMessage) - ]); - - InspectorTest.log('Checks that we reset termination after evaluation'); - InspectorTest.logMessage( - await Protocol.Runtime.evaluate({expression: 'console.log(42)'})); - InspectorTest.log( - 'Terminate evaluation when injected-script-source already compiled'); - await Promise.all([ - Protocol.Runtime.terminateExecution().then(InspectorTest.logMessage), - Protocol.Runtime.evaluate({expression: 'console.log(42)'}) - .then(InspectorTest.logMessage) - ]); - - InspectorTest.log('Terminate script evaluated with v8 API'); - const terminated = - Protocol.Runtime.terminateExecution().then(InspectorTest.logMessage); - contextGroup.addScript('console.log(42)'); - await terminated; - - InspectorTest.log('Terminate chained callback'); - Protocol.Debugger.enable(); - const paused = Protocol.Debugger.oncePaused(); - await Protocol.Runtime.evaluate({ - expression: `let p = new Promise(resolve => setTimeout(resolve, 0)); - p.then(() => { - while(true){ debugger; } - }).then(() => console.log('chained after chained callback')); - p.then(() => { console.log('another chained callback'); }); - undefined` - }); - await paused; - - InspectorTest.log('Pause inside microtask and terminate execution'); - Protocol.Runtime.terminateExecution().then(InspectorTest.logMessage); - await Protocol.Debugger.resume(); - await Protocol.Runtime - .evaluate({expression: `console.log('separate eval after while(true)')`}) - .then(InspectorTest.logMessage); - await Protocol.Debugger.disable(); - - await Protocol.Runtime.disable(); - InspectorTest.completeTest(); -})();