Title: [240912] trunk
Revision
240912
Author
[email protected]
Date
2019-02-03 17:48:13 -0800 (Sun, 03 Feb 2019)

Log Message

Make setNeedsLayout on the root more explicitly about triggering its side-effects
https://bugs.webkit.org/show_bug.cgi?id=194198

Reviewed by Antti Koivisto.

Source/WebCore:

Calling setNeedsLayout() on the FrameView or RenderView is an odd concept; the render tree
generally manages its own dirty state.

Most callers of setNeedsLayout() on the root are really trying to trigger the side-effects
of layout, like compositing updates, which are required when view configuration state, like
headers, footers and transparency, change. These dependencies are currently implicit and
poorly defined.

Renaming "setNeedsLayout" on FrameView is a step towards being more explicit about pure
rendering updates, vs updates of downstream data strutures like compositing. It's now called
setNeedsLayoutAfterViewConfigurationChange(). In addition, expose
setNeedsCompositingConfigurationUpdate() and setNeedsCompositingGeometryUpdate() so callers
can trigger the appropriate types of compositing updates on the root layer.

In addition, FrameViewLayoutContext::setNeedsLayoutAfterViewConfigurationChange() schedules a
layout. Withtout this, some callers would dirty the RenderView's layout but rely on some
other trigger to make the layout happen.

This cleanup was prompted by noticing that FrameView::setHeaderHeight() dirtied layout
but never scheduled it, making banner insertion in MiniBrowser unreliable.

This patch also removes the aliasing of headerHeight/footerHeight between Page and
FrameView. Banners are a property of Page, so FrameView fetches the banner heights
from Page.

* page/FrameView.cpp:
(WebCore::FrameView::headerHeight const):
(WebCore::FrameView::footerHeight const):
(WebCore::FrameView::availableContentSizeChanged):
(WebCore::FrameView::setNeedsLayoutAfterViewConfigurationChange):
(WebCore::FrameView::setNeedsCompositingConfigurationUpdate):
(WebCore::FrameView::setNeedsCompositingGeometryUpdate):
(WebCore::FrameView::scheduleSelectionUpdate):
(WebCore::FrameView::setTransparent):
(WebCore::FrameView::setBaseBackgroundColor):
(WebCore::FrameView::setAutoSizeFixedMinimumHeight):
(WebCore::FrameView::enableAutoSizeMode):
(WebCore::FrameView::setHeaderHeight): Deleted.
(WebCore::FrameView::setFooterHeight): Deleted.
(WebCore::FrameView::setNeedsLayout): Deleted.
* page/FrameView.h:
* page/FrameViewLayoutContext.cpp:
(WebCore::FrameViewLayoutContext::setNeedsLayoutAfterViewConfigurationChange):
(WebCore::FrameViewLayoutContext::setNeedsLayout): Deleted.
* page/FrameViewLayoutContext.h:
* page/Page.cpp:
(WebCore::Page::setPageScaleFactor):
(WebCore::Page::setHeaderHeight):
(WebCore::Page::setFooterHeight):
(WebCore::Page::addHeaderWithHeight): Deleted.
(WebCore::Page::addFooterWithHeight): Deleted.
* page/Page.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateBacking):
* testing/Internals.cpp:
(WebCore::Internals::resetToConsistentState):
(WebCore::Internals::setHeaderHeight):
(WebCore::Internals::setFooterHeight):

Source/WebKit:

Call the newly named functions.

* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::updateScrollbars):
* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::didInitializePlugin):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::setHeaderBannerHeightForTesting):
(WebKit::WebPage::setFooterBannerHeightForTesting):
* WebProcess/WebPage/mac/PageBannerMac.mm:
(WebKit::PageBanner::addToPage):
(WebKit::PageBanner::detachFromPage):
(WebKit::PageBanner::hide):

Source/WebKitLegacy/mac:

Call the newly named functions.

* WebView/WebFrame.mm:
(-[WebFrame setNeedsLayout]):
* WebView/WebHTMLView.mm:
(-[WebHTMLView setNeedsLayout:]):

Tools:

No need to set the banner heights on navigation now, since Page stores them.

