Title: [228856] trunk/Source
Revision
228856
Author
bb...@apple.com
Date
2018-02-20 20:13:42 -0800 (Tue, 20 Feb 2018)

Log Message

Web Automation: combine session commands to resize and move top-level browsing contexts
https://bugs.webkit.org/show_bug.cgi?id=182749
<rdar://problem/37515170>

Reviewed by Andy Estes.

Source/WebDriver:

The new command can take either size or origin. Just have one session command for use by endpoints.

* Session.cpp:
(WebDriver::Session::setWindowRect):
(WebDriver::Session::moveToplevelBrowsingContextWindow): Deleted.
(WebDriver::Session::resizeToplevelBrowsingContextWindow): Deleted.
* Session.h:

Source/WebKit:

Since moving and resizing the window are both accomplished by setting the window frame,
and the W3C WebDriver specification has a Get/Set Window Rect command, it's time to
deduplicate these two methods which basically do the same thing.

Adopt modern JSON::Value getters that return std::optional<float>. I have been trying
to move the protocol over to this style wholesale, but it is probably easier to do
this conversion in smaller pieces. And so, I have started to do so.

This change is covered by existing WebDriver tests.

* UIProcess/Automation/Automation.json: Add new command.
* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::setWindowFrameOfBrowsingContext): Added.
(WebKit::WebAutomationSession::resizeWindowOfBrowsingContext): Deleted.
(WebKit::WebAutomationSession::moveWindowOfBrowsingContext): Deleted.
* UIProcess/Automation/WebAutomationSession.h:

Source/WTF:

* wtf/JSONValues.h: add a getDouble() implementation that returns a std::optional<T>
rather than using an out-parameter. I'd like to move more code to this style.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (228855 => 228856)


--- trunk/Source/WTF/ChangeLog	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WTF/ChangeLog	2018-02-21 04:13:42 UTC (rev 228856)
@@ -1,3 +1,14 @@
+2018-02-14  Brian Burg  <bb...@apple.com>
+
+        Web Automation: combine session commands to resize and move top-level browsing contexts
+        https://bugs.webkit.org/show_bug.cgi?id=182749
+        <rdar://problem/37515170>
+
+        Reviewed by Andy Estes.
+
+        * wtf/JSONValues.h: add a getDouble() implementation that returns a std::optional<T>
+        rather than using an out-parameter. I'd like to move more code to this style.
+
 2018-02-20  Tim Horton  <timothy_hor...@apple.com>
 
         Introduce HAVE(IOSURFACE_ACCELERATOR)

Modified: trunk/Source/WTF/wtf/JSONValues.h (228855 => 228856)


--- trunk/Source/WTF/wtf/JSONValues.h	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WTF/wtf/JSONValues.h	2018-02-21 04:13:42 UTC (rev 228856)
@@ -217,6 +217,19 @@
         return value->asInteger(output);
     }
 
+    template<class T> std::optional<T> getNumber(const String& name) const
+    {
+        RefPtr<Value> value;
+        if (!getValue(name, value))
+            return std::nullopt;
+
+        T result;
+        if (!value->asDouble(result))
+            return std::nullopt;
+
+        return result;
+    }
+
     bool getString(const String& name, String& output) const;
     bool getObject(const String& name, RefPtr<Object>&) const;
     bool getArray(const String& name, RefPtr<Array>&) const;
@@ -260,6 +273,7 @@
     using ObjectBase::getBoolean;
     using ObjectBase::getInteger;
     using ObjectBase::getDouble;
+    using ObjectBase::getNumber;
     using ObjectBase::getString;
     using ObjectBase::getObject;
     using ObjectBase::getArray;
@@ -472,3 +486,4 @@
 namespace JSON {
 using namespace WTF::JSONImpl;
 }
+

Modified: trunk/Source/WebDriver/ChangeLog (228855 => 228856)


