[ChromeDriver] Implements ExecuteAsyncScript promise-handling.

Handles promises according to W3C-spec
(https://w3c.github.io/webdriver/#execute-async-script) for asynchronous
user scripts. This removes the global "result" object that contains
results of the latest script to execute and allows for scripts to wait
until results are ready before returning (instead of polling for
results after the script completes).

run_py_tests.py.

Tested: WPTs webdriver/test/execute_async_script, and python
Bug: chromedriver:2398
Change-Id: I9f6cdeb5b3e93fcfd3e63090ce8b3aed275d71be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1636627
Commit-Queue: Rohan Pavone <rohpavone@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665589}

chrome/test/chromedriver/chrome/web_view_impl.cc

@@ -996,58 +996,14 @@ Status WebViewImpl::CallAsyncFunctionInternal(
   async_args.AppendString("return (" + function + ").apply(null, arguments);");
   async_args.Append(args.CreateDeepCopy());
   async_args.AppendBoolean(is_user_supplied);
-  async_args.AppendInteger(timeout.InMilliseconds());
   std::unique_ptr<base::Value> tmp;
   Status status = CallFunctionWithTimeout(frame, kExecuteAsyncScriptScript,
                                           async_args, timeout, &tmp);
   if (status.IsError())
     return status;
 
-  const char kDocUnloadError[] = "document unloaded while waiting for result";
-  std::string kQueryResult = base::StringPrintf(
-      "function() {"
-      "  var info = document.$chrome_asyncScriptInfo;"
-      "  if (!info)"
-      "    return {status: %d, value: '%s'};"
-      "  var result = info.result;"
-      "  if (!result)"
-      "    return {status: 0};"
-      "  delete info.result;"
-      "  return result;"
-      "}",
-      kJavaScriptError,
-      kDocUnloadError);
-
-  while (true) {
-    base::ListValue no_args;
-    std::unique_ptr<base::Value> query_value;
-    Status status = CallFunction(frame, kQueryResult, no_args, &query_value);
-    if (status.IsError()) {
-      if (status.code() == kNoSuchFrame)
-        return Status(kJavaScriptError, kDocUnloadError);
-      return status;
-    }
-
-    base::DictionaryValue* result_info = NULL;
-    if (!query_value->GetAsDictionary(&result_info))
-      return Status(kUnknownError, "async result info is not a dictionary");
-    int status_code;
-    if (!result_info->GetInteger("status", &status_code))
-      return Status(kUnknownError, "async result info has no int 'status'");
-    if (status_code != kOk) {
-      std::string message;
-      result_info->GetString("value", &message);
-      return Status(static_cast<StatusCode>(status_code), message);
-    }
-
-    base::Value* value = NULL;
-    if (result_info->Get("value", &value)) {
-      result->reset(value->DeepCopy());
-      return Status(kOk);
-    }
-
-    base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));
-  }
+  *result = std::move(tmp);
+  return status;
 }
 
 Status WebViewImpl::IsNotPendingNavigation(const std::string& frame_id,

chrome/test/chromedriver/js/execute_async_script.js

@@ -13,28 +13,9 @@ var StatusCode = {
   SCRIPT_TIMEOUT: 28,
 };
 
-/**
- * Dictionary key for asynchronous script info.
- * @const
- */
-var ASYNC_INFO_KEY = '$chrome_asyncScriptInfo';
-
-/**
-* Return the information of asynchronous script execution.
-*
-* @return {Object<?>} Information of asynchronous script execution.
-*/
-function getAsyncScriptInfo() {
-  if (!(ASYNC_INFO_KEY in document))
-    document[ASYNC_INFO_KEY] = {'id': 0};
-  return document[ASYNC_INFO_KEY];
-}
 
 /**
-* Execute the given script and save its asynchronous result.
-*
-* If script1 finishes after script2 is executed, then script1's result will be
-* discarded while script2's will be saved.
+* Execute the given script and return a promise containing its result.
 *
 * @param {string} script The asynchronous script to be executed. The script
 *     should be a proper function body. It will be wrapped in a function and
@@ -45,57 +26,49 @@ function getAsyncScriptInfo() {
 *     If not, UnknownError will be used instead of JavaScriptError if an
 *     exception occurs during the script, and an additional error callback will
 *     be supplied to the script.
-* @param {?number} opt_timeoutMillis The timeout, in milliseconds, to use.
-*     If the timeout is exceeded and the callback has not been invoked, a error
-*     result will be saved and future invocation of the callback will be
-*     ignored.
 */
-function executeAsyncScript(script, args, isUserSupplied, opt_timeoutMillis) {
-  var info = getAsyncScriptInfo();
-  info.id++;
-  delete info.result;
-  var id = info.id;
-
-  function report(status, value) {
-    if (id != info.id)
-      return;
-    info.id++;
-    // Undefined value is skipped when the object is converted to JSON.
-    // Replace it with null so we don't lose the value.
-    if (value === undefined)
-      value = null;
-    info.result = {status: status, value: value};
-  }
-  function reportValue(value) {
-    report(StatusCode.OK, value);
-  }
-  function reportScriptError(error) {
-    var code = isUserSupplied ? StatusCode.JAVASCRIPT_ERROR :
-                                (error.code || StatusCode.UNKNOWN_ERROR);
-    var message = error.message;
-    if (error.stack) {
-      message += "\nJavaScript stack:\n" + error.stack;
-    }
-    report(code, message);
+function executeAsyncScript(script, args, isUserSupplied) {
+  function isThenable(value) {
+    return typeof value === 'object' && typeof value.then === 'function';
   }
-  args.push(reportValue);
+  let resolveHandle;
+  let rejectHandle;
+  var promise = new Promise((resolve, reject) => {
+    resolveHandle = resolve;
+    rejectHandle = reject;
+  });
+
+  args.push(resolveHandle);  // Append resolve to end of arguments list.
   if (!isUserSupplied)
-    args.push(reportScriptError);
+    args.push(rejectHandle);
 
+  // This confusing, round-about way accomplishing this script execution is to
+  // follow the W3C execute-async-script spec.
   try {
-    new Function(script).apply(null, args);
-  } catch (error) {
-    reportScriptError(error);
-    return;
-  }
+    // The assumption is that each script is an asynchronous script.
+    const scriptResult = new Function(script).apply(null, args);
 
-  if (typeof(opt_timeoutMillis) != 'undefined') {
-    window.setTimeout(function() {
-      var code = isUserSupplied ? StatusCode.SCRIPT_TIMEOUT :
-                                  StatusCode.UNKNOWN_ERROR;
-      var errorMsg = 'result was not received in ' + opt_timeoutMillis / 1000 +
-                     ' seconds';
-      report(code, errorMsg);
-    }, opt_timeoutMillis);
+    // First case is for user-scripts - they are all wrapped in an async
+    // function in order to allow for "await" commands. As a result, all async
+    // scripts from users will return a Promise that is thenable by default,
+    // even if it doesn't return anything.
+    if (isThenable(scriptResult)) {
+      const resolvedPromise = Promise.resolve(scriptResult);
+      resolvedPromise.then((value) => {
+        // Must be thenable if user-supplied.
+        if (!isUserSupplied || isThenable(value))
+          resolveHandle(value);
+      })
+      .catch(rejectHandle);
+    }
+  } catch(error) {
+    rejectHandle(error);
   }
+
+  return promise.catch((error) => {
+    const code = isUserSupplied ? StatusCode.JAVASCRIPT_ERROR :
+                            (error.code || StatusCode.UNKNOWN_ERROR);
+    error.code = code;
+    throw error;
+  });
 }

chrome/test/chromedriver/js/execute_async_script_test.html

@@ -4,23 +4,28 @@
 <script src='execute_async_script.js'></script>
 <script>
 
-function resetAsyncScriptInfo() {
-  delete document[ASYNC_INFO_KEY];
+async function testScriptThrows() {
+  let promise = executeAsyncScript('f(123);', [], true).then((result) => {
+    assert(false);
+  }).catch((error) => {
+    assertEquals(StatusCode.JAVASCRIPT_ERROR, error.code);
+    return 1;
+  });
+
+  let result = await promise;
+  assertEquals(1, result);
+
+  executeAsyncScript('f(123);', [], false).then((result) => {
+    assert(false);
+  }).catch((error) => {
+    assertEquals(StatusCode.UNKNOWN_ERROR, error.code);
+    return 1;
+  });
+  result = await promise;
+  assertEquals(1, result);
 }
 
-function testScriptThrows() {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript('f(123);', [], true);
-  assertEquals(StatusCode.JAVASCRIPT_ERROR, info.result.status);
-  executeAsyncScript('f(123);', [], false);
-  assertEquals(StatusCode.UNKNOWN_ERROR, info.result.status);
-}
-
-function testUserScriptWithArgs() {
-  resetAsyncScriptInfo();
-
+async function testUserScriptWithArgs() {
   var injectedArgs = null;
   function captureArguments(args) {
     injectedArgs = args;
@@ -30,106 +35,49 @@ function testUserScriptWithArgs() {
   var script =
       'var args = arguments; args[0](args); args[args.length - 1](args[1]);';
   var script_args = [captureArguments, 1];
-  executeAsyncScript(script, script_args, true);
-
+  await executeAsyncScript(script, script_args, true).then((result) => {
+    assertEquals(1, result);
+  });
   assertEquals(3, injectedArgs.length);
   assertEquals(captureArguments, injectedArgs[0]);
   assertEquals(1, injectedArgs[1]);
-
-  var info = getAsyncScriptInfo();
-  assertEquals(0, info.result.status);
-  assertEquals(1, info.result.value);
-  assertEquals(2, info.id);
-}
-
-function testNonUserScript() {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript('arguments[1](arguments[0])', [33], false);
-  assertEquals(0, info.result.status);
-  assertEquals(33, info.result.value);
-
-  executeAsyncScript('arguments[2](new Error("ERR"))', [33], false);
-  assertEquals(StatusCode.UNKNOWN_ERROR, info.result.status);
-  assertEquals(0, info.result.value.indexOf('ERR'));
-
-  executeAsyncScript('var e = new Error("ERR"); e.code = 111; arguments[1](e)',
-                     [], false);
-  assertEquals(111, info.result.status);
-  assertEquals(0, info.result.value.indexOf('ERR'));
-}
-
-function testNoResultBeforeTimeout() {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript(
-      'var a = arguments; window.setTimeout(function() {a[0](33)}, 0);',
-      [], true, 0);
-
-  assert(!info.result);
 }
 
-function testZeroTimeout(runner) {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript(
-      'var a = arguments; window.setTimeout(function() {a[0](33)}, 0);',
-      [], true, 0);
-
-  window.setTimeout(function() {
-    assertEquals(0, info.result.status);
-    assertEquals(33, info.result.value);
-    runner.continueTesting();
-  }, 0);
-  runner.waitForAsync();
-}
-
-function testUserScriptTimesOut(runner) {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript('', [], true, 500);
-
-  window.setTimeout(function() {
-    assertEquals(StatusCode.SCRIPT_TIMEOUT, info.result.status);
-    assert(info.result.value.indexOf('0.5') != -1);
-    runner.continueTesting();
-  }, 500);
-
-  runner.waitForAsync();
+async function testNonUserScript() {
+  await executeAsyncScript('arguments[1](arguments[0])', [33],
+                           false).then((result) => {
+    assertEquals(33, result);
+  });
+
+  await executeAsyncScript('arguments[2](new Error("ERR"))', [33],
+                           false).then((result) => {
+    assert(false);
+  }).catch((error) => {
+    assertEquals(StatusCode.UNKNOWN_ERROR, error.code);
+    assertEquals(0, error.message.indexOf('ERR'));
+  });
+
+  await executeAsyncScript(`
+    var e = new Error("ERR");
+    e.code = 111;
+    arguments[1](e)`,
+    [], false).then((result) => { assert(false); }).catch((error) => {
+      assertEquals(111, error.code);
+      assertEquals(0,   error.message.indexOf('ERR'));
+  });
 }
-
-function testNonUserScriptTimesOut(runner) {
-  resetAsyncScriptInfo();
-  var info = getAsyncScriptInfo();
-
-  executeAsyncScript('', [], false, 500);
-
-  window.setTimeout(function() {
-    assertEquals(StatusCode.UNKNOWN_ERROR, info.result.status);
-    assert(info.result.value.indexOf('0.5') != -1);
-    runner.continueTesting();
-  }, 500);
-
-  runner.waitForAsync();
-}
-
-function testFirstScriptFinishAfterSecondScriptExecute() {
-  resetAsyncScriptInfo();
-
+async function testFirstScriptFinishAfterSecondScriptExecute() {
+  let firstCompleted = false;
   executeAsyncScript(
-      'var f = arguments[0]; setTimeout(function(){ f(1); }, 100000);', []);
-  var info = getAsyncScriptInfo();
-  assert(!info.hasOwnProperty('result'));
-  assertEquals(1, info.id);
-
-  executeAsyncScript('var fn = arguments[0]; fn(2);', []);
-  assertEquals(0, info.result.status);
-  assertEquals(2, info.result.value);
-  assertEquals(3, info.id);
+    `var f = arguments[0];
+     setTimeout(function(){ f(1); }, 100000);`, []).then((result) => {
+        firstCompleted = true;
+  });
+  await executeAsyncScript('var fn = arguments[0]; fn(2);',
+                           []).then((result) => {
+    assert(!firstCompleted);
+    assertEquals(2, result);
+  });
 }
 
 </script>

chrome/test/chromedriver/window_commands.cc

@@ -601,7 +601,7 @@ Status ExecuteExecuteAsyncScript(Session* session,
     script = script + "\n";
 
   Status status = web_view->CallUserAsyncFunction(
-      session->GetCurrentFrameId(), "function(){" + script + "}", *args,
+      session->GetCurrentFrameId(), "async function(){" + script + "}", *args,
       session->script_timeout, value);
   if (status.code() == kTimeout)
     return Status(kScriptTimeout);