* MiniBrowser/mac/WK2BrowserWindowController.m:
(-[WK2BrowserWindowController webView:didFinishNavigation:]):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (240911 => 240912)


--- trunk/Source/WebCore/ChangeLog	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/ChangeLog	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1,3 +1,69 @@
+2019-02-03  Simon Fraser  <[email protected]>
+
+        Make setNeedsLayout on the root more explicitly about triggering its side-effects
+        https://bugs.webkit.org/show_bug.cgi?id=194198
+
+        Reviewed by Antti Koivisto.
+
+        Calling setNeedsLayout() on the FrameView or RenderView is an odd concept; the render tree
+        generally manages its own dirty state.
+
+        Most callers of setNeedsLayout() on the root are really trying to trigger the side-effects
+        of layout, like compositing updates, which are required when view configuration state, like
+        headers, footers and transparency, change. These dependencies are currently implicit and
+        poorly defined.
+
+        Renaming "setNeedsLayout" on FrameView is a step towards being more explicit about pure
+        rendering updates, vs updates of downstream data strutures like compositing. It's now called
+        setNeedsLayoutAfterViewConfigurationChange(). In addition, expose
+        setNeedsCompositingConfigurationUpdate() and setNeedsCompositingGeometryUpdate() so callers
+        can trigger the appropriate types of compositing updates on the root layer.
+
+        In addition, FrameViewLayoutContext::setNeedsLayoutAfterViewConfigurationChange() schedules a
+        layout. Withtout this, some callers would dirty the RenderView's layout but rely on some
+        other trigger to make the layout happen.
+
+        This cleanup was prompted by noticing that FrameView::setHeaderHeight() dirtied layout
+        but never scheduled it, making banner insertion in MiniBrowser unreliable.
+
+        This patch also removes the aliasing of headerHeight/footerHeight between Page and
+        FrameView. Banners are a property of Page, so FrameView fetches the banner heights
+        from Page.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::headerHeight const):
+        (WebCore::FrameView::footerHeight const):
+        (WebCore::FrameView::availableContentSizeChanged):
+        (WebCore::FrameView::setNeedsLayoutAfterViewConfigurationChange):
+        (WebCore::FrameView::setNeedsCompositingConfigurationUpdate):
+        (WebCore::FrameView::setNeedsCompositingGeometryUpdate):
+        (WebCore::FrameView::scheduleSelectionUpdate):
+        (WebCore::FrameView::setTransparent):
+        (WebCore::FrameView::setBaseBackgroundColor):
+        (WebCore::FrameView::setAutoSizeFixedMinimumHeight):
+        (WebCore::FrameView::enableAutoSizeMode):
+        (WebCore::FrameView::setHeaderHeight): Deleted.
+        (WebCore::FrameView::setFooterHeight): Deleted.
+        (WebCore::FrameView::setNeedsLayout): Deleted.
+        * page/FrameView.h:
+        * page/FrameViewLayoutContext.cpp:
+        (WebCore::FrameViewLayoutContext::setNeedsLayoutAfterViewConfigurationChange):
+        (WebCore::FrameViewLayoutContext::setNeedsLayout): Deleted.
+        * page/FrameViewLayoutContext.h:
+        * page/Page.cpp:
+        (WebCore::Page::setPageScaleFactor):
+        (WebCore::Page::setHeaderHeight):
+        (WebCore::Page::setFooterHeight):
+        (WebCore::Page::addHeaderWithHeight): Deleted.
+        (WebCore::Page::addFooterWithHeight): Deleted.
+        * page/Page.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateBacking):
+        * testing/Internals.cpp:
+        (WebCore::Internals::resetToConsistentState):
+        (WebCore::Internals::setHeaderHeight):
+        (WebCore::Internals::setFooterHeight):
+
 2019-02-03  John Wilander  <[email protected]>
 
         Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick()

Modified: trunk/Source/WebCore/page/FrameView.cpp (240911 => 240912)


--- trunk/Source/WebCore/page/FrameView.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/FrameView.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1047,24 +1047,20 @@
     return scrollPositionForFixedPosition();
 }
 
