WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merg...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jan 2018 13:52:40 +0000 (13:52 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jan 2018 13:52:40 +0000 (13:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181985

Reviewed by Carlos Alberto Lopez Perez.

Source/WebDriver:

The problem is that we are considering a failure when the browser name doesn't match the capabilities, instead
of trying with the next merged capabilities. This is happening because when processing capabilities, we only
match the ones that we know without having to launch the browser. Browser name and version are provided by the
browser during the session initialization. This patch reworks the session creation to make it possible to try
with the next merged capabilities when matching fails after the browser is launched.

* Session.cpp:
(WebDriver::Session::Session): Initialize timeouts from capabilities, because now we have the final capabilities here.
(WebDriver::Session::id const): Return the session ID from the SessionHost, since it's now created there.
(WebDriver::Session::createTopLevelBrowsingContext): Do not start the session, it has already been started a
this point.
(WebDriver::Session::createElement): Use id() instead of m_id.
* Session.h:
* SessionHost.h:
(WebDriver::SessionHost::sessionID const): Return the session ID.
* WebDriverService.cpp:
(WebDriver::WebDriverService::matchCapabilities const): Remove the error handling, and return a boolean instead,
since not mathing is not an error.
(WebDriver::WebDriverService::processCapabilities const): Return a list of matched capabilities, instead of the
JSON object corresponding to the first match.
(WebDriver::WebDriverService::newSession): Use helper connectToBrowser().
(WebDriver::WebDriverService::connectToBrowser): Create a session host for the next merged capabilities and
connect to the browser.
(WebDriver::WebDriverService::createSession): Start a new automation session. If capabilities didn't match,
start the process again calling connectToBrowser(), otherwise create the new session and top level.
* WebDriverService.h:
* glib/SessionHostGlib.cpp:
(WebDriver::matchBrowserOptions): Helper to check browser options.
(WebDriver::SessionHost::matchCapabilities): Use matchBrowserOptions() and return true or false instead of an
optional error message.
(WebDriver::SessionHost::startAutomationSession): Create the session ID here and notify the caller in case
capabilities didn't match.
(WebDriver::SessionHost::setTargetList): Notify that capabilities did match.
* gtk/WebDriverServiceGtk.cpp:
(WebDriver::WebDriverService::platformMatchCapability const): Make it return bool.
* wpe/WebDriverServiceWPE.cpp:
(WebDriver::WebDriverService::platformMatchCapability const): Ditto.

WebDriverTests:

Remove expectations for imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_browserName.

* TestExpectations.json:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227603 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebDriver/ChangeLog
Source/WebDriver/Session.cpp
Source/WebDriver/Session.h
Source/WebDriver/SessionHost.h
Source/WebDriver/WebDriverService.cpp
Source/WebDriver/WebDriverService.h
Source/WebDriver/glib/SessionHostGlib.cpp
Source/WebDriver/gtk/WebDriverServiceGtk.cpp
Source/WebDriver/wpe/WebDriverServiceWPE.cpp
WebDriverTests/ChangeLog
WebDriverTests/TestExpectations.json

index 57c5bff..9f2083c 100644 (file)
@@ -1,5 +1,50 @@
 2018-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_browserName fails
+        https://bugs.webkit.org/show_bug.cgi?id=181985
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        The problem is that we are considering a failure when the browser name doesn't match the capabilities, instead
+        of trying with the next merged capabilities. This is happening because when processing capabilities, we only
+        match the ones that we know without having to launch the browser. Browser name and version are provided by the
+        browser during the session initialization. This patch reworks the session creation to make it possible to try
+        with the next merged capabilities when matching fails after the browser is launched.
+
+        * Session.cpp:
+        (WebDriver::Session::Session): Initialize timeouts from capabilities, because now we have the final capabilities here.
+        (WebDriver::Session::id const): Return the session ID from the SessionHost, since it's now created there.
+        (WebDriver::Session::createTopLevelBrowsingContext): Do not start the session, it has already been started a
+        this point.
+        (WebDriver::Session::createElement): Use id() instead of m_id.
+        * Session.h:
+        * SessionHost.h:
+        (WebDriver::SessionHost::sessionID const): Return the session ID.
+        * WebDriverService.cpp:
+        (WebDriver::WebDriverService::matchCapabilities const): Remove the error handling, and return a boolean instead,
+        since not mathing is not an error.
+        (WebDriver::WebDriverService::processCapabilities const): Return a list of matched capabilities, instead of the
+        JSON object corresponding to the first match.
+        (WebDriver::WebDriverService::newSession): Use helper connectToBrowser().
+        (WebDriver::WebDriverService::connectToBrowser): Create a session host for the next merged capabilities and
+        connect to the browser.
+        (WebDriver::WebDriverService::createSession): Start a new automation session. If capabilities didn't match,
+        start the process again calling connectToBrowser(), otherwise create the new session and top level.
+        * WebDriverService.h:
+        * glib/SessionHostGlib.cpp:
+        (WebDriver::matchBrowserOptions): Helper to check browser options.
+        (WebDriver::SessionHost::matchCapabilities): Use matchBrowserOptions() and return true or false instead of an
+        optional error message.
+        (WebDriver::SessionHost::startAutomationSession): Create the session ID here and notify the caller in case
+        capabilities didn't match.
+        (WebDriver::SessionHost::setTargetList): Notify that capabilities did match.
+        * gtk/WebDriverServiceGtk.cpp:
+        (WebDriver::WebDriverService::platformMatchCapability const): Make it return bool.
+        * wpe/WebDriverServiceWPE.cpp:
+        (WebDriver::WebDriverService::platformMatchCapability const): Ditto.
+
+2018-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_platformName fails
         https://bugs.webkit.org/show_bug.cgi?id=181984
 
index 6a1abc4..48adfe8 100644 (file)
@@ -31,7 +31,6 @@
 #include "WebDriverAtoms.h"
 #include <wtf/CryptographicallyRandomNumber.h>
 #include <wtf/HexNumber.h>
-#include <wtf/UUID.h>
 
 namespace WebDriver {
 
@@ -48,17 +47,23 @@ static const Seconds defaultImplicitWaitTimeout = 0_s;
 
 Session::Session(std::unique_ptr<SessionHost>&& host)
     : m_host(WTFMove(host))
-    , m_id(createCanonicalUUIDString())
     , m_scriptTimeout(defaultScriptTimeout)
     , m_pageLoadTimeout(defaultPageLoadTimeout)
     , m_implicitWaitTimeout(defaultImplicitWaitTimeout)
 {
+    if (capabilities().timeouts)
+        setTimeouts(capabilities().timeouts.value(), [](CommandResult&&) { });
 }
 
 Session::~Session()
 {
 }
 
+const String& Session::id() const
+{
+    return m_host->sessionID();
+}
+
 const Capabilities& Session::capabilities() const
 {
     return m_host->capabilities();
@@ -162,24 +167,18 @@ std::optional<String> Session::pageLoadStrategyString() const
 void Session::createTopLevelBrowsingContext(Function<void (CommandResult&&)>&& completionHandler)
 {
     ASSERT(!m_toplevelBrowsingContext.value());
-    m_host->startAutomationSession(m_id, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](std::optional<String> errorMessage) mutable {
-        if (errorMessage) {
-            completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError, errorMessage.value()));
+    m_host->sendCommandToBackend(ASCIILiteral("createBrowsingContext"), nullptr, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) mutable {
+        if (response.isError || !response.responseObject) {
+            completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_host->sendCommandToBackend(ASCIILiteral("createBrowsingContext"), nullptr, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) mutable {
-            if (response.isError || !response.responseObject) {
-                completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
-                return;
-            }
-            String handle;
-            if (!response.responseObject->getString(ASCIILiteral("handle"), handle)) {
-                completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError));
-                return;
-            }
-            switchToTopLevelBrowsingContext(handle);
-            completionHandler(CommandResult::success());
-        });
+        String handle;
+        if (!response.responseObject->getString(ASCIILiteral("handle"), handle)) {
+            completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError));
+            return;
+        }
+        switchToTopLevelBrowsingContext(handle);
+        completionHandler(CommandResult::success());
     });
 }
 
