Changeset 267078 in webkit


Ignore:
Timestamp:
Sep 14, 2020 11:25:04 PM (4 years ago)
Author:
Carlos Garcia Campos
Message:

REGRESSION(r266885): WebDriver is broken since r266885
https://bugs.webkit.org/show_bug.cgi?id=216477

Reviewed by Devin Rousso.

There are several issues introduced in r266885, most of them related to JSValue::get methods now returning an
optional. In some cases of boolean values, the optional is checked instead of the contained bool or the logic is
inverted. JSONValue::getString() is problematic too because it doesn't use optional, when you call getString()
and null is returned it could be because the property is not present in the object or because the property is
not a string. In several cases we need to know that so we need to first get the value and then call asString(9
instead of calling getString() directly.

  • Session.cpp:

(WebDriver::Session::handleUserPrompts): Check the value of isShowingJavaScriptDialog, not the optional.
(WebDriver::parseAutomationCookie): Check also the value of the session optional bool, not only the optional.

  • SessionHost.cpp:

(WebDriver::SessionHost::dispatchMessage): Only set the responseObject if the object is not empty.

  • WebDriverService.cpp:

(WebDriver::WebDriverService::newWindow): Unknown values of type hint are not an error.
(WebDriver::WebDriverService::deleteCookie): Fix indentation.
(WebDriver::processKeyAction): Empty key value is an error, not the opposite.

  • gtk/WebDriverServiceGtk.cpp:

(WebDriver::WebDriverService::platformValidateCapability const): Only fail if optional values are wrong type,
not if they are not present.

  • wpe/WebDriverServiceWPE.cpp:

(WebDriver::WebDriverService::platformValidateCapability const): Ditto.

Location:
trunk/Source/WebDriver
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebDriver/ChangeLog

    r266889 r267078  
     12020-09-14  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        REGRESSION(r266885): WebDriver is broken since r266885
     4        https://bugs.webkit.org/show_bug.cgi?id=216477
     5
     6        Reviewed by Devin Rousso.
     7
     8        There are several issues introduced in r266885, most of them related to JSValue::get methods now returning an
     9        optional. In some cases of boolean values, the optional is checked instead of the contained bool or the logic is
     10        inverted. JSONValue::getString() is problematic too because it doesn't use optional, when you call getString()
     11        and null is returned it could be because the property is not present in the object or because the property is
     12        not a string. In several cases we need to know that so we need to first get the value and then call asString(9
     13        instead of calling getString() directly.
     14
     15        * Session.cpp:
     16        (WebDriver::Session::handleUserPrompts): Check the value of isShowingJavaScriptDialog, not the optional.
     17        (WebDriver::parseAutomationCookie): Check also the value of the session optional bool, not only the optional.
     18        * SessionHost.cpp:
     19        (WebDriver::SessionHost::dispatchMessage): Only set the responseObject if the object is not empty.
     20        * WebDriverService.cpp:
     21        (WebDriver::WebDriverService::newWindow): Unknown values of type hint are not an error.
     22        (WebDriver::WebDriverService::deleteCookie): Fix indentation.
     23        (WebDriver::processKeyAction): Empty key value is an error, not the opposite.
     24        * gtk/WebDriverServiceGtk.cpp:
     25        (WebDriver::WebDriverService::platformValidateCapability const): Only fail if optional values are wrong type,
     26        not if they are not present.
     27        * wpe/WebDriverServiceWPE.cpp:
     28        (WebDriver::WebDriverService::platformValidateCapability const): Ditto.
     29
    1302020-09-10  Fujii Hironori  <Hironori.Fujii@sony.com>
    231
  • trunk/Source/WebDriver/Session.cpp

    r266889 r267078  
    215215        }
    216216
    217         if (!isShowingJavaScriptDialog) {
     217        if (!isShowingJavaScriptDialog.value()) {
    218218            completionHandler(CommandResult::success());
    219219            return;
     
    23692369
    23702370    auto session = cookieObject.getBoolean("session"_s);
    2371     if (!session) {
     2371    if (!session || !*session) {
    23722372        if (auto expiry = cookieObject.getDouble("expires"_s))
    23732373            cookie.expiry = *expiry;
  • trunk/Source/WebDriver/SessionHost.cpp

    r266885 r267078  
    8282        response.responseObject = WTFMove(errorObject);
    8383        response.isError = true;
    84     } else if (auto resultObject = messageObject->getObject("result"_s))
    85         response.responseObject = WTFMove(resultObject);
     84    } else if (auto resultObject = messageObject->getObject("result"_s)) {
     85        if (resultObject->size())
     86            response.responseObject = WTFMove(resultObject);
     87    }
    8688
    8789    responseHandler(WTFMove(response));
  • trunk/Source/WebDriver/WebDriverService.cpp

    r266885 r267078  
    11821182    if (auto value = parameters->getValue("type"_s)) {
    11831183        auto valueString = value->asString();
    1184         if (valueString == "window" || valueString == "tab")
    1185             typeHint = valueString;
    1186         else if (!value->isNull()) {
     1184        if (!!valueString) {
     1185            if (valueString == "window" || valueString == "tab")
     1186                typeHint = valueString;
     1187        } else if (!value->isNull()) {
    11871188            completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument));
    11881189            return;
     
    17621763        return;
    17631764
    1764 auto name = parameters->getString("name"_s);
     1765    auto name = parameters->getString("name"_s);
    17651766    if (!name) {
    17661767        completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument));
     
    18541855        }
    18551856        auto key = keyValue->asString();
    1856         if (!key.isEmpty()) {
     1857        if (key.isEmpty()) {
    18571858            errorMessage = String("The paramater 'value' is invalid for key up/down action");
    18581859            return WTF::nullopt;
  • trunk/Source/WebDriver/gtk/WebDriverServiceGtk.cpp

    r266885 r267078  
    5353        return true;
    5454
    55     auto binary = browserOptions->getString("binary"_s);
    56     if (!binary)
    57         return false;
     55    if (auto binaryValue = browserOptions->getValue("binary"_s)) {
     56        if (!binaryValue->asString())
     57            return false;
     58    }
    5859
    59     auto useOverlayScrollbars = browserOptions->getBoolean("useOverlayScrollbars"_s);
    60     if (!useOverlayScrollbars)
    61         return false;
     60    if (auto useOverlayScrollbarsValue = browserOptions->getValue("useOverlayScrollbars"_s)) {
     61        if (!useOverlayScrollbarsValue->asBoolean())
     62            return false;
     63    }
    6264
    63     if (auto browserArguments = browserOptions->getArray("args"_s)) {
     65    if (auto browserArgumentsValue = browserOptions->getValue("args"_s)) {
     66        auto browserArguments = browserArgumentsValue->asArray();
     67        if (!browserArguments)
     68            return false;
     69
    6470        unsigned browserArgumentsLength = browserArguments->length();
    6571        for (unsigned i = 0; i < browserArgumentsLength; ++i) {
     
    7076    }
    7177
    72     if (auto certificates = browserOptions->getArray("certificates"_s)) {
     78    if (auto certificatesValue = browserOptions->getValue("certificates"_s)) {
     79        auto certificates = certificatesValue->asArray();
     80        if (!certificates)
     81            return false;
     82
    7383        unsigned certificatesLength = certificates->length();
    7484        for (unsigned i = 0; i < certificatesLength; ++i) {
  • trunk/Source/WebDriver/wpe/WebDriverServiceWPE.cpp

    r266885 r267078  
    5858        return false;
    5959
    60     auto browserArguments = browserOptions->getArray("args"_s);
    61     if (!browserArguments)
    62         return false;
     60    if (auto browserArgumentsValue = browserOptions->getValue("args"_s)) {
     61        auto browserArguments = browserArgumentsValue->asArray();
     62        if (!browserArguments)
     63            return false;
    6364
    64     unsigned browserArgumentsLength = browserArguments->length();
    65     for (unsigned i = 0; i < browserArgumentsLength; ++i) {
    66         auto argument = browserArguments->get(i)->asString();
    67         if (!argument)
    68             return false;
     65        unsigned browserArgumentsLength = browserArguments->length();
     66        for (unsigned i = 0; i < browserArgumentsLength; ++i) {
     67            auto argument = browserArguments->get(i)->asString();
     68            if (!argument)
     69                return false;
     70        }
    6971    }
    7072
    71     if (auto certificates = browserOptions->getArray("certificates"_s)) {
     73    if (auto certificatesValue = browserOptions->getValue("certificates"_s)) {
     74        auto certificates = certificatesValue->asArray();
     75        if (!certificates)
     76            return false;
     77
    7278        unsigned certificatesLength = certificates->length();
    7379        for (unsigned i = 0; i < certificatesLength; ++i) {
Note: See TracChangeset for help on using the changeset viewer.