-void FrameView::setHeaderHeight(int headerHeight)
+int FrameView::headerHeight() const
 {
-    if (frame().page())
-        ASSERT(frame().isMainFrame());
-    m_headerHeight = headerHeight;
-
-    if (RenderView* renderView = this->renderView())
-        renderView->setNeedsLayout();
+    if (!frame().isMainFrame())
+        return 0;
+    Page* page = frame().page();
+    return page ? page->headerHeight() : 0;
 }
 
-void FrameView::setFooterHeight(int footerHeight)
+int FrameView::footerHeight() const
 {
-    if (frame().page())
-        ASSERT(frame().isMainFrame());
-    m_footerHeight = footerHeight;
-
-    if (RenderView* renderView = this->renderView())
-        renderView->setNeedsLayout();
+    if (!frame().isMainFrame())
+        return 0;
+    Page* page = frame().page();
+    return page ? page->footerHeight() : 0;
 }
 
 float FrameView::topContentInset(TopContentInsetType contentInsetTypeToReturn) const
@@ -2673,7 +2669,7 @@
     }
 
     updateLayoutViewport();
-    setNeedsLayout();
+    setNeedsLayoutAfterViewConfigurationChange();
     ScrollView::availableContentSizeChanged(reason);
 }
 
@@ -2918,11 +2914,29 @@
     return layoutContext().needsLayout();
 }
 
-void FrameView::setNeedsLayout()
+void FrameView::setNeedsLayoutAfterViewConfigurationChange()
 {
-    layoutContext().setNeedsLayout();
+    layoutContext().setNeedsLayoutAfterViewConfigurationChange();
 }
 
+void FrameView::setNeedsCompositingConfigurationUpdate()
+{
+    RenderView* renderView = this->renderView();
+    if (renderView->usesCompositing()) {
+        if (auto* rootLayer = renderView->layer())
+            renderView->layer()->setNeedsCompositingConfigurationUpdate();
+    }
+}
+
+void FrameView::setNeedsCompositingGeometryUpdate()
+{
+    RenderView* renderView = this->renderView();
+    if (renderView->usesCompositing()) {
+        if (auto* rootLayer = renderView->layer())
+            renderView->layer()->setNeedsCompositingGeometryUpdate();
+    }
+}
+
 void FrameView::scheduleSelectionUpdate()
 {
     if (needsLayout())
@@ -2929,8 +2943,7 @@
         return;
     // FIXME: We should not need to go through the layout process since selection update does not change dimension/geometry.
     // However we can't tell at this point if the tree is stable yet, so let's just schedule a root only layout for now.
-    setNeedsLayout();
-    layoutContext().scheduleLayout();
+    setNeedsLayoutAfterViewConfigurationChange();
 }
 
 bool FrameView::isTransparent() const
@@ -2952,8 +2965,8 @@
     if (!isViewForDocumentInFrame())
         return;
 
-    renderView()->compositor().rootBackgroundColorOrTransparencyChanged();
-    setNeedsLayout();
+    setNeedsLayoutAfterViewConfigurationChange();
+    setNeedsCompositingConfigurationUpdate();
 }
 
 bool FrameView::hasOpaqueBackground() const
@@ -2974,9 +2987,8 @@
         return;
 
     recalculateScrollbarOverlayStyle();
-    setNeedsLayout();
-
-    renderView()->compositor().rootBackgroundColorOrTransparencyChanged();
+    setNeedsLayoutAfterViewConfigurationChange();
+    setNeedsCompositingConfigurationUpdate();
 }
 
 void FrameView::updateBackgroundRecursively(bool transparent)
@@ -3516,7 +3528,7 @@
 
     m_autoSizeFixedMinimumHeight = fixedMinimumHeight;
 
-    setNeedsLayout();
+    setNeedsLayoutAfterViewConfigurationChange();
 }
 
 RenderElement* FrameView::viewportRenderer() const
@@ -4516,7 +4528,7 @@
     m_maxAutoSize = maxSize;
     m_didRunAutosize = false;
 
