Merge r225210 - [GTK] WebDriver: stop making mandatory to provide a b…

…rowser path if webkitgtk:browserOptions is present in capabilities https://bugs.webkit.org/show_bug.cgi?id=180012 Reviewed by Carlos Alberto Lopez Perez. Everything should be optional. We might want to disable overlay scrollbars, but still using the default browser, for example, as I'm doing when running the selenium tests. We might also want to provide additional browser arguments, but using the default browser. * gtk/WebDriverServiceGtk.cpp: (WebDriver::WebDriverService::platformValidateCapability const): Do not consider invalid to not provide a browser binary when webkitgtk:browserOptions is present. (WebDriver::WebDriverService::platformParseCapabilities const): Override default capabilities with the ones provided.
WebKit · Dec 18, 2017 · a2ae05a · a2ae05a
1 parent e1e2a4a
commit a2ae05a
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 20 deletions.
diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog
@@ -1,3 +1,20 @@
+2017-11-28  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebDriver: stop making mandatory to provide a browser path if webkitgtk:browserOptions is present in capabilities
+        https://bugs.webkit.org/show_bug.cgi?id=180012
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Everything should be optional. We might want to disable overlay scrollbars, but still using the default browser,
+        for example, as I'm doing when running the selenium tests. We might also want to provide additional browser
+        arguments, but using the default browser.
+
+        * gtk/WebDriverServiceGtk.cpp:
+        (WebDriver::WebDriverService::platformValidateCapability const): Do not consider invalid to not provide a
+        browser binary when webkitgtk:browserOptions is present.
+        (WebDriver::WebDriverService::platformParseCapabilities const): Override default capabilities with the ones
+        provided.
+
 2017-11-27  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: Implement get active element command

diff --git a/Source/WebDriver/gtk/WebDriverServiceGtk.cpp b/Source/WebDriver/gtk/WebDriverServiceGtk.cpp
@@ -54,9 +54,9 @@ bool WebDriverService::platformValidateCapability(const String& name, const RefP
     if (browserOptions->isNull())
         return true;
 
-    // If browser options are provided, binary is required.
+    RefPtr<InspectorValue> binaryValue;
     String binary;
-    if (!browserOptions->getString(ASCIILiteral("binary"), binary))
+    if (browserOptions->getValue(ASCIILiteral("binary"), binaryValue) && !binaryValue->asString(binary))
         return false;
 
     RefPtr<InspectorValue> useOverlayScrollbarsValue;
@@ -66,15 +66,17 @@ bool WebDriverService::platformValidateCapability(const String& name, const RefP
 
     RefPtr<InspectorValue> browserArgumentsValue;
     RefPtr<InspectorArray> browserArguments;
-    if (browserOptions->getValue(ASCIILiteral("args"), browserArgumentsValue) && !browserArgumentsValue->asArray(browserArguments))
-        return false;
-
-    unsigned browserArgumentsLength = browserArguments->length();
-    for (unsigned i = 0; i < browserArgumentsLength; ++i) {
-        RefPtr<InspectorValue> value = browserArguments->get(i);
-        String argument;
-        if (!value->asString(argument))
+    if (browserOptions->getValue(ASCIILiteral("args"), browserArgumentsValue)) {
+        if (!browserArgumentsValue->asArray(browserArguments))
             return false;
+
+        unsigned browserArgumentsLength = browserArguments->length();
+        for (unsigned i = 0; i < browserArgumentsLength; ++i) {
+            RefPtr<InspectorValue> value = browserArguments->get(i);
+            String argument;
+            if (!value->asString(argument))
+                return false;
+        }
     }
 
     return true;
@@ -87,23 +89,22 @@ std::optional<String> WebDriverService::platformMatchCapability(const String&, c
 
 void WebDriverService::platformParseCapabilities(const InspectorObject& matchedCapabilities, Capabilities& capabilities) const
 {
+    capabilities.browserBinary = String(LIBEXECDIR "/webkit2gtk-" WEBKITGTK_API_VERSION_STRING "/MiniBrowser");
+    capabilities.browserArguments = Vector<String> { ASCIILiteral("--automation") };
+    capabilities.useOverlayScrollbars = true;
+
     RefPtr<InspectorObject> browserOptions;
-    if (!matchedCapabilities.getObject(ASCIILiteral("webkitgtk:browserOptions"), browserOptions)) {
-        capabilities.browserBinary = String(LIBEXECDIR "/webkit2gtk-" WEBKITGTK_API_VERSION_STRING "/MiniBrowser");
-        capabilities.browserArguments = Vector<String> { ASCIILiteral("--automation") };
-        capabilities.useOverlayScrollbars = true;
+    if (!matchedCapabilities.getObject(ASCIILiteral("webkitgtk:browserOptions"), browserOptions))
         return;
-    }
 
     String browserBinary;
-    browserOptions->getString(ASCIILiteral("binary"), browserBinary);
-    ASSERT(!browserBinary.isNull());
-    capabilities.browserBinary = browserBinary;
+    if (browserOptions->getString(ASCIILiteral("binary"), browserBinary))
+        capabilities.browserBinary = browserBinary;
 
-    capabilities.browserArguments = Vector<String>();
     RefPtr<InspectorArray> browserArguments;
-    if (browserOptions->getArray(ASCIILiteral("args"), browserArguments)) {
+    if (browserOptions->getArray(ASCIILiteral("args"), browserArguments) && browserArguments->length()) {
         unsigned browserArgumentsLength = browserArguments->length();
+        capabilities.browserArguments = Vector<String>();
         capabilities.browserArguments->reserveInitialCapacity(browserArgumentsLength);
         for (unsigned i = 0; i < browserArgumentsLength; ++i) {
             RefPtr<InspectorValue> value = browserArguments->get(i);