--- trunk/Source/WebDriver/ChangeLog	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebDriver/ChangeLog	2018-02-21 04:13:42 UTC (rev 228856)
@@ -1,3 +1,19 @@
+2018-02-14  Brian Burg  <bb...@apple.com>
+
+        Web Automation: combine session commands to resize and move top-level browsing contexts
+        https://bugs.webkit.org/show_bug.cgi?id=182749
+        <rdar://problem/37515170>
+
+        Reviewed by Andy Estes.
+
+        The new command can take either size or origin. Just have one session command for use by endpoints.
+
+        * Session.cpp:
+        (WebDriver::Session::setWindowRect):
+        (WebDriver::Session::moveToplevelBrowsingContextWindow): Deleted.
+        (WebDriver::Session::resizeToplevelBrowsingContextWindow): Deleted.
+        * Session.h:
+
 2018-01-30  Don Olmstead  <don.olmst...@sony.com>
 
         [CMake] Make WTF headers copies

Modified: trunk/Source/WebDriver/Session.cpp (228855 => 228856)


--- trunk/Source/WebDriver/Session.cpp	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebDriver/Session.cpp	2018-02-21 04:13:42 UTC (rev 228856)
@@ -715,40 +715,6 @@
     });
 }
 
-void Session::moveToplevelBrowsingContextWindow(double x, double y, Function<void (CommandResult&&)>&& completionHandler)
-{
-    RefPtr<JSON::Object> parameters = JSON::Object::create();
-    parameters->setString(ASCIILiteral("handle"), m_toplevelBrowsingContext.value());
-    RefPtr<JSON::Object> windowOrigin = JSON::Object::create();
-    windowOrigin->setDouble("x", x);
-    windowOrigin->setDouble("y", y);
-    parameters->setObject(ASCIILiteral("origin"), WTFMove(windowOrigin));
-    m_host->sendCommandToBackend(ASCIILiteral("moveWindowOfBrowsingContext"), WTFMove(parameters), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) {
-        if (response.isError) {
-            completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
-            return;
-        }
-        completionHandler(CommandResult::success());
-    });
-}
-
-void Session::resizeToplevelBrowsingContextWindow(double width, double height, Function<void (CommandResult&&)>&& completionHandler)
-{
-    RefPtr<JSON::Object> parameters = JSON::Object::create();
-    parameters->setString(ASCIILiteral("handle"), m_toplevelBrowsingContext.value());
-    RefPtr<JSON::Object> windowSize = JSON::Object::create();
-    windowSize->setDouble("width", width);
-    windowSize->setDouble("height", height);
-    parameters->setObject(ASCIILiteral("size"), WTFMove(windowSize));
-    m_host->sendCommandToBackend(ASCIILiteral("resizeWindowOfBrowsingContext"), WTFMove(parameters), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) {
-        if (response.isError) {
-            completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
-            return;
-        }
-        completionHandler(CommandResult::success());
-    });
-}
-
 void Session::getWindowRect(Function<void (CommandResult&&)>&& completionHandler)
 {
     if (!m_toplevelBrowsingContext) {
@@ -778,40 +744,27 @@
             return;
         }
 
-        if (width && height)  {
-            resizeToplevelBrowsingContextWindow(width.value(), height.value(), [this, x, y, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {
-                if (result.isError()) {
-                    completionHandler(WTFMove(result));
-                    return;
-                }
-                if (!x || !y) {
-                    getToplevelBrowsingContextRect(WTFMove(completionHandler));
-                    return;
-                }
-
-                moveToplevelBrowsingContextWindow(x.value(), y.value(), [this, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {
-                    if (result.isError()) {
-                        completionHandler(WTFMove(result));
-                        return;
-                    }
-                    getToplevelBrowsingContextRect(WTFMove(completionHandler));
-                });
-            });
-            return;
-        }
-
+        RefPtr<JSON::Object> parameters = JSON::Object::create();
+        parameters->setString(ASCIILiteral("handle"), m_toplevelBrowsingContext.value());
         if (x && y) {
-            moveToplevelBrowsingContextWindow(x.value(), y.value(), [this, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {
-                if (result.isError()) {
-                    completionHandler(WTFMove(result));
-                    return;
-                }
-                getToplevelBrowsingContextRect(WTFMove(completionHandler));
-            });
-            return;
+            RefPtr<JSON::Object> windowOrigin = JSON::Object::create();
+            windowOrigin->setDouble("x", x.value());
+            windowOrigin->setDouble("y", y.value());
+            parameters->setObject(ASCIILiteral("origin"), WTFMove(windowOrigin));
         }
-
-        getToplevelBrowsingContextRect(WTFMove(completionHandler));
+        if (width && height) {
+            RefPtr<JSON::Object> windowSize = JSON::Object::create();
+            windowSize->setDouble("width", width.value());
+            windowSize->setDouble("height", height.value());
+            parameters->setObject(ASCIILiteral("size"), WTFMove(windowSize));
+        }
+        m_host->sendCommandToBackend(ASCIILiteral("setWindowFrameOfBrowsingContext"), WTFMove(parameters), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (SessionHost::CommandResponse&& response) mutable {
+            if (response.isError) {
+                completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
+                return;
+            }
+            getToplevelBrowsingContextRect(WTFMove(completionHandler));
+        });
     });
 }
 

Modified: trunk/Source/WebDriver/Session.h (228855 => 228856)


--- trunk/Source/WebDriver/Session.h	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebDriver/Session.h	2018-02-21 04:13:42 UTC (rev 228856)
@@ -121,8 +121,6 @@
     void closeAllToplevelBrowsingContexts(const String& toplevelBrowsingContext, Function<void (CommandResult&&)>&&);
 
     void getToplevelBrowsingContextRect(Function<void (CommandResult&&)>&&);
-    void moveToplevelBrowsingContextWindow(double x, double y, Function<void (CommandResult&&)>&&);
-    void resizeToplevelBrowsingContextWindow(double width, double height, Function<void (CommandResult&&)>&&);
 
     std::optional<String> pageLoadStrategyString() const;
 

Modified: trunk/Source/WebKit/ChangeLog (228855 => 228856)


--- trunk/Source/WebKit/ChangeLog	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebKit/ChangeLog	2018-02-21 04:13:42 UTC (rev 228856)
@@ -1,3 +1,28 @@
+2018-02-14  Brian Burg  <bb...@apple.com>
+
+        Web Automation: combine session commands to resize and move top-level browsing contexts
+        https://bugs.webkit.org/show_bug.cgi?id=182749
+        <rdar://problem/37515170>
+
+        Reviewed by Andy Estes.
+
+        Since moving and resizing the window are both accomplished by setting the window frame,
+        and the W3C WebDriver specification has a Get/Set Window Rect command, it's time to
+        deduplicate these two methods which basically do the same thing.
+
+        Adopt modern JSON::Value getters that return std::optional<float>. I have been trying
+        to move the protocol over to this style wholesale, but it is probably easier to do
+        this conversion in smaller pieces. And so, I have started to do so.
+
+        This change is covered by existing WebDriver tests.
+
+        * UIProcess/Automation/Automation.json: Add new command.
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::setWindowFrameOfBrowsingContext): Added.
+        (WebKit::WebAutomationSession::resizeWindowOfBrowsingContext): Deleted.
+        (WebKit::WebAutomationSession::moveWindowOfBrowsingContext): Deleted.
+        * UIProcess/Automation/WebAutomationSession.h:
+
 2018-02-20  Brian Burg  <bb...@apple.com>
 
         ASSERT under WebAutomationSession::setProcessPool() when running W3C test suite a second time