-    setNeedsLayout();
+    setNeedsLayoutAfterViewConfigurationChange();
     layoutContext().scheduleLayout();
     if (m_shouldAutoSize) {
         overrideViewportSizeForCSSViewportUnits({ minSize.width(), m_overrideViewportSize ? m_overrideViewportSize->height : WTF::nullopt });

Modified: trunk/Source/WebCore/page/FrameView.h (240911 => 240912)


--- trunk/Source/WebCore/page/FrameView.h	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/FrameView.h	2019-02-04 01:48:13 UTC (rev 240912)
@@ -117,7 +117,11 @@
     void queuePostLayoutCallback(WTF::Function<void ()>&&);
 
     WEBCORE_EXPORT bool needsLayout() const;
-    WEBCORE_EXPORT void setNeedsLayout();
+    WEBCORE_EXPORT void setNeedsLayoutAfterViewConfigurationChange();
+
+    void setNeedsCompositingConfigurationUpdate();
+    void setNeedsCompositingGeometryUpdate();
+
     void setViewportConstrainedObjectsNeedLayout();
 
     WEBCORE_EXPORT bool renderedCharactersExceed(unsigned threshold);
@@ -573,10 +577,8 @@
 
     LayoutPoint scrollPositionRespectingCustomFixedPosition() const;
 
-    int headerHeight() const final { return m_headerHeight; }
-    WEBCORE_EXPORT void setHeaderHeight(int);
-    int footerHeight() const final { return m_footerHeight; }
-    WEBCORE_EXPORT void setFooterHeight(int);
+    WEBCORE_EXPORT int headerHeight() const final;
+    WEBCORE_EXPORT int footerHeight() const final;
 
     WEBCORE_EXPORT float topContentInset(TopContentInsetType = TopContentInsetType::WebCoreContentInset) const final;
     void topContentInsetDidChange(float newTopContentInset);

Modified: trunk/Source/WebCore/page/FrameViewLayoutContext.cpp (240911 => 240912)


--- trunk/Source/WebCore/page/FrameViewLayoutContext.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/FrameViewLayoutContext.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -313,7 +313,7 @@
         || (m_disableSetNeedsLayoutCount && m_setNeedsLayoutWasDeferred);
 }
 