@@ -818,7 +817,7 @@ RefPtr<JSON::Object> Session::createElement(RefPtr<JSON::Value>&& value)
         return nullptr;
 
     String elementID;
-    if (!valueObject->getString("session-node-" + m_id, elementID))
+    if (!valueObject->getString("session-node-" + id(), elementID))
         return nullptr;
 
     RefPtr<JSON::Object> elementObject = JSON::Object::create();
@@ -829,7 +828,7 @@ RefPtr<JSON::Object> Session::createElement(RefPtr<JSON::Value>&& value)
 RefPtr<JSON::Object> Session::createElement(const String& elementID)
 {
     RefPtr<JSON::Object> elementObject = JSON::Object::create();
-    elementObject->setString("session-node-" + m_id, elementID);
+    elementObject->setString("session-node-" + id(), elementID);
     return elementObject;
 }
 
index 5bcd9f6..03f77df 100644 (file)
@@ -46,7 +46,7 @@ public:
     }
     ~Session();
 
-    const String& id() const { return m_id; }
+    const String& id() const;
     const Capabilities& capabilities() const;
     Seconds scriptTimeout() const  { return m_scriptTimeout; }
     Seconds pageLoadTimeout() const { return m_pageLoadTimeout; }
@@ -181,7 +181,6 @@ private:
     void performKeyboardInteractions(Vector<KeyboardInteraction>&&, Function<void (CommandResult&&)>&&);
 
     std::unique_ptr<SessionHost> m_host;
