WebDriver: WebDriverService::matchCapabilities should follow the spec
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Nov 2017 07:27:30 +0000 (07:27 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Nov 2017 07:27:30 +0000 (07:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179371

Reviewed by Brian Burg.

The returned object should contain all the entries mentioned in the spec, not only the ones already present in
the passed in capabilities object.

7.2 Processing Capabilities
https://w3c.github.io/webdriver/webdriver-spec.html#dfn-matching-capabilities

* WebDriverService.cpp:
(WebDriver::WebDriverService::matchCapabilities const):
(WebDriver::WebDriverService::processCapabilities const):
* WebDriverService.h:

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

Source/WebDriver/ChangeLog
Source/WebDriver/WebDriverService.cpp
Source/WebDriver/WebDriverService.h

index 4ca9a6d..294b576 100644 (file)
@@ -1,5 +1,23 @@
 2017-11-09  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        WebDriver: WebDriverService::matchCapabilities should follow the spec
+        https://bugs.webkit.org/show_bug.cgi?id=179371
+
+        Reviewed by Brian Burg.
+
+        The returned object should contain all the entries mentioned in the spec, not only the ones already present in
+        the passed in capabilities object.
+
+        7.2 Processing Capabilities
+        https://w3c.github.io/webdriver/webdriver-spec.html#dfn-matching-capabilities
+
+        * WebDriverService.cpp:
+        (WebDriver::WebDriverService::matchCapabilities const):
+        (WebDriver::WebDriverService::processCapabilities const):
+        * WebDriverService.h:
+
+2017-11-09  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         WebDriver: capabilities with null value shouldn't be added to the validated capabilities object
         https://bugs.webkit.org/show_bug.cgi?id=179369
 
index c47ec34..423b8c5 100644 (file)
@@ -430,43 +430,64 @@ RefPtr<InspectorObject> WebDriverService::mergeCapabilities(const InspectorObjec
     return result;
 }
 
-std::optional<String> WebDriverService::matchCapabilities(const InspectorObject& mergedCapabilities) const
+RefPtr<InspectorObject> WebDriverService::matchCapabilities(const InspectorObject& mergedCapabilities, std::optional<String>& errorString) const
 {
     // ยง7.2 Processing Capabilities.
     // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-matching-capabilities
-    Capabilities matchedCapabilities = platformCapabilities();
+    Capabilities platformCapabilities = this->platformCapabilities();
 
     // Some capabilities like browser name and version might need to launch the browser,
     // so we only reject the known capabilities that don't match.
+    RefPtr<InspectorObject> matchedCapabilities = InspectorObject::create();
+    if (platformCapabilities.browserName)
+        matchedCapabilities->setString(ASCIILiteral("browserName"), platformCapabilities.browserName.value());
+    if (platformCapabilities.browserVersion)
+        matchedCapabilities->setString(ASCIILiteral("browserVersion"), platformCapabilities.browserVersion.value());
+    if (platformCapabilities.platformName)
+        matchedCapabilities->setString(ASCIILiteral("platformName"), platformCapabilities.platformName.value());
+    if (platformCapabilities.acceptInsecureCerts)
+        matchedCapabilities->setBoolean(ASCIILiteral("acceptInsecureCerts"), platformCapabilities.acceptInsecureCerts.value());
+
     auto end = mergedCapabilities.end();
     for (auto it = mergedCapabilities.begin(); it != end; ++it) {
-        if (it->key == "browserName" && matchedCapabilities.browserName) {
+        if (it->key == "browserName" && platformCapabilities.browserName) {
             String browserName;
             it->value->asString(browserName);
-            if (!equalIgnoringASCIICase(matchedCapabilities.browserName.value(), browserName))
-                return makeString("expected browserName ", matchedCapabilities.browserName.value(), " but got ", browserName);
-        } else if (it->key == "browserVersion" && matchedCapabilities.browserVersion) {
+            if (!equalIgnoringASCIICase(platformCapabilities.browserName.value(), browserName)) {
+                errorString = makeString("expected browserName ", platformCapabilities.browserName.value(), " but got ", browserName);
+                return nullptr;
+            }
+        } else if (it->key == "browserVersion" && platformCapabilities.browserVersion) {
             String browserVersion;
             it->value->asString(browserVersion);
-            if (!platformCompareBrowserVersions(browserVersion, matchedCapabilities.browserVersion.value()))
-                return makeString("requested browserVersion is ", browserVersion, " but actual version is ", matchedCapabilities.browserVersion.value());
-        } else if (it->key == "platformName" && matchedCapabilities.platformName) {
+            if (!platformCompareBrowserVersions(browserVersion, platformCapabilities.browserVersion.value())) {
+                errorString = makeString("requested browserVersion is ", browserVersion, " but actual version is ", platformCapabilities.browserVersion.value());
+                return nullptr;
+            }
+        } else if (it->key == "platformName" && platformCapabilities.platformName) {
             String platformName;
             it->value->asString(platformName);
-            if (!equalLettersIgnoringASCIICase(platformName, "any") && !equalIgnoringASCIICase(matchedCapabilities.platformName.value(), platformName))
-                return makeString("expected platformName ", matchedCapabilities.platformName.value(), " but got ", platformName);
-        } else if (it->key == "acceptInsecureCerts" && matchedCapabilities.acceptInsecureCerts) {
+            if (!equalLettersIgnoringASCIICase(platformName, "any") && !equalIgnoringASCIICase(platformCapabilities.platformName.value(), platformName)) {
+                errorString = makeString("expected platformName ", platformCapabilities.platformName.value(), " but got ", platformName);
+                return nullptr;
+            }
+        } else if (it->key == "acceptInsecureCerts" && platformCapabilities.acceptInsecureCerts) {
             bool acceptInsecureCerts;
             it->value->asBoolean(acceptInsecureCerts);
-            if (acceptInsecureCerts && !matchedCapabilities.acceptInsecureCerts.value())
-                return String("browser doesn't accept insecure TLS certificates");
+            if (acceptInsecureCerts && !platformCapabilities.acceptInsecureCerts.value()) {
+                errorString = String("browser doesn't accept insecure TLS certificates");
+                return nullptr;
+            }
         } else if (it->key == "proxy") {
             // FIXME: implement proxy support.
-        } else if (auto errorString = platformMatchCapability(it->key, it->value))
-            return errorString;
+        } else if (auto platformErrorString = platformMatchCapability(it->key, it->value)) {
+            errorString = platformErrorString;
+            return nullptr;
+        }
+        matchedCapabilities->setValue(it->key, RefPtr<InspectorValue>(it->value));
     }
 
-    return std::nullopt;
+    return matchedCapabilities;
 }
 
 RefPtr<InspectorObject> WebDriverService::processCapabilities(const InspectorObject& parameters, Function<void (CommandResult&&)>& completionHandler) const
@@ -544,10 +565,10 @@ RefPtr<InspectorObject> WebDriverService::processCapabilities(const InspectorObj
             return nullptr;
         }
         // 6.2. Let matched capabilities be the result of trying to match capabilities with merged capabilities as an argument.
-        errorString = matchCapabilities(*mergedCapabilities);
-        if (!errorString) {
+        auto matchedCapabilities = matchCapabilities(*mergedCapabilities, errorString);
+        if (matchedCapabilities) {
             // 6.3. If matched capabilities is not null return matched capabilities.
-            return mergedCapabilities;
+            return matchedCapabilities;
         }
     }
 
index fe87358..d296090 100644 (file)
@@ -114,7 +114,7 @@ private:
     RefPtr<Inspector::InspectorObject> processCapabilities(const Inspector::InspectorObject&, Function<void (CommandResult&&)>&) const;
     RefPtr<Inspector::InspectorObject> validatedCapabilities(const Inspector::InspectorObject&) const;
     RefPtr<Inspector::InspectorObject> mergeCapabilities(const Inspector::InspectorObject&, const Inspector::InspectorObject&) const;
-    std::optional<String> matchCapabilities(const Inspector::InspectorObject&) const;
+    RefPtr<Inspector::InspectorObject> matchCapabilities(const Inspector::InspectorObject&, std::optional<String>&) const;
     bool platformValidateCapability(const String&, const RefPtr<Inspector::InspectorValue>&) const;
     std::optional<String> platformMatchCapability(const String&, const RefPtr<Inspector::InspectorValue>&) const;
     void parseCapabilities(const Inspector::InspectorObject& desiredCapabilities, Capabilities&) const;