Merge r225367 - Web Automation: computeElementLayout does not correct…

…ly translate iframe client coordinates to main frame coordinates

https://bugs.webkit.org/show_bug.cgi?id=180213
<rdar://problem/30260141>

Reviewed by Simon Fraser.

The current implementation computes points in terms of the frame in which the element is located.
However, WebDriver expects coordinates to be relative to the top-level document since
these coordinates are used for generating click events, among other things.

To convert from frame client coordinates to main frame client coordinates, round-trip
both inViewCenterPoint and elementBounds to root view coordinates and back
to the main frame's contents/client coordinates. Then convert this to page coordinates if needed.

This progresses several tests in the Selenium Python test suite:

 - event_firing_webdriver_tests.py::test_should_fire_navigation_events
 - frame_switching_tests.py::testShouldBeAbleToClickInAFrameThatRewritesTopWindowLocation
 - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUs
 - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithFrameIndex
 - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithWebelement
 - frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs
 - position_and_size_tests.py::testShouldGetCoordinatesOfAnInvisibleElement

* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::computeElementLayout):
Get both the frame and main frame FrameViews and convert coordinates to the root view.
This is somewhat lossy as clientToDocument* deals with FloatPoints but contentsToRootView
deals with IntPoints. For the purposes of WebDriver, lossiness is not a problem since
integer values are expected anyway.

The imperative nature of the coordinate calculations is difficult to debug, so I converted
this function to only assign to each variable once.

Source/WebKit/ChangeLog

@@ -1,3 +1,39 @@
+2017-11-30  Brian Burg  <bburg@apple.com>
+
+        Web Automation: computeElementLayout does not correctly translate iframe client coordinates to main frame coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=180213
+        <rdar://problem/30260141>
+
+        Reviewed by Simon Fraser.
+
+        The current implementation computes points in terms of the frame in which the element is located.
+        However, WebDriver expects coordinates to be relative to the top-level document since
+        these coordinates are used for generating click events, among other things.
+
+        To convert from frame client coordinates to main frame client coordinates, round-trip
+        both inViewCenterPoint and elementBounds to root view coordinates and back
+        to the main frame's contents/client coordinates. Then convert this to page coordinates if needed.
+
+        This progresses several tests in the Selenium Python test suite:
+
+         - event_firing_webdriver_tests.py::test_should_fire_navigation_events
+         - frame_switching_tests.py::testShouldBeAbleToClickInAFrameThatRewritesTopWindowLocation
+         - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUs
+         - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithFrameIndex
+         - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithWebelement
+         - frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs
+         - position_and_size_tests.py::testShouldGetCoordinatesOfAnInvisibleElement
+
+        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+        (WebKit::WebAutomationSessionProxy::computeElementLayout):
+        Get both the frame and main frame FrameViews and convert coordinates to the root view.
+        This is somewhat lossy as clientToDocument* deals with FloatPoints but contentsToRootView
+        deals with IntPoints. For the purposes of WebDriver, lossiness is not a problem since
+        integer values are expected anyway.
+
+        The imperative nature of the coordinate calculations is difficult to debug, so I converted
+        this function to only assign to each variable once.
+
 2017-11-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Move JSONValues to WTF and convert uses of InspectorValues.h to JSONValues.h

Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp

@@ -572,32 +572,37 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
         return;
     }
 
-    WebCore::FloatRect elementBounds = coreElement->boundingClientRect();
-
-    auto* coreFrameView = frame->coreFrame()->view();
+    WebCore::FrameView* frameView = frame->coreFrame()->view();
+    WebCore::FrameView* mainView = frame->coreFrame()->mainFrame().view();
+    WebCore::IntRect frameElementBounds = roundedIntRect(coreElement->boundingClientRect());
+    WebCore::IntRect rootElementBounds = mainView->rootViewToContents(frameView->contentsToRootView(frameElementBounds));
+    WebCore::IntRect resultElementBounds;
     switch (coordinateSystem) {
     case CoordinateSystem::Page:
-        elementBounds = coreFrameView->clientToDocumentRect(elementBounds);
+        resultElementBounds = WebCore::IntRect(mainView->clientToDocumentRect(WebCore::FloatRect(rootElementBounds)));
         break;
     case CoordinateSystem::LayoutViewport:
         // The element bounds are already in client coordinates.
+        resultElementBounds = rootElementBounds;
         break;
     case CoordinateSystem::VisualViewport:
         ASSERT_NOT_REACHED();
         break;
     }
 
-    std::optional<WebCore::FloatPoint> inViewCenterPoint;
+    std::optional<WebCore::IntPoint> resultInViewCenterPoint;
     bool isObscured = false;
     if (containerElement) {
-        inViewCenterPoint = elementInViewClientCenterPoint(*containerElement, isObscured);
-        if (inViewCenterPoint.has_value()) {
+        std::optional<WebCore::FloatPoint> frameInViewCenterPoint = elementInViewClientCenterPoint(*containerElement, isObscured);
+        if (frameInViewCenterPoint.has_value()) {
+            WebCore::IntPoint rootInViewCenterPoint = mainView->rootViewToContents(frameView->contentsToRootView(WebCore::IntPoint(frameInViewCenterPoint.value())));
             switch (coordinateSystem) {
             case CoordinateSystem::Page:
-                inViewCenterPoint = coreFrameView->clientToDocumentPoint(inViewCenterPoint.value());
+                resultInViewCenterPoint = WebCore::IntPoint(mainView->clientToDocumentPoint(rootInViewCenterPoint));
                 break;
             case CoordinateSystem::LayoutViewport:
                 // The point is already in client coordinates.
+                resultInViewCenterPoint = rootInViewCenterPoint;
                 break;
             case CoordinateSystem::VisualViewport:
                 ASSERT_NOT_REACHED();
@@ -606,7 +611,7 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
         }
     }
 
-    WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidComputeElementLayout(callbackID, enclosingIntRect(elementBounds), WebCore::IntPoint(inViewCenterPoint.value()), isObscured, String()), 0);
+    WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidComputeElementLayout(callbackID, resultElementBounds, resultInViewCenterPoint.value(), isObscured, String()), 0);
 }
 
 void WebAutomationSessionProxy::selectOptionElement(uint64_t pageID, uint64_t frameID, String nodeHandle, uint64_t callbackID)