[inspector] resolve async evaluation on context destroyed

On context destroyed we discard corresponded injected-script and won't be able to wrap async evaluation result, so we can resolve callback with an error right now.

R=dgozman@chromium.org

Bug: none
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib62f255297f306ad9f2c96a2a5b80e4b5aa33475
Reviewed-on: https://chromium-review.googlesource.com/604213
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47267}

src/inspector/injected-script.cc

@@ -49,7 +49,7 @@ namespace v8_inspector {
 
 namespace {
 static const char privateKeyName[] = "v8-inspector#injectedScript";
-}
+}  // namespace
 
 using protocol::Array;
 using protocol::Runtime::PropertyDescriptor;
@@ -57,6 +57,179 @@ using protocol::Runtime::InternalPropertyDescriptor;
 using protocol::Runtime::RemoteObject;
 using protocol::Maybe;
 
+class InjectedScript::ProtocolPromiseHandler {
+ public:
+  static bool add(V8InspectorSessionImpl* session,
+                  v8::Local<v8::Context> context,
+                  v8::Local<v8::Promise> promise,
+                  const String16& notPromiseError, int executionContextId,
+                  const String16& objectGroup, bool returnByValue,
+                  bool generatePreview, EvaluateCallback* callback) {
+    V8InspectorImpl* inspector = session->inspector();
+    ProtocolPromiseHandler* handler =
+        new ProtocolPromiseHandler(session, executionContextId, objectGroup,
+                                   returnByValue, generatePreview, callback);
+    v8::Local<v8::Value> wrapper = handler->m_wrapper.Get(inspector->isolate());
+    v8::Local<v8::Function> thenCallbackFunction =
+        v8::Function::New(context, thenCallback, wrapper, 0,
+                          v8::ConstructorBehavior::kThrow)
+            .ToLocalChecked();
+    if (promise->Then(context, thenCallbackFunction).IsEmpty()) {
+      callback->sendFailure(Response::InternalError());
+      return false;
+    }
+    v8::Local<v8::Function> catchCallbackFunction =
+        v8::Function::New(context, catchCallback, wrapper, 0,
+                          v8::ConstructorBehavior::kThrow)
+            .ToLocalChecked();
+    if (promise->Catch(context, catchCallbackFunction).IsEmpty()) {
+      callback->sendFailure(Response::InternalError());
+      return false;
+    }
+    return true;
+  }
+
+ private:
+  static void thenCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
+    ProtocolPromiseHandler* handler = static_cast<ProtocolPromiseHandler*>(
+        info.Data().As<v8::External>()->Value());
+    DCHECK(handler);
+    v8::Local<v8::Value> value =
+        info.Length() > 0
+            ? info[0]
+            : v8::Local<v8::Value>::Cast(v8::Undefined(info.GetIsolate()));
+    handler->thenCallback(value);
+    delete handler;
+  }
+
+  static void catchCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
+    ProtocolPromiseHandler* handler = static_cast<ProtocolPromiseHandler*>(
+        info.Data().As<v8::External>()->Value());
+    DCHECK(handler);
+    v8::Local<v8::Value> value =
+        info.Length() > 0
+            ? info[0]
+            : v8::Local<v8::Value>::Cast(v8::Undefined(info.GetIsolate()));
+    handler->catchCallback(value);
+    delete handler;
+  }
+
+  ProtocolPromiseHandler(V8InspectorSessionImpl* session,
+                         int executionContextId, const String16& objectGroup,
+                         bool returnByValue, bool generatePreview,
+                         EvaluateCallback* callback)
+      : m_inspector(session->inspector()),
+        m_sessionId(session->sessionId()),
+        m_contextGroupId(session->contextGroupId()),
+        m_executionContextId(executionContextId),
+        m_objectGroup(objectGroup),
+        m_returnByValue(returnByValue),
+        m_generatePreview(generatePreview),
+        m_callback(std::move(callback)),
+        m_wrapper(m_inspector->isolate(),
+                  v8::External::New(m_inspector->isolate(), this)) {
+    m_wrapper.SetWeak(this, cleanup, v8::WeakCallbackType::kParameter);
+  }
+
+  static void cleanup(
+      const v8::WeakCallbackInfo<ProtocolPromiseHandler>& data) {
+    if (!data.GetParameter()->m_wrapper.IsEmpty()) {
+      data.GetParameter()->m_wrapper.Reset();
+      data.SetSecondPassCallback(cleanup);
+    } else {
+      data.GetParameter()->m_callback->sendFailure(
+          Response::Error("Promise was collected"));
+      delete data.GetParameter();
+    }
+  }
+
+  void thenCallback(v8::Local<v8::Value> result) {
+    V8InspectorSessionImpl* session =
+        m_inspector->sessionById(m_contextGroupId, m_sessionId);
+    if (!session) return;
+    InjectedScript::ContextScope scope(session, m_executionContextId);
+    Response response = scope.initialize();
+    if (!response.isSuccess()) return;
+    if (m_objectGroup == "console") {
+      scope.injectedScript()->setLastEvaluationResult(result);
+    }
+    std::unique_ptr<EvaluateCallback> callback =
+        scope.injectedScript()->takeEvaluateCallback(m_callback);
+    if (!callback) return;
+    std::unique_ptr<protocol::Runtime::RemoteObject> wrappedValue;
+    response = scope.injectedScript()->wrapObject(
+        result, m_objectGroup, m_returnByValue, m_generatePreview,
+        &wrappedValue);
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
+    callback->sendSuccess(std::move(wrappedValue),
+                          Maybe<protocol::Runtime::ExceptionDetails>());
+  }
+
+  void catchCallback(v8::Local<v8::Value> result) {
+    V8InspectorSessionImpl* session =
+        m_inspector->sessionById(m_contextGroupId, m_sessionId);
+    if (!session) return;
+    InjectedScript::ContextScope scope(session, m_executionContextId);
+    Response response = scope.initialize();
+    if (!response.isSuccess()) return;
+    std::unique_ptr<EvaluateCallback> callback =
+        scope.injectedScript()->takeEvaluateCallback(m_callback);
+    if (!callback) return;
+    std::unique_ptr<protocol::Runtime::RemoteObject> wrappedValue;
+    response = scope.injectedScript()->wrapObject(
+        result, m_objectGroup, m_returnByValue, m_generatePreview,
+        &wrappedValue);
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
+    String16 message;
+    std::unique_ptr<V8StackTraceImpl> stack;
+    v8::Isolate* isolate = session->inspector()->isolate();
+    if (result->IsNativeError()) {
+      message = " " + toProtocolString(
+                          result->ToDetailString(isolate->GetCurrentContext())
+                              .ToLocalChecked());
+      v8::Local<v8::StackTrace> stackTrace = v8::debug::GetDetailedStackTrace(
+          isolate, v8::Local<v8::Object>::Cast(result));
+      if (!stackTrace.IsEmpty()) {
+        stack = m_inspector->debugger()->createStackTrace(stackTrace);
+      }
+    }
+    if (!stack) {
+      stack = m_inspector->debugger()->captureStackTrace(true);
+    }
+    std::unique_ptr<protocol::Runtime::ExceptionDetails> exceptionDetails =
+        protocol::Runtime::ExceptionDetails::create()
+            .setExceptionId(m_inspector->nextExceptionId())
+            .setText("Uncaught (in promise)" + message)
+            .setLineNumber(stack && !stack->isEmpty() ? stack->topLineNumber()
+                                                      : 0)
+            .setColumnNumber(
+                stack && !stack->isEmpty() ? stack->topColumnNumber() : 0)
+            .setException(wrappedValue->clone())
+            .build();
+    if (stack)
+      exceptionDetails->setStackTrace(stack->buildInspectorObjectImpl());
+    if (stack && !stack->isEmpty())
+      exceptionDetails->setScriptId(toString16(stack->topScriptId()));
+    callback->sendSuccess(std::move(wrappedValue), std::move(exceptionDetails));
+  }
+
+  V8InspectorImpl* m_inspector;
+  int m_sessionId;
+  int m_contextGroupId;
+  int m_executionContextId;
+  String16 m_objectGroup;
+  bool m_returnByValue;
+  bool m_generatePreview;
+  EvaluateCallback* m_callback;
+  v8::Global<v8::External> m_wrapper;
+};
+
 std::unique_ptr<InjectedScript> InjectedScript::create(
     InspectedContext* inspectedContext, int sessionId) {
   v8::Isolate* isolate = inspectedContext->isolate();
@@ -122,7 +295,7 @@ InjectedScript::InjectedScript(InspectedContext* context,
       m_value(context->isolate(), object),
       m_sessionId(sessionId) {}
 
-InjectedScript::~InjectedScript() {}
+InjectedScript::~InjectedScript() { discardEvaluateCallbacks(); }
 
 Response InjectedScript::getProperties(
     v8::Local<v8::Object> object, const String16& groupName, bool ownProperties,
@@ -269,6 +442,47 @@ std::unique_ptr<protocol::Runtime::RemoteObject> InjectedScript::wrapTable(
                                                     &errors);
 }
 
+void InjectedScript::addPromiseCallback(
+    V8InspectorSessionImpl* session, v8::MaybeLocal<v8::Value> value,
+    const String16& notPromiseError, const String16& objectGroup,
+    bool returnByValue, bool generatePreview,
+    std::unique_ptr<EvaluateCallback> callback) {
+  if (value.IsEmpty()) {
+    callback->sendFailure(Response::InternalError());
+    return;
+  }
+  if (!value.ToLocalChecked()->IsPromise()) {
+    callback->sendFailure(Response::Error(notPromiseError));
+    return;
+  }
+  v8::MicrotasksScope microtasksScope(m_context->isolate(),
+                                      v8::MicrotasksScope::kRunMicrotasks);
+  if (ProtocolPromiseHandler::add(session, m_context->context(),
+                                  value.ToLocalChecked().As<v8::Promise>(),
+                                  notPromiseError, m_context->contextId(),
+                                  objectGroup, returnByValue, generatePreview,
+                                  callback.get())) {
+    m_evaluateCallbacks.insert(callback.release());
+  }
+}
+
+void InjectedScript::discardEvaluateCallbacks() {
+  for (auto& callback : m_evaluateCallbacks) {
+    callback->sendFailure(Response::Error("Execution context was destroyed."));
+    delete callback;
+  }
+  m_evaluateCallbacks.clear();
+}
+
+std::unique_ptr<EvaluateCallback> InjectedScript::takeEvaluateCallback(
+    EvaluateCallback* callback) {
+  auto it = m_evaluateCallbacks.find(callback);
+  if (it == m_evaluateCallbacks.end()) return nullptr;
+  std::unique_ptr<EvaluateCallback> value(*it);
+  m_evaluateCallbacks.erase(it);
+  return value;
+}
+
 Response InjectedScript::findObject(const RemoteObjectId& objectId,
                                     v8::Local<v8::Value>* outObject) const {
   auto it = m_idToWrappedObject.find(objectId.id());

src/inspector/injected-script.h

@@ -32,6 +32,7 @@
 #define V8_INSPECTOR_INJECTEDSCRIPT_H_
 
 #include <unordered_map>
+#include <unordered_set>
 
 #include "src/base/macros.h"
 #include "src/inspector/inspected-context.h"
@@ -52,6 +53,16 @@ class V8InspectorSessionImpl;
 using protocol::Maybe;
 using protocol::Response;
 
+class EvaluateCallback {
+ public:
+  virtual void sendSuccess(
+      std::unique_ptr<protocol::Runtime::RemoteObject> result,
+      protocol::Maybe<protocol::Runtime::ExceptionDetails>
+          exceptionDetails) = 0;
+  virtual void sendFailure(const protocol::DispatchResponse& response) = 0;
+  virtual ~EvaluateCallback() {}
+};
+
 class InjectedScript final {
  public:
   static std::unique_ptr<InjectedScript> create(InspectedContext*,
@@ -86,6 +97,13 @@ class InjectedScript final {
   std::unique_ptr<protocol::Runtime::RemoteObject> wrapTable(
       v8::Local<v8::Value> table, v8::Local<v8::Value> columns) const;
 
+  void addPromiseCallback(V8InspectorSessionImpl* session,
+                          v8::MaybeLocal<v8::Value> value,
+                          const String16& notPromiseError,
+                          const String16& objectGroup, bool returnByValue,
+                          bool generatePreview,
+                          std::unique_ptr<EvaluateCallback> callback);
+
   Response findObject(const RemoteObjectId&, v8::Local<v8::Value>*) const;
   String16 objectGroupName(const RemoteObjectId&) const;
   void releaseObjectGroup(const String16&);
@@ -191,6 +209,11 @@ class InjectedScript final {
   v8::Local<v8::Object> commandLineAPI();
   void unbindObject(int id);
 
+  class ProtocolPromiseHandler;
+  void discardEvaluateCallbacks();
+  std::unique_ptr<EvaluateCallback> takeEvaluateCallback(
+      EvaluateCallback* callback);
+
   InspectedContext* m_context;
   v8::Global<v8::Value> m_value;
   int m_sessionId;
@@ -200,6 +223,7 @@ class InjectedScript final {
   std::unordered_map<int, v8::Global<v8::Value>> m_idToWrappedObject;
   std::unordered_map<int, String16> m_idToObjectGroupName;
   std::unordered_map<String16, std::vector<int>> m_nameToObjectGroup;
+  std::unordered_set<EvaluateCallback*> m_evaluateCallbacks;
 
   DISALLOW_COPY_AND_ASSIGN(InjectedScript);
 };

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

@@ -203,3 +203,29 @@ Running test: testLastEvaluatedResult
         }
     }
 }
+
+Running test: testRuntimeDisable
+Resolving promise..
+{
+    id : <messageId>
+    result : {
+        result : {
+            className : Object
+            description : Object
+            objectId : <objectId>
+            type : object
+        }
+    }
+}
+Promise resolved
+
+Running test: testImmediatelyResolvedAfterAfterContextDestroyed
+Destroying context..
+{
+    error : {
+        code : -32000
+        message : Execution context was destroyed.
+    }
+    id : <messageId>
+}
+Context destroyed

test/inspector/runtime/evaluate-async.js

@@ -30,73 +30,109 @@ function throwSyntaxError()
 }
 `);
 
-InspectorTest.runTestSuite([
-  function testResolvedPromise(next)
+InspectorTest.runAsyncTestSuite([
+  async function testResolvedPromise()
   {
-    Protocol.Runtime.evaluate({ expression: "Promise.resolve(239)", awaitPromise: true, generatePreview: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "Promise.resolve(239)",
+      awaitPromise: true,
+      generatePreview: true
+    }));
   },
 
-  function testRejectedPromise(next)
+  async function testRejectedPromise()
   {
-    Protocol.Runtime.evaluate({ expression: "Promise.reject(239)", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "Promise.reject(239)",
+      awaitPromise: true
+    }));
   },
 
-  function testRejectedPromiseWithError(next)
+  async function testRejectedPromiseWithError()
   {
     Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({ expression: "Promise.resolve().then(throwError)", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(Protocol.Runtime.disable)
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "Promise.resolve().then(throwError)",
+      awaitPromise: true
+    }));
+    await Protocol.Runtime.disable();
   },
 
-  function testRejectedPromiseWithSyntaxError(next)
+  async function testRejectedPromiseWithSyntaxError()
   {
     Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({ expression: "Promise.resolve().then(throwSyntaxError)", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(Protocol.Runtime.disable)
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "Promise.resolve().then(throwSyntaxError)",
+      awaitPromise: true
+    }));
+    await Protocol.Runtime.disable();
   },
 
-  function testPrimitiveValueInsteadOfPromise(next)
+  async function testPrimitiveValueInsteadOfPromise()
   {
-    Protocol.Runtime.evaluate({ expression: "true", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "true",
+      awaitPromise: true
+    }));
   },
 
-  function testObjectInsteadOfPromise(next)
+  async function testObjectInsteadOfPromise()
   {
-    Protocol.Runtime.evaluate({ expression: "({})", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "({})",
+      awaitPromise: true
+    }));
   },
 
-  function testPendingPromise(next)
+  async function testPendingPromise()
   {
-    Protocol.Runtime.evaluate({ expression: "createPromiseAndScheduleResolve()", awaitPromise: true, returnByValue: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "createPromiseAndScheduleResolve()",
+      awaitPromise: true,
+      returnByValue: true
+    }));
   },
 
-  function testExceptionInEvaluate(next)
+  async function testExceptionInEvaluate()
   {
-    Protocol.Runtime.evaluate({ expression: "throw 239", awaitPromise: true })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: "throw 239",
+      awaitPromise: true
+    }));
   },
 
-  function testLastEvaluatedResult(next)
+  async function testLastEvaluatedResult()
   {
-    Protocol.Runtime.evaluate({ expression: 'Promise.resolve(42)', awaitPromise: true, objectGroup: 'console' })
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => Protocol.Runtime.evaluate({ expression: '$_', includeCommandLineAPI: true }))
-      .then(result => InspectorTest.logMessage(result))
-      .then(() => next());
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: 'Promise.resolve(42)',
+      awaitPromise: true,
+      objectGroup: 'console'
+    }));
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: '$_',
+      includeCommandLineAPI: true
+    }));
   },
+
+  async function testRuntimeDisable()
+  {
+    await Protocol.Runtime.enable();
+    Protocol.Runtime.evaluate({
+      expression: 'new Promise(r1 => r = r1)',
+      awaitPromise: true }).then(InspectorTest.logMessage);
+    await Protocol.Runtime.disable();
+    InspectorTest.log('Resolving promise..');
+    await Protocol.Runtime.evaluate({expression: 'r({a:1})'});
+    InspectorTest.log('Promise resolved');
+  },
+
+  async function testImmediatelyResolvedAfterAfterContextDestroyed()
+  {
+    Protocol.Runtime.evaluate({
+      expression: 'a = new Promise(() => 42)',
+      awaitPromise: true }).then(InspectorTest.logMessage);
+    InspectorTest.log('Destroying context..');
+    await Protocol.Runtime.evaluate({expression: 'inspector.fireContextDestroyed()'});
+    InspectorTest.log('Context destroyed');
+  }
 ]);