[inspector] aligned Runtime.evaluate(awaitPromise: true) with await semantic

This one allows us to support custom promises implementation.
With awaitPromise flag Runtime.evaluate awaits
Promise.resolve(<expression result>).
This also allows to await for any non-Promise value, similar to await
expression, which is more convenient for most protocol users.

R=dgozman@chromium.org

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

src/inspector/injected-script.cc

@@ -60,11 +60,21 @@ using protocol::Maybe;
 class InjectedScript::ProtocolPromiseHandler {
  public:
   static bool add(V8InspectorSessionImpl* session,
-                  v8::Local<v8::Context> context,
+                  v8::Local<v8::Context> context, v8::Local<v8::Value> value,
-                  v8::Local<v8::Promise> promise,
+                  int executionContextId, const String16& objectGroup,
-                  const String16& notPromiseError, int executionContextId,
+                  bool returnByValue, bool generatePreview,
-                  const String16& objectGroup, bool returnByValue,
+                  EvaluateCallback* callback) {
-                  bool generatePreview, EvaluateCallback* callback) {
+    v8::Local<v8::Promise::Resolver> resolver;
+    if (!v8::Promise::Resolver::New(context).ToLocal(&resolver)) {
+      callback->sendFailure(Response::InternalError());
+      return false;
+    }
+    if (!resolver->Resolve(context, value).FromMaybe(false)) {
+      callback->sendFailure(Response::InternalError());
+      return false;
+    }
+
+    v8::Local<v8::Promise> promise = resolver->GetPromise();
     V8InspectorImpl* inspector = session->inspector();
     ProtocolPromiseHandler* handler =
         new ProtocolPromiseHandler(session, executionContextId, objectGroup,
@@ -456,23 +466,17 @@ std::unique_ptr<protocol::Runtime::RemoteObject> InjectedScript::wrapTable(
 
 void InjectedScript::addPromiseCallback(
     V8InspectorSessionImpl* session, v8::MaybeLocal<v8::Value> value,
-    const String16& notPromiseError, const String16& objectGroup,
+    const String16& objectGroup, bool returnByValue, bool generatePreview,
-    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(),
+  if (ProtocolPromiseHandler::add(
-                                  value.ToLocalChecked().As<v8::Promise>(),
+          session, m_context->context(), value.ToLocalChecked(),
-                                  notPromiseError, m_context->contextId(),
+          m_context->contextId(), objectGroup, returnByValue, generatePreview,
-                                  objectGroup, returnByValue, generatePreview,
           callback.get())) {
     m_evaluateCallbacks.insert(callback.release());
   }

src/inspector/injected-script.h

@@ -99,7 +99,6 @@ class InjectedScript final {
 
   void addPromiseCallback(V8InspectorSessionImpl* session,
                           v8::MaybeLocal<v8::Value> value,
-                          const String16& notPromiseError,
                           const String16& objectGroup, bool returnByValue,
                           bool generatePreview,
                           std::unique_ptr<EvaluateCallback> callback);

src/inspector/js_protocol.json

@@ -218,7 +218,7 @@
                     { "name": "returnByValue", "type": "boolean", "optional": true, "description": "Whether the result is expected to be a JSON object that 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 wait for promise to be resolved. If the result of evaluation is not a Promise, it's considered to be an error." }
+                    { "name": "awaitPromise", "type": "boolean", "optional":true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "RemoteObject", "description": "Evaluation result." },
@@ -249,7 +249,7 @@
                     { "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 wait for promise to be resolved. If the result of evaluation is not a Promise, it's considered to be an error." }
+                    { "name": "awaitPromise", "type": "boolean", "optional":true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "RemoteObject", "description": "Call result." },
@@ -336,7 +336,7 @@
                     { "name": "includeCommandLineAPI", "type": "boolean", "optional": true, "description": "Determines whether Command Line API should be available during the evaluation." },
                     { "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, "description": "Whether preview should be generated for the result." },
-                    { "name": "awaitPromise", "type": "boolean", "optional": true, "description": "Whether execution should wait for promise to be resolved. If the result of evaluation is not a Promise, it's considered to be an error." }
+                    { "name": "awaitPromise", "type": "boolean", "optional": true, "description": "Whether execution should <code>await</code> for resulting value and return once awaited promise is resolved." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "RemoteObject", "description": "Run result." },

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

@@ -190,9 +190,8 @@ void V8RuntimeAgentImpl::evaluate(
     return;
   }
   scope.injectedScript()->addPromiseCallback(
-      m_session, maybeResultValue, "Result of the evaluation is not a promise",
+      m_session, maybeResultValue, objectGroup.fromMaybe(""),
-      objectGroup.fromMaybe(""), returnByValue.fromMaybe(false),
+      returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
-      generatePreview.fromMaybe(false),
       EvaluateCallbackWrapper<EvaluateCallback>::wrap(std::move(callback)));
 }
 
@@ -206,10 +205,14 @@ void V8RuntimeAgentImpl::awaitPromise(
     callback->sendFailure(response);
     return;
   }
+  if (!scope.object()->IsPromise()) {
+    callback->sendFailure(
+        Response::Error("Could not find promise with given id"));
+    return;
+  }
   scope.injectedScript()->addPromiseCallback(
-      m_session, scope.object(), "Could not find promise with given id",
+      m_session, scope.object(), scope.objectGroupName(),
-      scope.objectGroupName(), returnByValue.fromMaybe(false),
+      returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
-      generatePreview.fromMaybe(false),
       EvaluateCallbackWrapper<AwaitPromiseCallback>::wrap(std::move(callback)));
 }
 
@@ -304,8 +307,7 @@ void V8RuntimeAgentImpl::callFunctionOn(
   }
 
   scope.injectedScript()->addPromiseCallback(
-      m_session, maybeResultValue,
+      m_session, maybeResultValue, scope.objectGroupName(),
-      "Result of the function call is not a promise", scope.objectGroupName(),
       returnByValue.fromMaybe(false), generatePreview.fromMaybe(false),
       EvaluateCallbackWrapper<CallFunctionOnCallback>::wrap(
           std::move(callback)));
@@ -514,7 +516,6 @@ void V8RuntimeAgentImpl::runScript(
   }
   scope.injectedScript()->addPromiseCallback(
       m_session, maybeResultValue.ToLocalChecked(),
-      "Result of the script execution is not a promise",
       objectGroup.fromMaybe(""), returnByValue.fromMaybe(false),
       generatePreview.fromMaybe(false),
       EvaluateCallbackWrapper<RunScriptCallback>::wrap(std::move(callback)));

test/inspector/runtime/call-function-on-async-expected.txt

@@ -69,8 +69,14 @@ Running test: testExceptionInFunctionExpression
 
 Running test: testFunctionReturnNotPromise
 {
-    code : -32000
+    id : <messageId>
-    message : Result of the function call is not a promise
+    result : {
+        result : {
+            description : 239
+            type : number
+            value : 239
+        }
+    }
 }
 
 Running test: testFunctionReturnResolvedPromiseReturnByValue

test/inspector/runtime/call-function-on-async.js

@@ -50,10 +50,10 @@ InspectorTest.runTestSuite([
       "({a : 1})",
       "(function() { return 239; })",
       [],
-      /* returnByValue */ false,
+      /* returnByValue */ true,
       /* generatePreview */ false,
       /* awaitPromise */ true)
-      .then((result) => InspectorTest.logMessage(result.error))
+      .then((result) => InspectorTest.logMessage(result))
       .then(() => next());
   },
 

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

@@ -129,20 +129,25 @@ Running test: testRejectedPromiseWithSyntaxError
 
 Running test: testPrimitiveValueInsteadOfPromise
 {
-    error : {
-        code : -32000
-        message : Result of the evaluation is not a promise
-    }
     id : <messageId>
+    result : {
+        result : {
+            type : boolean
+            value : true
+        }
+    }
 }
 
 Running test: testObjectInsteadOfPromise
 {
-    error : {
-        code : -32000
-        message : Result of the evaluation is not a promise
-    }
     id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : {
+            }
+        }
+    }
 }
 
 Running test: testPendingPromise
@@ -182,6 +187,18 @@ Running test: testExceptionInEvaluate
     }
 }
 
+Running test: testThenableJob
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 42
+            type : number
+            value : 42
+        }
+    }
+}
+
 Running test: testLastEvaluatedResult
 {
     id : <messageId>

test/inspector/runtime/evaluate-async.js

@@ -80,7 +80,8 @@ InspectorTest.runAsyncTestSuite([
   {
     InspectorTest.logMessage(await Protocol.Runtime.evaluate({
       expression: "({})",
-      awaitPromise: true
+      awaitPromise: true,
+      returnByValue: true
     }));
   },
 
@@ -101,6 +102,13 @@ InspectorTest.runAsyncTestSuite([
     }));
   },
 
+  async function testThenableJob()
+  {
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+      expression: '({then: resolve => resolve(42)})',
+      awaitPromise: true}));
+  },
+
   async function testLastEvaluatedResult()
   {
     InspectorTest.logMessage(await Protocol.Runtime.evaluate({

test/inspector/runtime/run-script-async-expected.txt

@@ -141,11 +141,15 @@ Running test: testRunScriptReturnByValue
 
 Running test: testAwaitNotPromise
 {
-    error : {
-        code : -32000
-        message : Result of the script execution is not a promise
-    }
     id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : {
+                a : 1
+            }
+        }
+    }
 }
 
 Running test: testAwaitResolvedPromise

test/inspector/runtime/run-script-async.js

@@ -82,7 +82,7 @@ InspectorTest.runTestSuite([
   {
     Protocol.Runtime.enable()
       .then(() => Protocol.Runtime.compileScript({ expression: "({a:1})", sourceURL: "boo.js", persistScript: true }))
-      .then((result) => Protocol.Runtime.runScript({ scriptId: result.result.scriptId, awaitPromise: true }))
+      .then((result) => Protocol.Runtime.runScript({ scriptId: result.result.scriptId, awaitPromise: true, returnByValue: true }))
       .then((result) => InspectorTest.logMessage(result))
       .then(() => Protocol.Runtime.disable())
       .then(() => next());