Modified: trunk/Source/WebKit/UIProcess/Automation/Automation.json (228855 => 228856)


--- trunk/Source/WebKit/UIProcess/Automation/Automation.json	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebKit/UIProcess/Automation/Automation.json	2018-02-21 04:13:42 UTC (rev 228856)
@@ -289,24 +289,16 @@
             ]
         },
         {
-            "name": "resizeWindowOfBrowsingContext",
-            "description": "Resizes the window of the specified browsing context to the specified size.",
+            "name": "setWindowFrameOfBrowsingContext",
+            "description": "Moves and/or resizes the window of the specified top-level browsing context to the specified size and origin. Both the size and origin may be omitted. Regardless of the arguments, this command will exit fullscreen and deminiaturize the window if applicable.",
             "parameters": [
                 { "name": "handle", "$ref": "BrowsingContextHandle", "description": "The handle for the browsing context to be resized." },
-                { "name": "size", "$ref": "Size", "description": "The new size for the browsing context's window." }
+                { "name": "origin", "$ref": "Point", "optional": true, "description": "The new origin for the browsing context's window. The position is interpreted in screen coordinate space, relative to the upper left corner of the screen." },
+                { "name": "size", "$ref": "Size", "optional": true, "description": "The new size for the browsing context's window. The size is interpreted in screen pixels." }
             ],
             "async": true
         },
         {
-            "name": "moveWindowOfBrowsingContext",
-            "description": "Moves the window of the specified browsing context to the specified position.",
-            "parameters": [
-                { "name": "handle", "$ref": "BrowsingContextHandle", "description": "The handle for the browsing context to be moved." },
-                { "name": "origin", "$ref": "Point", "description": "The new origin for the browsing context's window. The position is interpreted in screen coordinate space, relative to the upper left corner of the screen." }
-            ],
-            "async": true
-        },
-        {
             "name": "navigateBrowsingContext",
             "description": "Navigates a browsing context to a specified URL.",
             "parameters": [

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp (228855 => 228856)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2018-02-21 04:13:42 UTC (rev 228856)
@@ -313,67 +313,52 @@
     page->process().send(Messages::WebAutomationSessionProxy::FocusFrame(page->pageID(), frameID.value()), 0);
 }
 
-void WebAutomationSession::resizeWindowOfBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const JSON::Object& sizeObject, Ref<ResizeWindowOfBrowsingContextCallback>&& callback)
+void WebAutomationSession::setWindowFrameOfBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const JSON::Object* optionalOriginObject, const JSON::Object* optionalSizeObject, Ref<SetWindowFrameOfBrowsingContextCallback>&& callback)
 {
 #if PLATFORM(IOS)
     FAIL_WITH_PREDEFINED_ERROR(NotImplemented);
 #else
-    float width;
-    if (!sizeObject.getDouble(WTF::ASCIILiteral("width"), width))
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'width' parameter was not found or invalid.");
+    std::optional<float> x;
+    std::optional<float> y;
+    if (optionalOriginObject) {
+        if (!(x = optionalOriginObject->getNumber<float>(WTF::ASCIILiteral("x"))))
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'x' parameter was not found or invalid.");
 
-    float height;
-    if (!sizeObject.getDouble(WTF::ASCIILiteral("height"), height))
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'height' parameter was not found or invalid.");
+        if (!(y = optionalOriginObject->getNumber<float>(WTF::ASCIILiteral("y"))))
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'y' parameter was not found or invalid.");
 
-    if (width < 0)
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'width' parameter had an invalid value.");
+        if (x.value() < 0)
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'x' parameter had an invalid value.");
 
-    if (height < 0)
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'height' parameter had an invalid value.");
+        if (y.value() < 0)
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'y' parameter had an invalid value.");
+    }
 
-    WebPageProxy* page = webPageProxyForHandle(handle);
-    if (!page)
-        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
+    std::optional<float> width;
+    std::optional<float> height;
+    if (optionalSizeObject) {
+        if (!(width = optionalSizeObject->getNumber<float>(WTF::ASCIILiteral("width"))))
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'width' parameter was not found or invalid.");
 
-    page->getWindowFrameWithCallback([callback = WTFMove(callback), page = makeRef(*page), width, height](WebCore::FloatRect originalFrame) mutable {
-        WebCore::FloatRect newFrame = WebCore::FloatRect(originalFrame.location(), WebCore::FloatSize(width, height));
-        if (newFrame == originalFrame)
-            return callback->sendSuccess();
+        if (!(height = optionalSizeObject->getNumber<float>(WTF::ASCIILiteral("height"))))
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'height' parameter was not found or invalid.");
 
-        page->setWindowFrame(newFrame);
-        callback->sendSuccess();
-    });
-#endif
-}
+        if (width.value() < 0)
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'width' parameter had an invalid value.");
 
-void WebAutomationSession::moveWindowOfBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const JSON::Object& positionObject, Ref<MoveWindowOfBrowsingContextCallback>&& callback)
-{
-#if PLATFORM(IOS)
-    FAIL_WITH_PREDEFINED_ERROR(NotImplemented);
-#else
-    float x;
-    if (!positionObject.getDouble(WTF::ASCIILiteral("x"), x))
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'x' parameter was not found or invalid.");
+        if (height.value() < 0)
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'height' parameter had an invalid value.");
+    }
 
