Revert "[inspector] added timeout argument for Runtime.evaluate"

This reverts commit deb875f7.

Reason for revert: ASAN failure closes tree. 
https://ci.chromium.org/buildbot/client.v8/V8%20Mac64%20ASAN/17377 


Original change's description:
> [inspector] added timeout argument for Runtime.evaluate
> 
> R=​yangguo@chromium.org,dgozman@chromium.org
> 
> Bug: none
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I31667b3d5f39db9d899d58acd5205a9c34e570db
> Reviewed-on: https://chromium-review.googlesource.com/1005985
> Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#52594}

TBR=dgozman@chromium.org,yangguo@chromium.org,kozyatinskiy@chromium.org

Change-Id: I61f996143d8c6436cbf9d3905d103047578aff0c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: none
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1012562Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52595}

src/api.cc

@@ -9467,10 +9467,6 @@ void debug::RemoveBreakpoint(Isolate* v8_isolate, BreakpointId id) {
   isolate->debug()->RemoveBreakpoint(id);
 }
 
-v8::Platform* debug::GetCurrentPlatform() {
-  return i::V8::GetCurrentPlatform();
-}
-
 debug::WasmScript* debug::WasmScript::Cast(debug::Script* script) {
   CHECK(script->IsWasm());
   return static_cast<WasmScript*>(script);

src/debug/debug-interface.h

@@ -514,8 +514,6 @@ int GetDebuggingId(v8::Local<v8::Function> function);
 bool SetFunctionBreakpoint(v8::Local<v8::Function> function,
                            v8::Local<v8::String> condition, BreakpointId* id);
 
-v8::Platform* GetCurrentPlatform();
-
 }  // namespace debug
 }  // namespace v8
 

src/inspector/js_protocol.json

@@ -2251,11 +2251,6 @@
                     "description": "Number of milliseconds since epoch.",
                     "type": "number"
                 },