-    String m_id;
     Seconds m_scriptTimeout;
     Seconds m_pageLoadTimeout;
     Seconds m_implicitWaitTimeout;
index cbb0512..efaf127 100644 (file)
@@ -51,10 +51,11 @@ public:
 
     bool isConnected() const;
 
+    const String& sessionID() const { return m_sessionID; }
     const Capabilities& capabilities() const { return m_capabilities; }
 
     void connectToBrowser(Function<void (std::optional<String> error)>&&);
-    void startAutomationSession(const String& sessionID, Function<void (std::optional<String>)>&&);
+    void startAutomationSession(Function<void (bool, std::optional<String>)>&&);
 
     struct CommandResponse {
         RefPtr<JSON::Object> responseObject;
@@ -78,7 +79,7 @@ private:
     static const GDBusInterfaceVTable s_interfaceVTable;
     void launchBrowser(Function<void (std::optional<String> error)>&&);
     void connectToBrowser(std::unique_ptr<ConnectToBrowserAsyncData>&&);
-    std::optional<String> matchCapabilities(GVariant*);
+    bool matchCapabilities(GVariant*);
     void setupConnection(GRefPtr<GDBusConnection>&&);
     void setTargetList(uint64_t connectionID, Vector<Target>&&);
     void sendMessageToFrontend(uint64_t connectionID, uint64_t targetID, const char* message);
@@ -86,13 +87,14 @@ private:
 
     Capabilities m_capabilities;
 
+    String m_sessionID;
     uint64_t m_connectionID { 0 };
     Target m_target;
 
     HashMap<long, Function<void (CommandResponse&&)>> m_commandRequests;
 
 #if USE(GLIB)
-    Function<void (std::optional<String>)> m_startSessionCompletionHandler;
+    Function<void (bool, std::optional<String>)> m_startSessionCompletionHandler;
     GRefPtr<GSubprocess> m_browser;
     GRefPtr<GDBusConnection> m_dbusConnection;
     GRefPtr<GCancellable> m_cancellable;
index c19c9ee..e1dc83c 100644 (file)
@@ -448,7 +448,7 @@ RefPtr<JSON::Object> WebDriverService::mergeCapabilities(const JSON::Object& req
     return result;
 }
 
-RefPtr<JSON::Object> WebDriverService::matchCapabilities(const JSON::Object& mergedCapabilities, std::optional<String>& errorString) const
+RefPtr<JSON::Object> WebDriverService::matchCapabilities(const JSON::Object& mergedCapabilities) const
 {
     // §7.2 Processing Capabilities.
     // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-matching-capabilities
@@ -473,44 +473,34 @@ RefPtr<JSON::Object> WebDriverService::matchCapabilities(const JSON::Object& mer
         if (it->key == "browserName" && platformCapabilities.browserName) {
             String browserName;
             it->value->asString(browserName);
-            if (!equalIgnoringASCIICase(platformCapabilities.browserName.value(), browserName)) {
-                errorString = makeString("expected browserName ", platformCapabilities.browserName.value(), " but got ", browserName);
+            if (!equalIgnoringASCIICase(platformCapabilities.browserName.value(), browserName))
                 return nullptr;
-            }
         } else if (it->key == "browserVersion" && platformCapabilities.browserVersion) {
             String browserVersion;
             it->value->asString(browserVersion);
-            if (!platformCompareBrowserVersions(browserVersion, platformCapabilities.browserVersion.value())) {
-                errorString = makeString("requested browserVersion is ", browserVersion, " but actual version is ", platformCapabilities.browserVersion.value());
+            if (!platformCompareBrowserVersions(browserVersion, platformCapabilities.browserVersion.value()))
                 return nullptr;
-            }
         } else if (it->key == "platformName" && platformCapabilities.platformName) {
             String platformName;
             it->value->asString(platformName);
-            if (!equalLettersIgnoringASCIICase(platformName, "any") && platformCapabilities.platformName.value() != platformName) {
-                errorString = makeString("expected platformName ", platformCapabilities.platformName.value(), " but got ", platformName);
+            if (!equalLettersIgnoringASCIICase(platformName, "any") && platformCapabilities.platformName.value() != platformName)
                 return nullptr;
-            }
         } else if (it->key == "acceptInsecureCerts" && platformCapabilities.acceptInsecureCerts) {
             bool acceptInsecureCerts;
             it->value->asBoolean(acceptInsecureCerts);
-            if (acceptInsecureCerts && !platformCapabilities.acceptInsecureCerts.value()) {
-                errorString = String("browser doesn't accept insecure TLS certificates");
+            if (acceptInsecureCerts && !platformCapabilities.acceptInsecureCerts.value())
                 return nullptr;
-            }
         } else if (it->key == "proxy") {
             // FIXME: implement proxy support.
-        } else if (auto platformErrorString = platformMatchCapability(it->key, it->value)) {
-            errorString = platformErrorString;
+        } else if (!platformMatchCapability(it->key, it->value))
             return nullptr;
-        }
         matchedCapabilities->setValue(it->key, RefPtr<JSON::Value>(it->value));
     }
 
     return matchedCapabilities;
 }
 
-RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& parameters, Function<void (CommandResult&&)>& completionHandler) const
+Vector<Capabilities> WebDriverService::processCapabilities(const JSON::Object& parameters, Function<void (CommandResult&&)>& completionHandler) const
 {
     // §7.2 Processing Capabilities.
     // https://w3c.github.io/webdriver/webdriver-spec.html#processing-capabilities
@@ -519,7 +509,7 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
     RefPtr<JSON::Object> capabilitiesObject;
     if (!parameters.getObject(ASCIILiteral("capabilities"), capabilitiesObject)) {
         completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument));
-        return nullptr;
+        return { };
     }
 
     // 2. Let required capabilities be the result of getting the property "alwaysMatch" from capabilities request.
@@ -530,14 +520,14 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
         requiredCapabilities = JSON::Object::create();
     else if (!requiredCapabilitiesValue->asObject(requiredCapabilities)) {
         completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("alwaysMatch is invalid in capabilities")));
-        return nullptr;
+        return { };
     }
 
     // 2.2. Let required capabilities be the result of trying to validate capabilities with argument required capabilities.
     requiredCapabilities = validatedCapabilities(*requiredCapabilities);
     if (!requiredCapabilities) {
         completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("Invalid alwaysMatch capabilities")));
-        return nullptr;
+        return { };
     }
 
     // 3. Let all first match capabilities be the result of getting the property "firstMatch" from capabilities request.
