Changeset 240195 in webkit

Timestamp:

Jan 18, 2019 8:22:41 PM (5 years ago)

Author:

BJ Burg

Message:

Automation.computeElementLayout should return visual viewport-aware coordinates
​https://bugs.webkit.org/show_bug.cgi?id=193598
<rdar://problem/35325644>

Reviewed by Simon Fraser.

Source/WebCore:

• page/FrameView.h: export symbol to be usable from WebKit.
• page/FrameView.cpp:

(WebCore::FrameView::clientToLayoutViewportRect const): Added.
Do the same thing as clientToLayoutViewportPoint with a rect instead.

Source/WebKit:

Previously I added CoordinateSystem::VisualViewport to stub out this for iOS.
But I think that it's a mistake for safaridriver to care about VisualViewport
being enabled or not, because it is a runtime-switchable setting.

This patch removes CoordinateSystem::VisualViewport. Make the existing
CoordinateSystem::LayoutViewport use visual viewport semantics if needed.

This is tested by WebDriver element clicking tests. There should not be any
difference in behavior until it is possible to zoom with gestures via WebDriver.

• Shared/CoordinateSystem.h:
• UIProcess/Automation/Automation.json:
• UIProcess/Automation/WebAutomationSession.cpp:

(WebKit::protocolStringToCoordinateSystem):

• WebProcess/Automation/WebAutomationSessionProxy.cpp:

(WebKit::WebAutomationSessionProxy::computeElementLayout):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (1 diff)
• WebCore/page/FrameView.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/Shared/CoordinateSystem.h (modified) (2 diffs)
• WebKit/UIProcess/Automation/Automation.json (modified) (1 diff)
• WebKit/UIProcess/Automation/WebAutomationSession.cpp (modified) (1 diff)
• WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-18  Brian Burg  <bburg@apple.com>
+
+        Automation.computeElementLayout should return visual viewport-aware coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=193598
+        <rdar://problem/35325644>
+
+        Reviewed by Simon Fraser.
+
+        * page/FrameView.h: export symbol to be usable from WebKit.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::clientToLayoutViewportRect const): Added.
+        Do the same thing as clientToLayoutViewportPoint with a rect instead.
+
+
 2019-01-18  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -4884 +4884 @@
 }
 
+FloatRect FrameView::clientToLayoutViewportRect(FloatRect rect) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    rect.scale(frame().pageZoomFactor());
+    return rect;
+}
+
 FloatPoint FrameView::clientToLayoutViewportPoint(FloatPoint p) const
 {

trunk/Source/WebCore/page/FrameView.h

@@ -487 +487 @@
 
     // Unlike client coordinates, layout viewport coordinates are affected by page zoom.
-    FloatPoint clientToLayoutViewportPoint(FloatPoint) const;
+    WEBCORE_EXPORT FloatRect clientToLayoutViewportRect(FloatRect) const;
+    WEBCORE_EXPORT FloatPoint clientToLayoutViewportPoint(FloatPoint) const;
 
     bool isFrameViewScrollCorner(const RenderScrollbarPart& scrollCorner) const { return m_scrollCorner.get() == &scrollCorner; }

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-01-18  Brian Burg  <bburg@apple.com>
+
+        Automation.computeElementLayout should return visual viewport-aware coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=193598
+        <rdar://problem/35325644>
+
+        Reviewed by Simon Fraser.
+
+        Previously I added CoordinateSystem::VisualViewport to stub out this for iOS.
+        But I think that it's a mistake for safaridriver to care about VisualViewport
+        being enabled or not, because it is a runtime-switchable setting.
+
+        This patch removes CoordinateSystem::VisualViewport. Make the existing
+        CoordinateSystem::LayoutViewport use visual viewport semantics if needed.
+
+        This is tested by WebDriver element clicking tests. There should not be any
+        difference in behavior until it is possible to zoom with gestures via WebDriver.
+
+        * Shared/CoordinateSystem.h:
+        * UIProcess/Automation/Automation.json:
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::protocolStringToCoordinateSystem):
+        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+        (WebKit::WebAutomationSessionProxy::computeElementLayout):
+
 2019-01-18  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebKit/Shared/CoordinateSystem.h

@@ -32 +32 @@
 enum class CoordinateSystem : uint8_t {
     Page = 0,
-    LayoutViewport,
-    VisualViewport,
+    LayoutViewport
 };
 
@@ -43 +42 @@
     using values = EnumValues<
     WebKit::CoordinateSystem,
-    WebKit::CoordinateSystem::Page,
-    WebKit::CoordinateSystem::LayoutViewport,
-    WebKit::CoordinateSystem::VisualViewport
+    WebKit::CoordinateSystem::Page
     >;
 };

trunk/Source/WebKit/UIProcess/Automation/Automation.json

@@ -33 +33 @@
             "enum": [
                 "Page",
-                "LayoutViewport",
-                "VisualViewport"
+                "LayoutViewport"
             ]
         },

trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp

@@ -1005 +1005 @@
     if (coordinateSystemString == "LayoutViewport")
         return CoordinateSystem::LayoutViewport;
-    if (coordinateSystemString == "VisualViewport")
-        return CoordinateSystem::VisualViewport;
     return WTF::nullopt;
 }

trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp

@@ -568 +568 @@
     }
 
-    if (coordinateSystem == CoordinateSystem::VisualViewport) {
-        WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidComputeElementLayout(callbackID, { }, WTF::nullopt, false, notImplementedErrorType), 0);
-        return;
-    }
-
     WebCore::FrameView* frameView = frame->coreFrame()->view();
     WebCore::FrameView* mainView = frame->coreFrame()->mainFrame().view();
@@ -584 +579 @@
     case CoordinateSystem::LayoutViewport:
         // The element bounds are already in client coordinates.
-        resultElementBounds = rootElementBounds;
-        break;
-    case CoordinateSystem::VisualViewport:
-        ASSERT_NOT_REACHED();
+        if (frame->coreFrame()->settings().visualViewportEnabled())
+            resultElementBounds = WebCore::IntRect(mainView->clientToLayoutViewportRect(WebCore::FloatRect(rootElementBounds)));
+        else
+            resultElementBounds = rootElementBounds;
         break;
     }
@@ -603 +598 @@
             case CoordinateSystem::LayoutViewport:
                 // The point is already in client coordinates.
-                resultInViewCenterPoint = rootInViewCenterPoint;
-                break;
-            case CoordinateSystem::VisualViewport:
-                ASSERT_NOT_REACHED();
+                if (frame->coreFrame()->settings().visualViewportEnabled())
+                    resultInViewCenterPoint = WebCore::IntPoint(mainView->clientToLayoutViewportPoint(rootInViewCenterPoint));
+                else
+                    resultInViewCenterPoint = rootInViewCenterPoint;
                 break;
             }