-                {
-                    "id": "TimeDelta",
-                    "description": "Number of milliseconds.",
-                    "type": "number"
-                },
                 {
                     "id": "CallFrame",
                     "description": "Stack entry for runtime errors and assertions.",
@@ -2582,13 +2577,6 @@
                             "experimental": true,
                             "optional": true,
                             "type": "boolean"
-                        },
-                        {
-                            "name": "timeout",
-                            "description": "Terminate execution after timing out (number of milliseconds).",
-                            "experimental": true,
-                            "optional": true,
-                            "$ref": "TimeDelta"
                         }
                     ],
                     "returns": [

src/inspector/js_protocol.pdl

@@ -1046,9 +1046,6 @@ domain Runtime
   # Number of milliseconds since epoch.
   type Timestamp extends number
 
-  # Number of milliseconds.
-  type TimeDelta extends number
-
   # Stack entry for runtime errors and assertions.
   type CallFrame extends object
     properties
@@ -1192,8 +1189,6 @@ domain Runtime
       optional boolean awaitPromise
       # Whether to throw an exception if side effect cannot be ruled out during evaluation.
       experimental optional boolean throwOnSideEffect
-      # Terminate execution after timing out (number of milliseconds).
-      experimental optional TimeDelta timeout
     returns
       # Evaluation result.
       RemoteObject result

src/inspector/v8-debugger.cc

@@ -168,10 +168,12 @@ V8Debugger::V8Debugger(v8::Isolate* isolate, V8InspectorImpl* inspector)
       m_wasmTranslation(isolate) {}
 
 V8Debugger::~V8Debugger() {
-  m_isolate->RemoveCallCompletedCallback(
-      &V8Debugger::terminateExecutionCompletedCallback);
-  m_isolate->RemoveMicrotasksCompletedCallback(
-      &V8Debugger::terminateExecutionCompletedCallback);
+  if (m_terminateExecutionCallback) {
+    m_isolate->RemoveCallCompletedCallback(
+        &V8Debugger::terminateExecutionCompletedCallback);
+    m_isolate->RemoveMicrotasksCompletedCallback(
+        &V8Debugger::terminateExecutionCompletedCallback);
+  }
 }
 
 void V8Debugger::enable() {
@@ -341,10 +343,8 @@ void V8Debugger::pauseOnAsyncCall(int targetContextGroupId, uintptr_t task,
 void V8Debugger::terminateExecution(
     std::unique_ptr<TerminateExecutionCallback> callback) {
   if (m_terminateExecutionCallback) {
-    if (callback) {
-      callback->sendFailure(
-          Response::Error("There is current termination request in progress"));
-    }
+    callback->sendFailure(
+        Response::Error("There is current termination request in progress"));
     return;
   }
   m_terminateExecutionCallback = std::move(callback);
@@ -364,10 +364,8 @@ void V8Debugger::terminateExecutionCompletedCallback(v8::Isolate* isolate) {
       static_cast<V8InspectorImpl*>(v8::debug::GetInspector(isolate));
   V8Debugger* debugger = inspector->debugger();
   debugger->m_isolate->CancelTerminateExecution();
-  if (debugger->m_terminateExecutionCallback) {
-    debugger->m_terminateExecutionCallback->sendSuccess();
-    debugger->m_terminateExecutionCallback.reset();
-  }
+  debugger->m_terminateExecutionCallback->sendSuccess();
+  debugger->m_terminateExecutionCallback.reset();
 }
 
 Response V8Debugger::continueToLocation(

src/inspector/v8-inspector-impl.cc

@@ -376,18 +376,4 @@ void V8InspectorImpl::forEachSession(
   }
 }
 
-intptr_t V8InspectorImpl::evaluateStarted() {
-  intptr_t id = ++m_lastEvaluateId;
-  m_runningEvaluates.insert(id);
-  return id;
-}
-
-void V8InspectorImpl::evaluateFinished(intptr_t id) {
-  m_runningEvaluates.erase(id);
-}
-
-bool V8InspectorImpl::evaluateStillRunning(intptr_t id) {
-  return m_runningEvaluates.find(id) != m_runningEvaluates.end();
-}
-
 }  // namespace v8_inspector

src/inspector/v8-inspector-impl.h

@@ -33,7 +33,6 @@
 
 #include <functional>
 #include <map>
-#include <set>
 
 #include "src/base/macros.h"
 #include "src/inspector/protocol/Protocol.h"
@@ -121,10 +120,6 @@ class V8InspectorImpl : public V8Inspector {
   void forEachSession(int contextGroupId,
                       std::function<void(V8InspectorSessionImpl*)> callback);
 
-  intptr_t evaluateStarted();
-  void evaluateFinished(intptr_t);
-  bool evaluateStillRunning(intptr_t);
-
  private:
   v8::Isolate* m_isolate;
   V8InspectorClient* m_client;
@@ -156,9 +151,6 @@ class V8InspectorImpl : public V8Inspector {
 
   std::unique_ptr<V8Console> m_console;
 
-  intptr_t m_lastEvaluateId = 0;
-  std::set<intptr_t> m_runningEvaluates;
-
   DISALLOW_COPY_AND_ASSIGN(V8InspectorImpl);
 };
 

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

@@ -214,31 +214,6 @@ Response ensureContext(V8InspectorImpl* inspector, int contextGroupId,
   return Response::OK();
 }
 
-class TerminateTask : public v8::Task {
- public:
-  TerminateTask(V8InspectorImpl* inspector, intptr_t evaluateId)
-      : m_inspector(inspector), m_evaluateId(evaluateId) {}
-
-  void Run() {
-    if (!m_inspector->evaluateStillRunning(m_evaluateId)) return;
-    // We should call terminateExecution on the main thread.
-    m_inspector->isolate()->RequestInterrupt(
-        InterruptCallback, reinterpret_cast<void*>(m_evaluateId));
-  }
-
-  static void InterruptCallback(v8::Isolate* isolate, void* rawEvaluateId) {
-    intptr_t evaluateId = reinterpret_cast<intptr_t>(rawEvaluateId);
-    V8InspectorImpl* inspector =
-        static_cast<V8InspectorImpl*>(v8::debug::GetInspector(isolate));
-    if (!inspector->evaluateStillRunning(evaluateId)) return;
-    inspector->debugger()->terminateExecution(nullptr);
-  }
-
- private:
-  V8InspectorImpl* m_inspector;
-  intptr_t m_evaluateId;
-};
-
 }  // namespace
 
 V8RuntimeAgentImpl::V8RuntimeAgentImpl(
@@ -258,7 +233,7 @@ void V8RuntimeAgentImpl::evaluate(
     Maybe<int> executionContextId, Maybe<bool> returnByValue,
     Maybe<bool> generatePreview, Maybe<bool> userGesture,
     Maybe<bool> awaitPromise, Maybe<bool> throwOnSideEffect,
-    Maybe<double> timeout, std::unique_ptr<EvaluateCallback> callback) {
+    std::unique_ptr<EvaluateCallback> callback) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
                "EvaluateScript");
   int contextId = 0;
@@ -284,21 +259,6 @@ void V8RuntimeAgentImpl::evaluate(
   // Temporarily enable allow evals for inspector.
   scope.allowCodeGenerationFromStrings();
 
-  V8InspectorImpl* inspector = m_inspector;
-  intptr_t evaluateId = inspector->evaluateStarted();
-  if (timeout.isJust()) {
-    std::shared_ptr<v8::TaskRunner> taskRunner =
-        v8::debug::GetCurrentPlatform()->GetWorkerThreadsTaskRunner(
-            m_inspector->isolate());
-    if (!taskRunner) {
-      callback->sendFailure(
-          Response::Error("Timeout is not supported by embedder"));
-      return;
-    }
-    taskRunner->PostDelayedTask(
-        v8::base::make_unique<TerminateTask>(m_inspector, evaluateId),
-        timeout.fromJust() / 1000.0);
-  }
   v8::MaybeLocal<v8::Value> maybeResultValue;
   {
     v8::MicrotasksScope microtasksScope(m_inspector->isolate(),
@@ -307,7 +267,6 @@ void V8RuntimeAgentImpl::evaluate(
         m_inspector->isolate(), toV8String(m_inspector->isolate(), expression),
         throwOnSideEffect.fromMaybe(false));
   }  // Run microtasks before returning result.
-  inspector->evaluateFinished(evaluateId);
 
   // Re-initialize after running client's code, as it could have destroyed
   // context or session.

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

@@ -64,7 +64,6 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
                 Maybe<int> executionContextId, Maybe<bool> returnByValue,
                 Maybe<bool> generatePreview, Maybe<bool> userGesture,
                 Maybe<bool> awaitPromise, Maybe<bool> throwOnSideEffect,
-                Maybe<double> timeout,
                 std::unique_ptr<EvaluateCallback>) override;
   void awaitPromise(const String16& promiseObjectId, Maybe<bool> returnByValue,
                     Maybe<bool> generatePreview,

src/libplatform/default-worker-threads-task-runner.cc

@@ -40,12 +40,8 @@ void DefaultWorkerThreadsTaskRunner::PostDelayedTask(std::unique_ptr<Task> task,
                                                      double delay_in_seconds) {
   base::LockGuard<base::Mutex> guard(&lock_);
   if (terminated_) return;
-  if (delay_in_seconds == 0) {
-    queue_.Append(std::move(task));
-    return;
-  }
-  // There is no use case for this function with non zero delay_in_second on a
-  // worker thread at the moment, but it is still part of the interface.
+  // There is no use case for this function on a worker thread at the
+  // moment, but it is still part of the interface.
   UNIMPLEMENTED();
 }
 

test/inspector/runtime/evaluate-timeout-expected.txt

-Tests that Runtime.evaluate's timeout argument
-Run trivial expression:
-Execution was terminated
-Run expression without interrupts:
-{
-    type : undefined
-}
-Run infinite loop:
-Execution was terminated

test/inspector/runtime/evaluate-timeout.js

-// 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.
-
-const {Protocol} = InspectorTest.start(
-    `Tests that Runtime.evaluate's timeout argument`);
-
-(async function test(){
-  {
-    InspectorTest.log('Run trivial expression:');
-    const {error:{message}} = await Protocol.Runtime.evaluate({
-      expression: 'function foo() {} foo()',
-      timeout: 0
-    });
-    InspectorTest.log(message);
-  }
-  {
-    InspectorTest.log('Run expression without interrupts:');
-    const {result:{result}} = await Protocol.Runtime.evaluate({
-      expression: '',
-      timeout: 0
-    });
-    InspectorTest.logMessage(result);
-  }
-  {
-    InspectorTest.log('Run infinite loop:');
-    const {error:{message}} = await Protocol.Runtime.evaluate({
-      expression: 'for(;;){}',
-      timeout: 0
-    });
-    InspectorTest.log(message);
-  }
-  InspectorTest.completeTest();
-})();