Regression (r244291): Broken API Test AutoLayoutRenderingProgressRelativeOrdering
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Apr 2019 21:12:59 +0000 (21:12 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Apr 2019 21:12:59 +0000 (21:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196948
<rdar://problem/49927131>

Reviewed by Tim Horton.

Source/WebCore:

* page/FrameView.cpp:
(WebCore::FrameView::setContentsSize):
(WebCore::FrameView::autoSizeIfEnabled):
* page/FrameView.h:

Source/WebKit:

Move intrinsicContentSizeDidChange out of DrawingArea. Intrinsic content size is a layout concept and
after r244291 there's no reason to have it in DrawingArea.

* UIProcess/DrawingAreaProxy.h:
(WebKit::DrawingAreaProxy::didUpdateGeometry):
(WebKit::DrawingAreaProxy::intrinsicContentSizeDidChange): Deleted.
* UIProcess/DrawingAreaProxy.messages.in:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didChangeIntrinsicContentSize):
(WebKit::WebPageProxy::setViewLayoutSize):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h:
* UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
(WebKit::TiledCoreAnimationDrawingAreaProxy::intrinsicContentSizeDidChange): Deleted.
* UIProcess/mac/WebPageProxyMac.mm:
(WebKit::WebPageProxy::intrinsicContentSizeDidChange): Deleted.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::updateIntrinsicContentSizeIfNeeded):
(WebKit::WebPage::dispatchDidReachLayoutMilestone):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::flushLayers):
(WebKit::TiledCoreAnimationDrawingArea::updateIntrinsicContentSizeIfNeeded): Deleted.

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm:
(TEST):
The expected order of incoming events is
1. didInvalidateIntrinsicContentSize
2. didFirstLayout
At setRenderingProgressDidChange, we already check if didInvalidateIntrinsicContentSize comes in first.
However it's not guaranteed that the milestone event is delayed until after TestWebKitAPI::Util::run() is finished
(and remember, all we care about is ordering).

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244434 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/DrawingAreaProxy.h
Source/WebKit/UIProcess/DrawingAreaProxy.messages.in
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h
Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm
Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm

index 1428662..504afa3 100644 (file)
@@ -1,3 +1,16 @@
+2019-04-18  Zalan Bujtas  <zalan@apple.com>
+
+        Regression (r244291): Broken API Test AutoLayoutRenderingProgressRelativeOrdering
+        https://bugs.webkit.org/show_bug.cgi?id=196948
+        <rdar://problem/49927131>
+
+        Reviewed by Tim Horton.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setContentsSize):
+        (WebCore::FrameView::autoSizeIfEnabled):
+        * page/FrameView.h:
+
 2019-04-18  Shawn Roberts  <sroberts@apple.com>
 
         Unreviewed manual rollout of r244248 and r244409
index 8ecef07..6789a3d 100644 (file)
@@ -631,6 +631,8 @@ void FrameView::setContentsSize(const IntSize& size)
         PageCache::singleton().markPagesForContentsSizeChanged(*page);
     }
     layoutContext().enableSetNeedsLayout();
+    if (m_shouldAutoSize)
+        m_autoSizeContentSize = size; 
 }
 
 void FrameView::adjustViewSize()
@@ -3463,7 +3465,6 @@ void FrameView::autoSizeIfEnabled()
     resize(m_autoSizeConstraint.width(), m_autoSizeConstraint.height());
     document->updateStyleIfNeeded();
     document->updateLayoutIgnorePendingStylesheets();
-    m_autoSizeContentSize = contentsSize();
 
     auto finalWidth = std::max(m_autoSizeConstraint.width(), m_autoSizeContentSize.width());
     auto finalHeight = m_autoSizeFixedMinimumHeight ? std::max(m_autoSizeFixedMinimumHeight, m_autoSizeContentSize.height()) : m_autoSizeContentSize.height();
@@ -4470,6 +4471,7 @@ void FrameView::enableAutoSizeMode(bool enable, const IntSize& viewSize)
 
     m_shouldAutoSize = enable;
     m_autoSizeConstraint = viewSize;
+    m_autoSizeContentSize = contentsSize();
     m_didRunAutosize = false;
 
     setNeedsLayoutAfterViewConfigurationChange();
index 253db2a..81d1cc6 100644 (file)
@@ -401,6 +401,7 @@ public:
     bool isVisuallyNonEmpty() const { return m_isVisuallyNonEmpty; }
     WEBCORE_EXPORT void enableAutoSizeMode(bool enable, const IntSize& minSize);
     WEBCORE_EXPORT void setAutoSizeFixedMinimumHeight(int);