-void FrameViewLayoutContext::setNeedsLayout()
+void FrameViewLayoutContext::setNeedsLayoutAfterViewConfigurationChange()
 {
     if (m_disableSetNeedsLayoutCount) {
         m_setNeedsLayoutWasDeferred = true;
@@ -323,6 +323,7 @@
     if (auto* renderView = this->renderView()) {
         ASSERT(!renderView->inHitTesting());
         renderView->setNeedsLayout();
+        scheduleLayout();
     }
 }
 

Modified: trunk/Source/WebCore/page/FrameViewLayoutContext.h (240911 => 240912)


--- trunk/Source/WebCore/page/FrameViewLayoutContext.h	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/FrameViewLayoutContext.h	2019-02-04 01:48:13 UTC (rev 240912)
@@ -50,10 +50,12 @@
     ~FrameViewLayoutContext();
 
     void layout();
-
-    void setNeedsLayout();
     bool needsLayout() const;
 
+    // We rely on the side-effects of layout, like compositing updates, to update state in various subsystems
+    // whose dependencies are poorly defined. This call triggers such updates.
+    void setNeedsLayoutAfterViewConfigurationChange();
+
     void scheduleLayout();
     void scheduleSubtreeLayout(RenderElement& layoutRoot);
     void unscheduleLayout();

Modified: trunk/Source/WebCore/page/Page.cpp (240911 => 240912)


--- trunk/Source/WebCore/page/Page.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/Page.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1005,11 +1005,8 @@
     m_pageScaleFactor = scale;
 
     if (!m_settings->delegatesPageScaling()) {
-        if (auto* renderView = document->renderView()) {
-            renderView->setNeedsLayout();
-            if (renderView->hasLayer() && renderView->layer()->isComposited())
-                renderView->layer()->setNeedsCompositingGeometryUpdate();
-        }
+        view->setNeedsLayoutAfterViewConfigurationChange();
+        view->setNeedsCompositingGeometryUpdate();
 
         document->resolveStyle(Document::ResolveStyleType::Rebuild);
 
@@ -1968,9 +1965,11 @@
     return VisibilityState::Hidden;
 }
 
-#if ENABLE(RUBBER_BANDING)
-void Page::addHeaderWithHeight(int headerHeight)
+void Page::setHeaderHeight(int headerHeight)
 {
+    if (headerHeight == m_headerHeight)
+        return;
+
     m_headerHeight = headerHeight;
 
     FrameView* frameView = mainFrame().view();
@@ -1981,12 +1980,15 @@
     if (!renderView)
         return;
 
-    frameView->setHeaderHeight(m_headerHeight);
-    renderView->compositor().updateLayerForHeader(m_headerHeight);
+    frameView->setNeedsLayoutAfterViewConfigurationChange();
+    frameView->setNeedsCompositingGeometryUpdate();
 }
 
-void Page::addFooterWithHeight(int footerHeight)
+void Page::setFooterHeight(int footerHeight)
 {
+    if (footerHeight == m_footerHeight)
+        return;
+
     m_footerHeight = footerHeight;
 
     FrameView* frameView = mainFrame().view();
@@ -1997,10 +1999,9 @@
     if (!renderView)
         return;
 
-    frameView->setFooterHeight(m_footerHeight);
-    renderView->compositor().updateLayerForFooter(m_footerHeight);
+    frameView->setNeedsLayoutAfterViewConfigurationChange();
+    frameView->setNeedsCompositingGeometryUpdate();
 }
-#endif
 
 void Page::incrementNestedRunLoopCount()
 {

Modified: trunk/Source/WebCore/page/Page.h (240911 => 240912)


--- trunk/Source/WebCore/page/Page.h	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/page/Page.h	2019-02-04 01:48:13 UTC (rev 240912)
@@ -519,10 +519,8 @@
     WEBCORE_EXPORT void removeLayoutMilestones(OptionSet<LayoutMilestone>);
     OptionSet<LayoutMilestone> requestedLayoutMilestones() const { return m_requestedLayoutMilestones; }
 
-#if ENABLE(RUBBER_BANDING)
-    WEBCORE_EXPORT void addHeaderWithHeight(int);
-    WEBCORE_EXPORT void addFooterWithHeight(int);
-#endif
+    WEBCORE_EXPORT void setHeaderHeight(int);
+    WEBCORE_EXPORT void setFooterHeight(int);
 
     int headerHeight() const { return m_headerHeight; }
     int footerHeight() const { return m_footerHeight; }

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (240911 => 240912)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1543,16 +1543,17 @@
             layer.ensureBacking();
 
             if (layer.isRenderViewLayer() && useCoordinatedScrollingForLayer(layer)) {
+                auto& frameView = m_renderView.frameView();
                 if (auto* scrollingCoordinator = this->scrollingCoordinator())
-                    scrollingCoordinator->frameViewRootLayerDidChange(m_renderView.frameView());
+                    scrollingCoordinator->frameViewRootLayerDidChange(frameView);
 #if ENABLE(RUBBER_BANDING)
-                updateLayerForHeader(page().headerHeight());
-                updateLayerForFooter(page().footerHeight());
+                updateLayerForHeader(frameView.headerHeight());
+                updateLayerForFooter(frameView.footerHeight());
 #endif
                 updateRootContentLayerClipping();
 
                 if (auto* tiledBacking = layer.backing()->tiledBacking())
-                    tiledBacking->setTopContentInset(m_renderView.frameView().topContentInset());
+                    tiledBacking->setTopContentInset(frameView.topContentInset());
             }
 
             // This layer and all of its descendants have cached repaints rects that are relative to

Modified: trunk/Source/WebCore/testing/Internals.cpp (240911 => 240912)


--- trunk/Source/WebCore/testing/Internals.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebCore/testing/Internals.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -449,8 +449,8 @@
 
     FrameView* mainFrameView = page.mainFrame().view();
     if (mainFrameView) {
-        mainFrameView->setHeaderHeight(0);
-        mainFrameView->setFooterHeight(0);
+        page.setHeaderHeight(0);
+        page.setFooterHeight(0);
         page.setTopContentInset(0);
         mainFrameView->setUseFixedLayout(false);
         mainFrameView->setFixedLayoutSize(IntSize());
@@ -2903,7 +2903,7 @@
     if (!document || !document->view())
         return;
 
-    document->view()->setHeaderHeight(height);
+    document->page()->setHeaderHeight(height);
 }
 
 void Internals::setFooterHeight(float height)
@@ -2912,7 +2912,7 @@
     if (!document || !document->view())
         return;
 
-    document->view()->setFooterHeight(height);
+    document->page()->setFooterHeight(height);
 }
 
 void Internals::setTopContentInset(float contentInset)

Modified: trunk/Source/WebKit/ChangeLog (240911 => 240912)


--- trunk/Source/WebKit/ChangeLog	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKit/ChangeLog	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1,3 +1,24 @@
+2019-02-03  Simon Fraser  <[email protected]>
+
+        Make setNeedsLayout on the root more explicitly about triggering its side-effects
+        https://bugs.webkit.org/show_bug.cgi?id=194198
+
+        Reviewed by Antti Koivisto.
+
+        Call the newly named functions.
+
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::updateScrollbars):
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::didInitializePlugin):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::setHeaderBannerHeightForTesting):
+        (WebKit::WebPage::setFooterBannerHeightForTesting):
+        * WebProcess/WebPage/mac/PageBannerMac.mm:
+        (WebKit::PageBanner::addToPage):
+        (WebKit::PageBanner::detachFromPage):
+        (WebKit::PageBanner::hide):
+
 2019-02-03  Ryosuke Niwa  <[email protected]>
 
         Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy

Modified: trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm (240911 => 240912)


--- trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm	2019-02-04 01:48:13 UTC (rev 240912)
@@ -735,7 +735,7 @@
         else
             frameView->removeScrollableArea(this);
 
-        frameView->setNeedsLayout();
+        frameView->setNeedsLayoutAfterViewConfigurationChange();
     }
     
     if (m_verticalScrollbarLayer) {

Modified: trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp (240911 => 240912)


--- trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -663,7 +663,7 @@
     if (wantsWheelEvents()) {
         if (Frame* frame = m_pluginElement->document().frame()) {
             if (FrameView* frameView = frame->view())
-                frameView->setNeedsLayout();
+                frameView->setNeedsLayoutAfterViewConfigurationChange();
         }
     }
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (240911 => 240912)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-02-04 01:48:13 UTC (rev 240912)
@@ -2128,16 +2128,12 @@
 
 void WebPage::setHeaderBannerHeightForTesting(int height)
 {
-#if ENABLE(RUBBER_BANDING)
-    corePage()->addHeaderWithHeight(height);
-#endif
+    corePage()->setHeaderHeight(height);
 }
 
 void WebPage::setFooterBannerHeightForTesting(int height)
 {
-#if ENABLE(RUBBER_BANDING)
-    corePage()->addFooterWithHeight(height);
-#endif
+    corePage()->setFooterHeight(height);
 }
 
 #endif // !PLATFORM(IOS_FAMILY)

Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/PageBannerMac.mm (240911 => 240912)


--- trunk/Source/WebKit/WebProcess/WebPage/mac/PageBannerMac.mm	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/PageBannerMac.mm	2019-02-04 01:48:13 UTC (rev 240912)
@@ -57,10 +57,10 @@
 
     switch (m_type) {
     case Header:
-        m_webPage->corePage()->addHeaderWithHeight(m_height);
+        m_webPage->corePage()->setHeaderHeight(m_height);
         break;
     case Footer:
-        m_webPage->corePage()->addFooterWithHeight(m_height);
+        m_webPage->corePage()->setFooterHeight(m_height);
         break;
     case NotSet:
         ASSERT_NOT_REACHED();
@@ -86,9 +86,9 @@
     if (m_webPage->corePage()) {
         // We can hide the banner by removing the parent layer that hosts it.
         if (m_type == Header)
-            m_webPage->corePage()->addHeaderWithHeight(0);
+            m_webPage->corePage()->setHeaderHeight(0);
         else if (m_type == Footer)
-            m_webPage->corePage()->addFooterWithHeight(0);
+            m_webPage->corePage()->setFooterHeight(0);
     }
 
     m_type = NotSet;
@@ -99,9 +99,9 @@
 {
     // We can hide the banner by removing the parent layer that hosts it.
     if (m_type == Header)
-        m_webPage->corePage()->addHeaderWithHeight(0);
+        m_webPage->corePage()->setHeaderHeight(0);
     else if (m_type == Footer)
-        m_webPage->corePage()->addFooterWithHeight(0);
+        m_webPage->corePage()->setFooterHeight(0);
 
     m_isHidden = true;
 }

Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (240911 => 240912)


--- trunk/Source/WebKitLegacy/mac/ChangeLog	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1,3 +1,17 @@
+2019-02-03  Simon Fraser  <[email protected]>
+
+        Make setNeedsLayout on the root more explicitly about triggering its side-effects
+        https://bugs.webkit.org/show_bug.cgi?id=194198
+
+        Reviewed by Antti Koivisto.
+
+        Call the newly named functions.
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame setNeedsLayout]):
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView setNeedsLayout:]):
+
 2019-02-03  Ryosuke Niwa  <[email protected]>
 
         Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy

