Skip to content
Permalink
Browse files
Regression (r244291): Broken API Test AutoLayoutRenderingProgressRela…
…tiveOrdering

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).


Canonical link: https://commits.webkit.org/211316@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244434 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
alanbujtas committed Apr 18, 2019
1 parent 6039bd7 commit 7e4ba8570e5a5da808d0b8a55e0366ab72632d32
Showing 18 changed files with 102 additions and 44 deletions.
@@ -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
@@ -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();
@@ -401,6 +401,7 @@ class FrameView final : public ScrollView {
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);
@@ -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.
@@ -143,7 +143,6 @@ class DrawingAreaProxy : public IPC::MessageReceiver, protected IPC::MessageSend
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;
@@ -31,6 +31,5 @@ messages -> DrawingAreaProxy {
#if PLATFORM(COCOA)
// Used by TiledCoreAnimationDrawingAreaProxy.
DidUpdateGeometry()
IntrinsicContentSizeDidChange(WebCore::IntSize newIntrinsicContentSize)
#endif
}
@@ -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
}

@@ -789,7 +789,6 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
NSView *inspectorAttachmentView();
_WKRemoteObjectRegistry *remoteObjectRegistry();

void intrinsicContentSizeDidChange(const WebCore::IntSize& intrinsicContentSize);
CGRect boundsOfLayerInLayerBackedWindowCoordinates(CALayer *) const;
#endif // PLATFORM(MAC)

@@ -1714,6 +1713,7 @@ WEBPAGEPROXY_LOADOPTIMIZER_ADDITIONS_1
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>&&);
@@ -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);
@@ -60,7 +60,6 @@ class TiledCoreAnimationDrawingAreaProxy : public DrawingAreaProxy {

// Message handlers.
void didUpdateGeometry() override;
void intrinsicContentSizeDidChange(const WebCore::IntSize&) override;

void sendUpdateGeometry();

@@ -125,12 +125,6 @@
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();
@@ -433,11 +433,6 @@ static inline bool expectsLegacyImplicitRubberBandControl()
return result;
}

void WebPageProxy::intrinsicContentSizeDidChange(const IntSize& intrinsicContentSize)
{
pageClient().intrinsicContentSizeDidChange(intrinsicContentSize);
}

void WebPageProxy::setRemoteLayerTreeRootNode(RemoteLayerTreeNode* rootNode)
{
pageClient().setRemoteLayerTreeRootNode(rootNode);
@@ -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));
}
@@ -1555,6 +1555,8 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP

bool shouldDispatchUpdateAfterFocusingElement(const WebCore::Element&) const;

void updateIntrinsicContentSizeIfNeeded();

uint64_t m_pageID;

std::unique_ptr<WebCore::Page> m_page;
@@ -1894,6 +1896,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
#if PLATFORM(COCOA)
WeakPtr<RemoteObjectRegistry> m_remoteObjectRegistry;
#endif
WebCore::IntSize m_lastSentIntrinsicContentSize;
};

} // namespace WebKit
@@ -121,7 +121,6 @@ class TiledCoreAnimationDrawingArea : public DrawingArea, public CanMakeWeakPtr<
WebCore::TiledBacking* mainFrameTiledBacking() const;
void updateDebugInfoLayer(bool showLayer);

void updateIntrinsicContentSizeIfNeeded();
void updateScrolledExposedRect();
void scaleViewToFitDocumentIfNeeded();

@@ -150,8 +149,6 @@ class TiledCoreAnimationDrawingArea : public DrawingArea, public CanMakeWeakPtr<
WebCore::IntSize m_lastViewSizeForScaleToFit;
WebCore::IntSize m_lastDocumentSizeForScaleToFit;

WebCore::IntSize m_lastSentIntrinsicContentSize;

double m_transientZoomScale { 1 };
WebCore::FloatPoint m_transientZoomOrigin;

@@ -278,26 +278,6 @@ + (void)synchronize;
{
}

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)synchronize;
m_webPage.updateRendering();
m_webPage.flushPendingEditorStateUpdate();

updateIntrinsicContentSizeIfNeeded();

if (m_pendingRootLayer) {
setRootCompositingLayer(m_pendingRootLayer.get());
m_pendingRootLayer = nullptr;
@@ -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.
@@ -174,7 +174,7 @@ - (void)invalidateIntrinsicContentSize
[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 @@ - (void)invalidateIntrinsicContentSize
[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];

0 comments on commit 7e4ba85

Please sign in to comment.