@@ -550,7 +540,7 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
     } else if (!firstMatchCapabilitiesValue->asArray(firstMatchCapabilitiesList)) {
         // 3.2. If all first match capabilities is not a JSON List, return error with error code invalid argument.
         completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("firstMatch is invalid in capabilities")));
-        return nullptr;
+        return { };
     }
 
     // 4. Let validated first match capabilities be an empty JSON List.
@@ -563,13 +553,13 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
         RefPtr<JSON::Object> firstMatchCapabilities;
         if (!firstMatchCapabilitiesValue->asObject(firstMatchCapabilities)) {
             completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("Invalid capabilities found in firstMatch")));
-            return nullptr;
+            return { };
         }
         // 5.1. Let validated capabilities be the result of trying to validate capabilities with argument first match capabilities.
         firstMatchCapabilities = validatedCapabilities(*firstMatchCapabilities);
         if (!firstMatchCapabilities) {
             completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("Invalid firstMatch capabilities")));
-            return nullptr;
+            return { };
         }
 
         // Validate here that firstMatchCapabilities don't shadow alwaysMatchCapabilities.
@@ -579,7 +569,7 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
             if (requiredCapabilities->find(it->key) != requiredEnd) {
                 completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument,
                     makeString("Invalid firstMatch capabilities: key ", it->key, " is present in alwaysMatch")));
-                return nullptr;
+                return { };
             }
         }
 
@@ -588,24 +578,27 @@ RefPtr<JSON::Object> WebDriverService::processCapabilities(const JSON::Object& p
     }
 
     // 6. For each first match capabilities corresponding to an indexed property in validated first match capabilities.
-    std::optional<String> errorString;
+    Vector<Capabilities> matchedCapabilitiesList;
+    matchedCapabilitiesList.reserveInitialCapacity(validatedFirstMatchCapabilitiesList.size());
     for (auto& validatedFirstMatchCapabilies : validatedFirstMatchCapabilitiesList) {
         // 6.1. Let merged capabilities be the result of trying to merge capabilities with required capabilities and first match capabilities as arguments.
         auto mergedCapabilities = mergeCapabilities(*requiredCapabilities, *validatedFirstMatchCapabilies);
-        if (!mergedCapabilities) {
-            completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, String("Same capability found in firstMatch and alwaysMatch")));
-            return nullptr;
-        }
+
         // 6.2. Let matched capabilities be the result of trying to match capabilities with merged capabilities as an argument.
-        auto matchedCapabilities = matchCapabilities(*mergedCapabilities, errorString);
-        if (matchedCapabilities) {
+        if (auto matchedCapabilities = matchCapabilities(*mergedCapabilities)) {
             // 6.3. If matched capabilities is not null return matched capabilities.
-            return matchedCapabilities;
+            Capabilities capabilities;
+            parseCapabilities(*matchedCapabilities, capabilities);
+            matchedCapabilitiesList.uncheckedAppend(WTFMove(capabilities));
         }
     }
 
-    completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidArgument, errorString ? errorString.value() : String("Invalid capabilities")));
-    return nullptr;
+    if (matchedCapabilitiesList.isEmpty()) {
+        completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, String("Failed to match capabilities")));
+        return { };
+    }
+
+    return matchedCapabilitiesList;
 }
 
 void WebDriverService::newSession(RefPtr<JSON::Object>&& parameters, Function<void (CommandResult&&)>&& completionHandler)
@@ -617,20 +610,47 @@ void WebDriverService::newSession(RefPtr<JSON::Object>&& parameters, Function<vo
         return;
     }
 