Modified: trunk/Source/WebKitLegacy/mac/WebView/WebFrame.mm (240911 => 240912)


--- trunk/Source/WebKitLegacy/mac/WebView/WebFrame.mm	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebFrame.mm	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1310,7 +1310,7 @@
 {
     WebCore::Frame *frame = core(self);
     if (frame->view())
-        frame->view()->setNeedsLayout();
+        frame->view()->setNeedsLayoutAfterViewConfigurationChange();
 }
 
 - (CGSize)renderedSizeOfNode:(DOMNode *)node constrainedToWidth:(float)width

Modified: trunk/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm (240911 => 240912)


--- trunk/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm	2019-02-04 01:48:13 UTC (rev 240912)
@@ -3869,7 +3869,7 @@
         if (frame->document() && frame->document()->pageCacheState() != Document::NotInPageCache)
             return;
         if (FrameView* view = frame->view())
-            view->setNeedsLayout();
+            view->setNeedsLayoutAfterViewConfigurationChange();
     }
 }
 

Modified: trunk/Tools/ChangeLog (240911 => 240912)


--- trunk/Tools/ChangeLog	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Tools/ChangeLog	2019-02-04 01:48:13 UTC (rev 240912)
@@ -1,3 +1,15 @@
+2019-02-03  Simon Fraser  <[email protected]>
+
+        Make setNeedsLayout on the root more explicitly about triggering its side-effects
+        https://bugs.webkit.org/show_bug.cgi?id=194198
+
+        Reviewed by Antti Koivisto.
+
+        No need to set the banner heights on navigation now, since Page stores them.
+
+        * MiniBrowser/mac/WK2BrowserWindowController.m:
+        (-[WK2BrowserWindowController webView:didFinishNavigation:]):
+
 2019-02-03  John Wilander  <[email protected]>
 
         Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick()

Modified: trunk/Tools/MiniBrowser/mac/WK2BrowserWindowController.m (240911 => 240912)


--- trunk/Tools/MiniBrowser/mac/WK2BrowserWindowController.m	2019-02-04 00:44:07 UTC (rev 240911)
+++ trunk/Tools/MiniBrowser/mac/WK2BrowserWindowController.m	2019-02-04 01:48:13 UTC (rev 240912)
@@ -674,12 +674,6 @@
 - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
 {
     LOG(@"didFinishNavigation: %@", navigation);
-    
-    // Banner heights don't persist across page loads (oddly, since Page stores them), so reset on every page load.
-    if ([[SettingsController shared] isSpaceReservedForBanners]) {
-        [_webView _setHeaderBannerHeight:testHeaderBannerHeight];
-        [_webView _setFooterBannerHeight:testFooterBannerHeight];
-    }
 }
 
 - (void)webView:(WKWebView *)webView didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *__nullable credential))completionHandler
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to