[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}

src/api.cc

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

src/inspector/v8-inspector-impl.cc

@@ -376,4 +376,18 @@ 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,6 +33,7 @@
 
 #include <functional>
 #include <map>
+#include <set>
 
 #include "src/base/macros.h"
 #include "src/inspector/protocol/Protocol.h"
@@ -120,6 +121,10 @@ 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;
@@ -151,6 +156,9 @@ 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,6 +214,31 @@ 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(
@@ -233,7 +258,7 @@ void V8RuntimeAgentImpl::evaluate(
     Maybe<int> executionContextId, Maybe<bool> returnByValue,
     Maybe<bool> generatePreview, Maybe<bool> userGesture,
     Maybe<bool> awaitPromise, Maybe<bool> throwOnSideEffect,
-    std::unique_ptr<EvaluateCallback> callback) {
+    Maybe<double> timeout, std::unique_ptr<EvaluateCallback> callback) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
                "EvaluateScript");
   int contextId = 0;
@@ -259,6 +284,21 @@ 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(),
@@ -267,6 +307,7 @@ 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,6 +64,7 @@ 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,8 +40,12 @@ void DefaultWorkerThreadsTaskRunner::PostDelayedTask(std::unique_ptr<Task> task,
                                                      double delay_in_seconds) {
   base::LockGuard<base::Mutex> guard(&lock_);
   if (terminated_) return;
-  // There is no use case for this function on a worker thread at the
-  // moment, but it is still part of the interface.
+  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.
   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();
+})();