+    bool isAutoSizeEnabled() const { return m_shouldAutoSize; }
     IntSize autoSizingIntrinsicContentSize() const { return m_autoSizeContentSize; }
 
     WEBCORE_EXPORT void forceLayout(bool allowSubtreeLayout = false);
index 1eb5986..c0494a9 100644 (file)
@@ -1,3 +1,37 @@
+2019-04-18  Zalan Bujtas  <zalan@apple.com>
+
+        Regression (r244291): Broken API Test AutoLayoutRenderingProgressRelativeOrdering
+        https://bugs.webkit.org/show_bug.cgi?id=196948
+        <rdar://problem/49927131>
+
+        Reviewed by Tim Horton.
+
+        Move intrinsicContentSizeDidChange out of DrawingArea. Intrinsic content size is a layout concept and
+        after r244291 there's no reason to have it in DrawingArea.
+
+        * UIProcess/DrawingAreaProxy.h:
+        (WebKit::DrawingAreaProxy::didUpdateGeometry):
+        (WebKit::DrawingAreaProxy::intrinsicContentSizeDidChange): Deleted.
+        * UIProcess/DrawingAreaProxy.messages.in:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didChangeIntrinsicContentSize):
+        (WebKit::WebPageProxy::setViewLayoutSize):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h:
+        * UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
+        (WebKit::TiledCoreAnimationDrawingAreaProxy::intrinsicContentSizeDidChange): Deleted.
+        * UIProcess/mac/WebPageProxyMac.mm:
+        (WebKit::WebPageProxy::intrinsicContentSizeDidChange): Deleted.
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::updateIntrinsicContentSizeIfNeeded):
+        (WebKit::WebPage::dispatchDidReachLayoutMilestone):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::flushLayers):
+        (WebKit::TiledCoreAnimationDrawingArea::updateIntrinsicContentSizeIfNeeded): Deleted.
+
 2019-04-18  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r244299.
index b6905b0..46e1697 100644 (file)
@@ -143,7 +143,6 @@ private:
     virtual void updateAcceleratedCompositingMode(uint64_t /* backingStoreStateID */, const LayerTreeContext&) { }
 #if PLATFORM(COCOA)
     virtual void didUpdateGeometry() { }
-    virtual void intrinsicContentSizeDidChange(const WebCore::IntSize&) { }
 
 #if PLATFORM(MAC)
     RunLoop::Timer<DrawingAreaProxy> m_viewExposedRectChangedTimer;
index 3e58de5..bcf2fb9 100644 (file)
@@ -31,6 +31,5 @@ messages -> DrawingAreaProxy {
 #if PLATFORM(COCOA)
     // Used by TiledCoreAnimationDrawingAreaProxy.
     DidUpdateGeometry()
-    IntrinsicContentSizeDidChange(WebCore::IntSize newIntrinsicContentSize)
 #endif
 }
index 915a029..bbf043f 100644 (file)
@@ -5464,6 +5464,13 @@ void WebPageProxy::didChangeContentSize(const IntSize& size)
     pageClient().didChangeContentSize(size);
 }
 
+void WebPageProxy::didChangeIntrinsicContentSize(const IntSize& intrinsicContentSize)
+{
+#if USE(APPKIT)
+    pageClient().intrinsicContentSizeDidChange(intrinsicContentSize);
+#endif
+}
+
 #if ENABLE(INPUT_TYPE_COLOR)
 void WebPageProxy::showColorPicker(const WebCore::Color& initialColor, const IntRect& elementRect, Vector<WebCore::Color>&& suggestions)
 {
@@ -7596,7 +7603,7 @@ void WebPageProxy::setViewLayoutSize(const IntSize& viewLayoutSize)
 
 #if USE(APPKIT)
     if (m_viewLayoutSize.width() <= 0)
-        intrinsicContentSizeDidChange(IntSize(-1, -1));
+        didChangeIntrinsicContentSize(IntSize(-1, -1));
 #endif
 }
 
index b2480fa..0845d15 100644 (file)
@@ -789,7 +789,6 @@ public:
     NSView *inspectorAttachmentView();
     _WKRemoteObjectRegistry *remoteObjectRegistry();
 
-    void intrinsicContentSizeDidChange(const WebCore::IntSize& intrinsicContentSize);
     CGRect boundsOfLayerInLayerBackedWindowCoordinates(CALayer *) const;
 #endif // PLATFORM(MAC)
 
@@ -1714,6 +1713,7 @@ private:
     void didDestroyNotification(uint64_t notificationID);
 
     void didChangeContentSize(const WebCore::IntSize&);
