Attachment 359557 Details for Bug 193598 – Patch

[patch] Patch

bug-193598-20190118165932.patch (text/plain), 8.50 KB, created by BJ Burg on 2019-01-18 16:59:32 PST

(hide)

Description:

Filename:

MIME Type:

Creator: BJ Burg

Created: 2019-01-18 16:59:32 PST

Size: 8.50 KB

patch

obsolete

>Subversion Revision: 240190
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c041dc2d1645f44974f0bd49b44573c0b63763d6..dbf4330b3549fefcbe45f1c3a8ddd5e5d0612ffb 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+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 NOBODY (OOPS!).
>+
>+        * 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>
> 
>         Revert r238815, it broke WK1 video fullscreen on Mac
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d771bef9cf6895523ecfdb0a2f9fb75cf7ea80e0..673f31a3a243d8cc999e43e012ea3473cfc4d2ef 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+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 NOBODY (OOPS!).
>+
>+        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>
> 
>         Fix some build issues.
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 06698156db4b5c210ee00868d1dc47fa8c07dc41..e4714aadf6c0d52a887e62e1e46926316b69cd9b 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -4883,6 +4883,13 @@ FloatPoint FrameView::layoutViewportToAbsolutePoint(FloatPoint p) const
>     return p.scaled(frame().frameScaleFactor());
> }
> 
>+FloatRect FrameView::clientToLayoutViewportRect(FloatRect rect) const
>+{
>+    ASSERT(frame().settings().visualViewportEnabled());
>+    rect.scale(frame().pageZoomFactor());
>+    return rect;
>+}
>+
> FloatPoint FrameView::clientToLayoutViewportPoint(FloatPoint p) const
> {
>     ASSERT(frame().settings().visualViewportEnabled());
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index 0c781ee62a3637f37ee67bac1e8361781fb955af..fd6995deebf55f13354c9c3194c19a61d413c6b1 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -486,7 +486,8 @@ public:
>     FloatPoint layoutViewportToAbsolutePoint(FloatPoint) const;
> 
>     // 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; }
> 
>diff --git a/Source/WebKit/Shared/CoordinateSystem.h b/Source/WebKit/Shared/CoordinateSystem.h
>index 52b888172e07aa5ca6078b0982b88fb4b2ea3687..4b55cf6b234410430936f7af686b55869a8e823e 100644
>--- a/Source/WebKit/Shared/CoordinateSystem.h
>+++ b/Source/WebKit/Shared/CoordinateSystem.h
>@@ -31,8 +31,7 @@ namespace WebKit {
> 
> enum class CoordinateSystem : uint8_t {
>     Page = 0,
>-    LayoutViewport,
>-    VisualViewport,
>+    LayoutViewport
> };
> 
> } // namespace WebKit
>@@ -42,9 +41,7 @@ namespace WTF {
> template<> struct EnumTraits<WebKit::CoordinateSystem> {
>     using values = EnumValues<
>     WebKit::CoordinateSystem,
>-    WebKit::CoordinateSystem::Page,
>-    WebKit::CoordinateSystem::LayoutViewport,
>-    WebKit::CoordinateSystem::VisualViewport
>+    WebKit::CoordinateSystem::Page
>     >;
> };
> 
>diff --git a/Source/WebKit/UIProcess/Automation/Automation.json b/Source/WebKit/UIProcess/Automation/Automation.json
>index 671a761ae3637b58158fa10f1f4afd1b2a5bd36e..7f74172d3e6bb97ff1107f8e7ee042471a5ad516 100644
>--- a/Source/WebKit/UIProcess/Automation/Automation.json
>+++ b/Source/WebKit/UIProcess/Automation/Automation.json
>@@ -32,8 +32,7 @@
>             "description": "The coordinate system in which rects, points, and sizes are to be interpreted.",
>             "enum": [
>                 "Page",
>-                "LayoutViewport",
>-                "VisualViewport"
>+                "LayoutViewport"
>             ]
>         },
>         {
>diff --git a/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp b/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>index 6c4edebb1729f7cefb48922a9565b0e7ea5b0610..8995dc6125f459390c180eeb543c11516b1cee41 100644
>--- a/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>+++ b/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>@@ -1004,8 +1004,6 @@ static Optional<CoordinateSystem> protocolStringToCoordinateSystem(const String&
>         return CoordinateSystem::Page;
>     if (coordinateSystemString == "LayoutViewport")
>         return CoordinateSystem::LayoutViewport;
>-    if (coordinateSystemString == "VisualViewport")
>-        return CoordinateSystem::VisualViewport;
>     return WTF::nullopt;
> }
> 
>diff --git a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>index 009ccec0c230e2fdb6a18fcdbe5337ab02e8cece..39461adb83a0e4d5e3f7dfdff735739054e43984 100644
>--- a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>+++ b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>@@ -567,11 +567,6 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
>         // FIXME: Wait in an implementation-specific way up to the session implicit wait timeout for the element to become in view.
>     }
> 
>-    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();
>     WebCore::IntRect frameElementBounds = roundedIntRect(coreElement->boundingClientRect());
>@@ -583,10 +578,10 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
>         break;
>     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;
>     }
> 
>@@ -602,10 +597,10 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
>                 break;
>             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;
>             }
>         }

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193598: 359557