-    auto matchedCapabilities = processCapabilities(*parameters, completionHandler);
-    if (!matchedCapabilities)
+    auto matchedCapabilitiesList = processCapabilities(*parameters, completionHandler);
+    if (matchedCapabilitiesList.isEmpty())
+        return;
+
+    // Reverse the vector to always take last item.
+    matchedCapabilitiesList.reverse();
+    connectToBrowser(WTFMove(matchedCapabilitiesList), WTFMove(completionHandler));
+}
+
+void WebDriverService::connectToBrowser(Vector<Capabilities>&& capabilitiesList, Function<void (CommandResult&&)>&& completionHandler)
+{
+    if (capabilitiesList.isEmpty()) {
+        completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, String("Failed to match capabilities")));
         return;
+    }
 
-    Capabilities capabilities;
-    parseCapabilities(*matchedCapabilities, capabilities);
-    auto sessionHost = std::make_unique<SessionHost>(WTFMove(capabilities));
+    auto sessionHost = std::make_unique<SessionHost>(capabilitiesList.takeLast());
     auto* sessionHostPtr = sessionHost.get();
-    sessionHostPtr->connectToBrowser([this, sessionHost = WTFMove(sessionHost), completionHandler = WTFMove(completionHandler)](std::optional<String> error) mutable {
+    sessionHostPtr->connectToBrowser([this, capabilitiesList = WTFMove(capabilitiesList), sessionHost = WTFMove(sessionHost), completionHandler = WTFMove(completionHandler)](std::optional<String> error) mutable {
         if (error) {
             completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, makeString("Failed to connect to browser: ", error.value())));
             return;
         }
 
+        createSession(WTFMove(capabilitiesList), WTFMove(sessionHost), WTFMove(completionHandler));
+    });
+}
+
+void WebDriverService::createSession(Vector<Capabilities>&& capabilitiesList, std::unique_ptr<SessionHost>&& sessionHost, Function<void (CommandResult&&)>&& completionHandler)
+{
+    auto* sessionHostPtr = sessionHost.get();
+    sessionHostPtr->startAutomationSession([this, capabilitiesList = WTFMove(capabilitiesList), sessionHost = WTFMove(sessionHost), completionHandler = WTFMove(completionHandler)](bool capabilitiesDidMatch, std::optional<String> errorMessage) mutable {
+        if (errorMessage) {
+            completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError, errorMessage.value()));
+            return;
+        }
+        if (!capabilitiesDidMatch) {
+            connectToBrowser(WTFMove(capabilitiesList), WTFMove(completionHandler));
+            return;
+        }
+
         RefPtr<Session> session = Session::create(WTFMove(sessionHost));
         session->createTopLevelBrowsingContext([this, session, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {
             if (result.isError()) {
@@ -640,13 +660,10 @@ void WebDriverService::newSession(RefPtr<JSON::Object>&& parameters, Function<vo
 
             m_session = WTFMove(session);
 
-            const auto& capabilities = m_session->capabilities();
-            if (capabilities.timeouts)
-                m_session->setTimeouts(capabilities.timeouts.value(), [](CommandResult&&) { });
-
             RefPtr<JSON::Object> resultObject = JSON::Object::create();
             resultObject->setString(ASCIILiteral("sessionId"), m_session->id());
             RefPtr<JSON::Object> capabilitiesObject = JSON::Object::create();
+            const auto& capabilities = m_session->capabilities();
             if (capabilities.browserName)
                 capabilitiesObject->setString(ASCIILiteral("browserName"), capabilities.browserName.value());
             if (capabilities.browserVersion)
index e6b6fbc..40e37ed 100644 (file)
@@ -111,14 +111,16 @@ private:
     void takeElementScreenshot(RefPtr<JSON::Object>&&, Function<void (CommandResult&&)>&&);
 
     static Capabilities platformCapabilities();
-    RefPtr<JSON::Object> processCapabilities(const JSON::Object&, Function<void (CommandResult&&)>&) const;
+    Vector<Capabilities> processCapabilities(const JSON::Object&, Function<void (CommandResult&&)>&) const;
     RefPtr<JSON::Object> validatedCapabilities(const JSON::Object&) const;
     RefPtr<JSON::Object> mergeCapabilities(const JSON::Object&, const JSON::Object&) const;
-    RefPtr<JSON::Object> matchCapabilities(const JSON::Object&, std::optional<String>&) const;
+    RefPtr<JSON::Object> matchCapabilities(const JSON::Object&) const;
     bool platformValidateCapability(const String&, const RefPtr<JSON::Value>&) const;
-    std::optional<String> platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const;
+    bool platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const;
     void parseCapabilities(const JSON::Object& desiredCapabilities, Capabilities&) const;
     void platformParseCapabilities(const JSON::Object& desiredCapabilities, Capabilities&) const;
+    void connectToBrowser(Vector<Capabilities>&&, Function<void (CommandResult&&)>&&);
+    void createSession(Vector<Capabilities>&&, std::unique_ptr<SessionHost>&&, Function<void (CommandResult&&)>&&);
     bool findSessionOrCompleteWithError(JSON::Object&, Function<void (CommandResult&&)>&);
 
     void handleRequest(HTTPRequestHandler::Request&&, Function<void (HTTPRequestHandler::Response&&)>&& replyHandler) override;
index b2b414c..795b092 100644 (file)
@@ -29,6 +29,7 @@
 #include "WebDriverService.h"
 #include <gio/gio.h>
 #include <wtf/RunLoop.h>
+#include <wtf/UUID.h>
 #include <wtf/glib/GUniquePtr.h>
 
 #define REMOTE_INSPECTOR_CLIENT_DBUS_INTERFACE "org.webkit.RemoteInspectorClient"
@@ -234,37 +235,43 @@ void SessionHost::setupConnection(GRefPtr<GDBusConnection>&& connection)
     g_dbus_connection_register_object(m_dbusConnection.get(), REMOTE_INSPECTOR_CLIENT_OBJECT_PATH, introspectionData->interfaces[0], &s_interfaceVTable, this, nullptr, nullptr);
 }
 
-std::optional<String> SessionHost::matchCapabilities(GVariant* capabilities)
+static bool matchBrowserOptions(const String& browserName, const String& browserVersion, const Capabilities& capabilities)
 {
-    const char* browserName;
-    const char* browserVersion;
-    g_variant_get(capabilities, "(&s&s)", &browserName, &browserVersion);
-
-    if (m_capabilities.browserName) {
-        if (m_capabilities.browserName.value() != browserName)
-            return makeString("expected browserName ", m_capabilities.browserName.value(), " but got ", browserName);
-    } else
-        m_capabilities.browserName = String(browserName);
-
-    if (m_capabilities.browserVersion) {
-        if (!WebDriverService::platformCompareBrowserVersions(m_capabilities.browserVersion.value(), browserVersion))
-            return makeString("requested browserVersion is ", m_capabilities.browserVersion.value(), " but actual version is ", browserVersion);
-    } else
-        m_capabilities.browserVersion = String(browserVersion);
-
-    return std::nullopt;
+    if (capabilities.browserName && capabilities.browserName.value() != browserName)
+        return false;
+
+    if (capabilities.browserVersion && !WebDriverService::platformCompareBrowserVersions(capabilities.browserVersion.value(), browserVersion))
+        return false;
+
+    return true;
+}
+
+bool SessionHost::matchCapabilities(GVariant* capabilities)
+{
+    const char* name;
+    const char* version;
+    g_variant_get(capabilities, "(&s&s)", &name, &version);
+
+    auto browserName = String::fromUTF8(name);
+    auto browserVersion = String::fromUTF8(version);
+    bool didMatch = matchBrowserOptions(browserName, browserVersion, m_capabilities);
+    m_capabilities.browserName = browserName;
+    m_capabilities.browserVersion = browserVersion;
+
+    return didMatch;
 }
 
-void SessionHost::startAutomationSession(const String& sessionID, Function<void (std::optional<String>)>&& completionHandler)
+void SessionHost::startAutomationSession(Function<void (bool, std::optional<String>)>&& completionHandler)
 {
     ASSERT(m_dbusConnection);
     ASSERT(!m_startSessionCompletionHandler);
     m_startSessionCompletionHandler = WTFMove(completionHandler);
+    m_sessionID = createCanonicalUUIDString();
     g_dbus_connection_call(m_dbusConnection.get(), nullptr,
         INSPECTOR_DBUS_OBJECT_PATH,
         INSPECTOR_DBUS_INTERFACE,
         "StartAutomationSession",
-        g_variant_new("(s)", sessionID.utf8().data()),
+        g_variant_new("(s)", m_sessionID.utf8().data()),
         nullptr, G_DBUS_CALL_FLAGS_NO_AUTO_START,
         -1, m_cancellable.get(), [](GObject* source, GAsyncResult* result, gpointer userData) {
             GUniqueOutPtr<GError> error;
@@ -275,14 +282,13 @@ void SessionHost::startAutomationSession(const String& sessionID, Function<void
             auto sessionHost = static_cast<SessionHost*>(userData);
             if (!resultVariant) {
                 auto completionHandler = std::exchange(sessionHost->m_startSessionCompletionHandler, nullptr);
-                completionHandler(String("Failed to start automation session"));
+                completionHandler(false, String("Failed to start automation session"));
                 return;
             }
 
-            auto errorString = sessionHost->matchCapabilities(resultVariant.get());
-            if (errorString) {
+            if (!sessionHost->matchCapabilities(resultVariant.get())) {
                 auto completionHandler = std::exchange(sessionHost->m_startSessionCompletionHandler, nullptr);
-                completionHandler(errorString);
+                completionHandler(false, std::nullopt);
                 return;
             }
         }, this
@@ -324,7 +330,7 @@ void SessionHost::setTargetList(uint64_t connectionID, Vector<Target>&& targetLi
         -1, m_cancellable.get(), dbusConnectionCallAsyncReadyCallback, nullptr);
 
     auto startSessionCompletionHandler = std::exchange(m_startSessionCompletionHandler, nullptr);
-    startSessionCompletionHandler(std::nullopt);
+    startSessionCompletionHandler(true, std::nullopt);
 }
 
 void SessionHost::sendMessageToFrontend(uint64_t connectionID, uint64_t targetID, const char* message)
index 191342d..0c4d274 100644 (file)
@@ -81,9 +81,9 @@ bool WebDriverService::platformValidateCapability(const String& name, const RefP
     return true;
 }
 
-std::optional<String> WebDriverService::platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const
+bool WebDriverService::platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const
 {
-    return std::nullopt;
+    return true;
 }
 
 void WebDriverService::platformParseCapabilities(const JSON::Object& matchedCapabilities, Capabilities& capabilities) const
index 608cb7c..523e8e7 100644 (file)
@@ -74,9 +74,9 @@ bool WebDriverService::platformValidateCapability(const String& name, const RefP
     return true;
 }
 
-std::optional<String> WebDriverService::platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const
+bool WebDriverService::platformMatchCapability(const String&, const RefPtr<JSON::Value>&) const
 {
-    return std::nullopt;
+    return true;
 }
 
 void WebDriverService::platformParseCapabilities(const JSON::Object& matchedCapabilities, Capabilities& capabilities) const
index 5e232c0..ded5f38 100644 (file)
@@ -1,5 +1,16 @@
 2018-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_browserName fails
+        https://bugs.webkit.org/show_bug.cgi?id=181985
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Remove expectations for imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_browserName.
+
+        * TestExpectations.json:
+
+2018-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/merge.py::test_merge_platformName fails
         https://bugs.webkit.org/show_bug.cgi?id=181984
 
index f527013..e0167b2 100644 (file)
             }
         }
     },
-    "imported/w3c/webdriver/tests/sessions/new_session/merge.py": {
-        "subtests": {
-            "test_merge_browserName": {
-                "expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/181985"}}
-            }
-        }
-    },
     "imported/w3c/webdriver/tests/sessions/new_session/invalid_capabilities.py": {
         "subtests": {
             "test_invalid_values[proxy-1-body0]": {