+    void didChangeIntrinsicContentSize(const WebCore::IntSize&);
 
 #if ENABLE(INPUT_TYPE_COLOR)
     void showColorPicker(const WebCore::Color& initialColor, const WebCore::IntRect&, Vector<WebCore::Color>&&);
index 1d44c6a..e74c5fd 100644 (file)
@@ -88,6 +88,7 @@ messages -> WebPageProxy {
     SetCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents)
 
     DidChangeContentSize(WebCore::IntSize newSize)
+    DidChangeIntrinsicContentSize(WebCore::IntSize newIntrinsicContentSize)
 
 #if ENABLE(INPUT_TYPE_COLOR)
     ShowColorPicker(WebCore::Color initialColor, WebCore::IntRect elementRect, Vector<WebCore::Color> suggestions);
index f2f2702..44200c2 100644 (file)
@@ -60,7 +60,6 @@ private:
 
     // Message handlers.
     void didUpdateGeometry() override;
-    void intrinsicContentSizeDidChange(const WebCore::IntSize&) override;
 
     void sendUpdateGeometry();
 
index 0739fdc..f70f1a4 100644 (file)
@@ -125,12 +125,6 @@ void TiledCoreAnimationDrawingAreaProxy::waitForDidUpdateActivityState(ActivityS
     process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DidUpdateActivityState>(m_webPageProxy.pageID(), activityStateUpdateTimeout, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
 }
 
-void TiledCoreAnimationDrawingAreaProxy::intrinsicContentSizeDidChange(const IntSize& newIntrinsicContentSize)
-{
-    if (m_webPageProxy.viewLayoutSize().width() > 0)
-        m_webPageProxy.intrinsicContentSizeDidChange(newIntrinsicContentSize);
-}
-
 void TiledCoreAnimationDrawingAreaProxy::willSendUpdateGeometry()
 {
     m_lastSentViewLayoutSize = m_webPageProxy.viewLayoutSize();
index 74f6a69..138d132 100644 (file)
@@ -433,11 +433,6 @@ bool WebPageProxy::acceptsFirstMouse(int eventNumber, const WebKit::WebMouseEven
     return result;
 }
 
-void WebPageProxy::intrinsicContentSizeDidChange(const IntSize& intrinsicContentSize)
-{
-    pageClient().intrinsicContentSizeDidChange(intrinsicContentSize);
-}
-
 void WebPageProxy::setRemoteLayerTreeRootNode(RemoteLayerTreeNode* rootNode)
 {
     pageClient().setRemoteLayerTreeRootNode(rootNode);
index 547c965..fbd1553 100644 (file)
@@ -6229,6 +6229,24 @@ void WebPage::removeAllUserContent()
     m_userContentController->removeAllUserContent();
 }
 
+void WebPage::updateIntrinsicContentSizeIfNeeded()
+{
+    if (!viewLayoutSize().width())
+        return;
+
+    ASSERT(mainFrameView());
+    if (!mainFrameView()->isAutoSizeEnabled())
+        return; 
+
+    ASSERT(!mainFrameView()->needsLayout());
+    auto contentSize = mainFrameView()->autoSizingIntrinsicContentSize();
+    if (m_lastSentIntrinsicContentSize == contentSize)
+        return;
+
+    m_lastSentIntrinsicContentSize = contentSize;
+    send(Messages::WebPageProxy::DidChangeIntrinsicContentSize(contentSize));
+}
+
 void WebPage::dispatchDidReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone> milestones)
 {
     RefPtr<API::Object> userData;
@@ -6244,6 +6262,8 @@ void WebPage::dispatchDidReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone
         if (drawingAreaRelatedMilestones && m_drawingArea->addMilestonesToDispatch(drawingAreaRelatedMilestones))
             milestones.remove(drawingAreaRelatedMilestones);
     }
+    if (milestones.contains(DidFirstLayout))
+        updateIntrinsicContentSizeIfNeeded();
 
     send(Messages::WebPageProxy::DidReachLayoutMilestone(milestones));
 }
index 2acc328..7c6cc1e 100644 (file)
@@ -1555,6 +1555,8 @@ private:
 
     bool shouldDispatchUpdateAfterFocusingElement(const WebCore::Element&) const;
 
+    void updateIntrinsicContentSizeIfNeeded();
+
     uint64_t m_pageID;
 
     std::unique_ptr<WebCore::Page> m_page;
@@ -1894,6 +1896,7 @@ private:
 #if PLATFORM(COCOA)
     WeakPtr<RemoteObjectRegistry> m_remoteObjectRegistry;
 #endif