-    float y;
-    if (!positionObject.getDouble(WTF::ASCIILiteral("y"), y))
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'y' parameter was not found or invalid.");
-
-    if (x < 0)
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'x' parameter had an invalid value.");
-
-    if (y < 0)
-        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'y' parameter had an invalid value.");
-
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
         FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
-    WebCore::FloatRect originalFrame;
-    page->getWindowFrameWithCallback([callback = WTFMove(callback), page = makeRef(*page), x, y](WebCore::FloatRect originalFrame) mutable {
+    // FIXME (§10.7.2 Step 10): We need to exit fullscreen before setting the frame.
+    // FIXME (§10.7.2 Step 11): We need to de-miniaturize the window before setting the frame.
 
-        WebCore::FloatRect newFrame = WebCore::FloatRect(WebCore::FloatPoint(x, y), originalFrame.size());
+    page->getWindowFrameWithCallback([callback = WTFMove(callback), page = makeRef(*page), width, height, x, y](WebCore::FloatRect originalFrame) mutable {
+        WebCore::FloatRect newFrame = WebCore::FloatRect(WebCore::FloatPoint(x.value_or(originalFrame.location().x()), y.value_or(originalFrame.location().y())), WebCore::FloatSize(width.value_or(originalFrame.size().width()), height.value_or(originalFrame.size().height())));
         if (newFrame == originalFrame)
             return callback->sendSuccess();
 

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h (228855 => 228856)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h	2018-02-21 03:27:09 UTC (rev 228855)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h	2018-02-21 04:13:42 UTC (rev 228856)
@@ -122,8 +122,7 @@
     void createBrowsingContext(Inspector::ErrorString&, String*) override;
     void closeBrowsingContext(Inspector::ErrorString&, const String&) override;
     void switchToBrowsingContext(Inspector::ErrorString&, const String& browsingContextHandle, const String* optionalFrameHandle) override;
-    void resizeWindowOfBrowsingContext(Inspector::ErrorString&, const String& handle, const JSON::Object& size, Ref<ResizeWindowOfBrowsingContextCallback>&&) final;
-    void moveWindowOfBrowsingContext(Inspector::ErrorString&, const String& handle, const JSON::Object& position, Ref<MoveWindowOfBrowsingContextCallback>&&) final;
+    void setWindowFrameOfBrowsingContext(Inspector::ErrorString&, const String& handle, const JSON::Object* origin, const JSON::Object* size, Ref<SetWindowFrameOfBrowsingContextCallback>&&) final;
     void navigateBrowsingContext(Inspector::ErrorString&, const String& handle, const String& url, const String* optionalPageLoadStrategyString, const int* optionalPageLoadTimeout, Ref<NavigateBrowsingContextCallback>&&) override;
     void goBackInBrowsingContext(Inspector::ErrorString&, const String&, const String* optionalPageLoadStrategyString, const int* optionalPageLoadTimeout, Ref<GoBackInBrowsingContextCallback>&&) override;
     void goForwardInBrowsingContext(Inspector::ErrorString&, const String&, const String* optionalPageLoadStrategyString, const int* optionalPageLoadTimeout, Ref<GoForwardInBrowsingContextCallback>&&) override;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to