Diff
Modified: trunk/Source/WebDriver/ChangeLog (254328 => 254329)
--- trunk/Source/WebDriver/ChangeLog 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebDriver/ChangeLog 2020-01-10 08:44:37 UTC (rev 254329)
@@ -1,3 +1,16 @@
+2020-01-10 Carlos Garcia Campos <[email protected]>
+
+ Automation: evaluateJavaScriptFunction should use Promises
+ https://bugs.webkit.org/show_bug.cgi?id=204151
+
+ Reviewed by Brian Burg.
+
+ * CommandResult.cpp:
+ (WebDriver::CommandResult::httpStatusCode const): Timeout errors should return 500 not 408.
+ * Session.cpp:
+ (WebDriver::Session::executeScript): Ensure the script body goes between new lines to avoid problems with
+ trailing comments like in function() { return foo; // Comment }.
+
2020-01-07 Carlos Garcia Campos <[email protected]>
WebDriver: handle no such element errors
Modified: trunk/Source/WebDriver/CommandResult.cpp (254328 => 254329)
--- trunk/Source/WebDriver/CommandResult.cpp 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebDriver/CommandResult.cpp 2020-01-10 08:44:37 UTC (rev 254329)
@@ -148,12 +148,11 @@
case ErrorCode::InvalidSessionID:
case ErrorCode::UnknownCommand:
return 404;
- case ErrorCode::ScriptTimeout:
- case ErrorCode::Timeout:
- return 408;
case ErrorCode::_javascript_Error:
case ErrorCode::MoveTargetOutOfBounds:
+ case ErrorCode::ScriptTimeout:
case ErrorCode::SessionNotCreated:
+ case ErrorCode::Timeout:
case ErrorCode::UnableToCaptureScreen:
case ErrorCode::UnexpectedAlertOpen:
case ErrorCode::UnknownError:
Modified: trunk/Source/WebDriver/Session.cpp (254328 => 254329)
--- trunk/Source/WebDriver/Session.cpp 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebDriver/Session.cpp 2020-01-10 08:44:37 UTC (rev 254329)
@@ -2133,13 +2133,12 @@
parameters->setString("browsingContextHandle"_s, m_toplevelBrowsingContext.value());
if (m_currentBrowsingContext)
parameters->setString("frameHandle"_s, m_currentBrowsingContext.value());
- parameters->setString("function"_s, "function(){" + script + '}');
+ parameters->setString("function"_s, "function(){\n" + script + "\n}");
parameters->setArray("arguments"_s, WTFMove(arguments));
- if (mode == ExecuteScriptMode::Async) {
+ if (mode == ExecuteScriptMode::Async)
parameters->setBoolean("expectsImplicitCallbackArgument"_s, true);
- if (m_scriptTimeout != std::numeric_limits<double>::infinity())
- parameters->setDouble("callbackTimeout"_s, m_scriptTimeout);
- }
+ if (m_scriptTimeout != std::numeric_limits<double>::infinity())
+ parameters->setDouble("callbackTimeout"_s, m_scriptTimeout);
m_host->sendCommandToBackend("evaluateJavaScriptFunction"_s, WTFMove(parameters), [this, protectedThis = protectedThis.copyRef(), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) mutable {
if (response.isError || !response.responseObject) {
auto result = CommandResult::fail(WTFMove(response.responseObject));
Modified: trunk/Source/WebKit/ChangeLog (254328 => 254329)
--- trunk/Source/WebKit/ChangeLog 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebKit/ChangeLog 2020-01-10 08:44:37 UTC (rev 254329)
@@ -1,5 +1,28 @@
2020-01-10 Carlos Garcia Campos <[email protected]>
+ Automation: evaluateJavaScriptFunction should use Promises
+ https://bugs.webkit.org/show_bug.cgi?id=204151
+
+ Reviewed by Brian Burg.
+
+ Make the function to run scripts async and handle the result as a promise. To implement the script timeout we
+ use another promise that starts the timer and then we run a Promise.race() with both promises. To simplify the
+ results reporting, all exceptions (including timeout errors that are now handled as exceptions) are now handled
+ as errors passed to the resultCallback. The boolean parameter has been removed, we can simply check the type of
+ the value received because results are always strings and errors are always exception objects.
+
+ * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+ (WebKit::evaluateJavaScriptCallback): Handle the script result, including all possible errors now (not only timeouts).
+ (WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction): Any exception running the script should be an
+ internal error now. The code to handle error has been moved to evaluateJavaScriptCallback().
+ * WebProcess/Automation/WebAutomationSessionProxy.js:
+ (WebKitAutomation.AutomationSessionProxy.prototype.evaluateJavaScriptFunction): Call _execute and handle the
+ promise result to call resultCallback wityh either the result or the error.
+ (WebKitAutomation.AutomationSessionProxy.prototype._execute): Make the function to run the script async and
+ handle the result as a promise.
+
+2020-01-10 Carlos Garcia Campos <[email protected]>
+
Automation: scripts are executed in the wrong js context after a history navigation
https://bugs.webkit.org/show_bug.cgi?id=204880
<rdar://problem/58413615>
Modified: trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp (254328 => 254329)
--- trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp 2020-01-10 08:44:37 UTC (rev 254329)
@@ -181,11 +181,10 @@
static JSValueRef evaluateJavaScriptCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
- ASSERT_ARG(argumentCount, argumentCount == 4);
+ ASSERT_ARG(argumentCount, argumentCount == 3);
ASSERT_ARG(arguments, JSValueIsNumber(context, arguments[0]));
ASSERT_ARG(arguments, JSValueIsNumber(context, arguments[1]));
- ASSERT_ARG(arguments, JSValueIsString(context, arguments[2]));
- ASSERT_ARG(arguments, JSValueIsBoolean(context, arguments[3]));
+ ASSERT_ARG(arguments, JSValueIsObject(context, arguments[2]) || JSValueIsString(context, arguments[2]));
auto automationSessionProxy = WebProcess::singleton().automationSessionProxy();
if (!automationSessionProxy)
@@ -193,22 +192,37 @@
WebCore::FrameIdentifier frameID = WebCore::frameIdentifierFromID(JSValueToNumber(context, arguments[0], exception));
uint64_t callbackID = JSValueToNumber(context, arguments[1], exception);
- auto result = adoptRef(JSValueToStringCopy(context, arguments[2], exception));
+ if (JSValueIsString(context, arguments[2])) {
+ auto result = adoptRef(JSValueToStringCopy(context, arguments[2], exception));
+ automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, result->string(), { });
+ } else if (JSValueIsObject(context, arguments[2])) {
+ JSObjectRef error = JSValueToObject(context, arguments[2], exception);
+ JSValueRef nameValue = JSObjectGetProperty(context, error, OpaqueJSString::tryCreate("name"_s).get(), exception);
+ String exceptionName = adoptRef(JSValueToStringCopy(context, nameValue, nullptr))->string();
+ String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::_javascript_Error);
+ if (exceptionName == "_javascript_Timeout")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::_javascript_Timeout);
+ else if (exceptionName == "NodeNotFound")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NodeNotFound);
+ else if (exceptionName == "InvalidNodeIdentifier")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidNodeIdentifier);
+ else if (exceptionName == "InvalidElementState")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidElementState);
+ else if (exceptionName == "InvalidParameter")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidParameter);
+ else if (exceptionName == "InvalidSelector")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidSelector);
+ else if (exceptionName == "ElementNotInteractable")
+ errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ElementNotInteractable);
- bool resultIsErrorName = JSValueToBoolean(context, arguments[3]);
+ JSValueRef messageValue = JSObjectGetProperty(context, error, OpaqueJSString::tryCreate("message"_s).get(), exception);
+ auto exceptionMessage = adoptRef(JSValueToStringCopy(context, messageValue, exception))->string();
+ automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, exceptionMessage, errorType);
+ } else {
+ String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InternalError);
+ automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, { }, errorType);
+ }
- if (resultIsErrorName) {
- if (result->string() == "_javascript_Timeout") {
- String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::_javascript_Timeout);
- automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, String(), errorType);
- } else {
- ASSERT_NOT_REACHED();
- String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InternalError);
- automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, String(), errorType);
- }
- } else
- automationSessionProxy->didEvaluateJavaScriptFunction(frameID, callbackID, result->string(), String());
-
return JSValueMakeUndefined(context);
}
@@ -338,25 +352,10 @@
if (!exception)
return;
- String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::_javascript_Error);
+ String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InternalError);
String exceptionMessage;
if (JSValueIsObject(context, exception)) {
- JSValueRef nameValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), OpaqueJSString::tryCreate("name"_s).get(), nullptr);
- auto exceptionName = adoptRef(JSValueToStringCopy(context, nameValue, nullptr))->string();
- if (exceptionName == "NodeNotFound")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NodeNotFound);
- else if (exceptionName == "InvalidNodeIdentifier")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidNodeIdentifier);
- else if (exceptionName == "InvalidElementState")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidElementState);
- else if (exceptionName == "InvalidParameter")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidParameter);
- else if (exceptionName == "InvalidSelector")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidSelector);
- else if (exceptionName == "ElementNotInteractable")
- errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ElementNotInteractable);
-
JSValueRef messageValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), OpaqueJSString::tryCreate("message"_s).get(), nullptr);
exceptionMessage = adoptRef(JSValueToStringCopy(context, messageValue, nullptr))->string();
} else
Modified: trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.js (254328 => 254329)
--- trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.js 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.js 2020-01-10 08:44:37 UTC (rev 254329)
@@ -41,33 +41,9 @@
evaluateJavaScriptFunction(functionString, argumentStrings, expectsImplicitCallbackArgument, frameID, callbackID, resultCallback, callbackTimeout)
{
- // The script is expected to be a function declaration. Evaluate it inside parenthesis to get the function value.
- let functionValue = evaluate("(" + functionString + ")");
- if (typeof functionValue !== "function")
- throw new TypeError("Script did not evaluate to a function.");
-
- this._clearStaleNodes();
-
- let argumentValues = argumentStrings.map(this._jsonParse, this);
-
- let timeoutIdentifier = 0;
- let resultReported = false;
-
- let reportResult = (result) => {
- if (timeoutIdentifier)
- clearTimeout(timeoutIdentifier);
- resultCallback(frameID, callbackID, this._jsonStringify(result), false);
- resultReported = true;
- };
- let reportTimeoutError = () => { resultCallback(frameID, callbackID, "_javascript_Timeout", true); };
-
- if (expectsImplicitCallbackArgument) {
- argumentValues.push(reportResult);
- functionValue.apply(null, argumentValues);
- if (!resultReported && callbackTimeout >= 0)
- timeoutIdentifier = setTimeout(reportTimeoutError, callbackTimeout);
- } else
- reportResult(functionValue.apply(null, argumentValues));
+ this._execute(functionString, argumentStrings, expectsImplicitCallbackArgument, callbackTimeout)
+ .then(result => { resultCallback(frameID, callbackID, this._jsonStringify(result)); })
+ .catch(error => { resultCallback(frameID, callbackID, error); });
}
nodeForIdentifier(identifier)
@@ -82,6 +58,59 @@
// Private
+ _execute(functionString, argumentStrings, expectsImplicitCallbackArgument, callbackTimeout)
+ {
+ let timeoutPromise;
+ let timeoutIdentifier = 0;
+ if (callbackTimeout >= 0) {
+ timeoutPromise = new Promise((resolve, reject) => {
+ timeoutIdentifier = setTimeout(() => {
+ reject({ name: "_javascript_Timeout", message: "script timed out after " + callbackTimeout + "ms" });
+ }, callbackTimeout);
+ });
+ }
+
+ let promise = new Promise((resolve, reject) => {
+ // The script is expected to be a function declaration. Evaluate it inside parenthesis to get the function value.
+ let functionValue = evaluate("(async " + functionString + ")");
+ if (typeof functionValue !== "function")
+ reject(new TypeError("Script did not evaluate to a function."));
+
+ this._clearStaleNodes();
+
+ let argumentValues = argumentStrings.map(this._jsonParse, this);
+ if (expectsImplicitCallbackArgument)
+ argumentValues.push(resolve);
+ let resultPromise = functionValue.apply(null, argumentValues);
+
+ let promises = [resultPromise];
+ if (timeoutPromise)
+ promises.push(timeoutPromise);
+ Promise.race(promises)
+ .then(result => {
+ if (!expectsImplicitCallbackArgument) {
+ resolve(result);
+ }
+ })
+ .catch(error => {
+ reject(error);
+ });
+ });
+
+ // Async scripts can call Promise.resolve() in the function script, generating a new promise that is resolved in a
+ // timer (see w3c test execute_async_script/promise.py::test_promise_resolve_timeout). In that case, the internal race
+ // finishes resolved, so we need to start a new one here to wait for the second promise to be resolved or the timeout.
+ let promises = [promise];
+ if (timeoutPromise)
+ promises.push(timeoutPromise);
+ return Promise.race(promises)
+ .finally(() => {
+ if (timeoutIdentifier) {
+ clearTimeout(timeoutIdentifier);
+ }
+ });
+ }
+
_jsonParse(string)
{
if (!string)
Modified: trunk/WebDriverTests/ChangeLog (254328 => 254329)
--- trunk/WebDriverTests/ChangeLog 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/WebDriverTests/ChangeLog 2020-01-10 08:44:37 UTC (rev 254329)
@@ -1,3 +1,14 @@
+2020-01-10 Carlos Garcia Campos <[email protected]>
+
+ Automation: evaluateJavaScriptFunction should use Promises
+ https://bugs.webkit.org/show_bug.cgi?id=204151
+
+ Reviewed by Brian Burg.
+
+ Remove expectations for tests that are now passing.
+
+ * TestExpectations.json:
+
2020-01-09 Carlos Garcia Campos <[email protected]>
Unreviewed gardening. Mark imported/w3c/webdriver/tests/switch_to_frame/cross_origin.py::test_nested_cross_origin_iframe as failure
Modified: trunk/WebDriverTests/TestExpectations.json (254328 => 254329)
--- trunk/WebDriverTests/TestExpectations.json 2020-01-10 08:42:11 UTC (rev 254328)
+++ trunk/WebDriverTests/TestExpectations.json 2020-01-10 08:44:37 UTC (rev 254329)
@@ -535,22 +535,6 @@
}
}
},
- "imported/w3c/webdriver/tests/execute_async_script/promise.py": {
- "expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/204151"}},
- "subtests": {
- "test_await_promise_reject": {
- "expected": {"all": {"status": ["PASS"]}}
- }
- }
- },
- "imported/w3c/webdriver/tests/execute_script/promise.py": {
- "expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/204151"}},
- "subtests": {
- "test_await_promise_reject": {
- "expected": {"all": {"status": ["PASS"]}}
- }
- }
- },
"imported/w3c/webdriver/tests/element_send_keys/content_editable.py": {
"expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/180403"}}
},