+    WebCore::IntSize m_lastSentIntrinsicContentSize;
 };
 
 } // namespace WebKit
index 9af1e7f..1d8aeee 100644 (file)
@@ -121,7 +121,6 @@ private:
     WebCore::TiledBacking* mainFrameTiledBacking() const;
     void updateDebugInfoLayer(bool showLayer);
 
-    void updateIntrinsicContentSizeIfNeeded();
     void updateScrolledExposedRect();
     void scaleViewToFitDocumentIfNeeded();
 
@@ -150,8 +149,6 @@ private:
     WebCore::IntSize m_lastViewSizeForScaleToFit;
     WebCore::IntSize m_lastDocumentSizeForScaleToFit;
 
-    WebCore::IntSize m_lastSentIntrinsicContentSize;
-
     double m_transientZoomScale { 1 };
     WebCore::FloatPoint m_transientZoomOrigin;
 
index 85f0ca2..8095e67 100644 (file)
@@ -278,26 +278,6 @@ void TiledCoreAnimationDrawingArea::mainFrameContentSizeChanged(const IntSize& s
 {
 }
 
-void TiledCoreAnimationDrawingArea::updateIntrinsicContentSizeIfNeeded()
-{
-    if (!m_webPage.viewLayoutSize().width())
-        return;
-
-    FrameView* frameView = m_webPage.mainFrameView();
-    if (!frameView)
-        return;
-
-    if (frameView->needsLayout())
-        return;
-
-    IntSize contentSize = frameView->autoSizingIntrinsicContentSize();
-    if (m_lastSentIntrinsicContentSize == contentSize)
-        return;
-
-    m_lastSentIntrinsicContentSize = contentSize;
-    send(Messages::DrawingAreaProxy::IntrinsicContentSizeDidChange(contentSize));
-}
-
 void TiledCoreAnimationDrawingArea::setShouldScaleViewToFitDocument(bool shouldScaleView)
 {
     if (m_shouldScaleViewToFitDocument == shouldScaleView)
@@ -462,8 +442,6 @@ void TiledCoreAnimationDrawingArea::flushLayers(FlushType flushType)
         m_webPage.updateRendering();
         m_webPage.flushPendingEditorStateUpdate();
 
-        updateIntrinsicContentSizeIfNeeded();
-
         if (m_pendingRootLayer) {
             setRootCompositingLayer(m_pendingRootLayer.get());
             m_pendingRootLayer = nullptr;
index d95f74a..eb1ff5e 100644 (file)
@@ -1,3 +1,20 @@
+2019-04-18  Zalan Bujtas  <zalan@apple.com>
+
+        Regression (r244291): Broken API Test AutoLayoutRenderingProgressRelativeOrdering
+        https://bugs.webkit.org/show_bug.cgi?id=196948
+        <rdar://problem/49927131>
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm:
+        (TEST): 
+        The expected order of incoming events is
+        1. didInvalidateIntrinsicContentSize
+        2. didFirstLayout
+        At setRenderingProgressDidChange, we already check if didInvalidateIntrinsicContentSize comes in first.
+        However it's not guaranteed that the milestone event is delayed until after TestWebKitAPI::Util::run() is finished
+        (and remember, all we care about is ordering).
+
 2019-04-18  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r244299.
index a5d8c42..e0b1f93 100644 (file)
@@ -174,7 +174,7 @@ TEST(WebKit, AutoLayoutIntegration)
     [webView _setShouldExpandContentToViewHeightForAutoLayout:NO];
 }
 
-TEST(WebKit, DISABLED_AutoLayoutRenderingProgressRelativeOrdering)
+TEST(WebKit, AutoLayoutRenderingProgressRelativeOrdering)
 {
     RetainPtr<AutoLayoutWKWebView> webView = adoptNS([[AutoLayoutWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 1000, 1000)]);
 
@@ -199,7 +199,6 @@ TEST(WebKit, DISABLED_AutoLayoutRenderingProgressRelativeOrdering)
     [webView setExpectedIntrinsicContentSize:NSMakeSize(100, 400)];
     [webView loadHTMLString:@"<body style='margin: 0; height: 400px;'></body>" baseURL:nil];
     TestWebKitAPI::Util::run(&didInvalidateIntrinsicContentSize);
-    EXPECT_FALSE(didFirstLayout);
     TestWebKitAPI::Util::run(&didFirstLayout);
     TestWebKitAPI::Util::run(&didFinishNavigation);
     [webView setNavigationDelegate:nil];