Title: [178661] trunk/Source/WebCore
Revision
178661
Author
bfulg...@apple.com
Date
2015-01-19 12:02:01 -0800 (Mon, 19 Jan 2015)

Log Message

Layers need to be already updated before we call adjustViewSize
https://bugs.webkit.org/show_bug.cgi?id=135514

Reviewed by Simon Fraser.

Tested by 'fast/dynamic/layer-no-longer-paginated.html'

Defer painting operations until we have finished layout. This
has a couple of benefits:
(1) We do not attempt to modify render layers during layout.
(2) In WK1 we do not attempt to paint during layout.

Add a new virtual predicate to ScrollView indicating when we are in
layout so that calls to setContentsSize do not attempt
to adjust scrollbars.

Modify FrameView to set its ScrollView state to block paint
operations during layout. Also add a post-layout handler to
complete the scrollbar updates after layout is finished.

* WebCore.exp.in: Move linker symbol to ScrollView (from FrameView).
* page/FrameView.cpp:
(WebCore::FrameView::layout):
(WebCore::FrameView::shouldDeferScrollUpdateAfterContentSizeChange): Added.
(WebCore::FrameView::scrollPositionChangedViaPlatformWidget): Removed (Renamed).
(WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Added (Renamed)
(WebCore::FrameView::paintContents): Do not paint if we are inside view size adjustment.
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::scrollPositionChangedViaPlatformWidget): Added. Checks whether we need to defer
painting, and calls virtual scrollPositionChangedViaPlatformWidgetImpl if we do not.
(WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Added.
(WebCore::ScrollView::handleDeferredScrollUpdateAfterContentSizeChange): Added.
(WebCore::ScrollView::scrollTo): If we should defer painting, cache the
the scroll delta and apply it after the layout is complete.
(WebCore::ScrollView::completeUpdatesAfterScrollTo): Split off part of 'scrollTo' into its own method
so we can reuse it in handleDeferredScrollUpdateAfterContentSizeChange.
* platform/ScrollView.h:
(WebCore::ScrollView::shouldDeferScrollUpdateAfterContentSizeChange): Added.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (178660 => 178661)


--- trunk/Source/WebCore/ChangeLog	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/ChangeLog	2015-01-19 20:02:01 UTC (rev 178661)
@@ -1,3 +1,45 @@
+2015-01-19  Brent Fulgham  <bfulg...@apple.com>
+
+        Layers need to be already updated before we call adjustViewSize
+        https://bugs.webkit.org/show_bug.cgi?id=135514
+
+        Reviewed by Simon Fraser.
+
+        Tested by 'fast/dynamic/layer-no-longer-paginated.html'
+
+        Defer painting operations until we have finished layout. This
+        has a couple of benefits:
+        (1) We do not attempt to modify render layers during layout.
+        (2) In WK1 we do not attempt to paint during layout.
+
+        Add a new virtual predicate to ScrollView indicating when we are in
+        layout so that calls to setContentsSize do not attempt
+        to adjust scrollbars.
+
+        Modify FrameView to set its ScrollView state to block paint
+        operations during layout. Also add a post-layout handler to
+        complete the scrollbar updates after layout is finished.
+
+        * WebCore.exp.in: Move linker symbol to ScrollView (from FrameView).
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout):
+        (WebCore::FrameView::shouldDeferScrollUpdateAfterContentSizeChange): Added.
+        (WebCore::FrameView::scrollPositionChangedViaPlatformWidget): Removed (Renamed).
+        (WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Added (Renamed)
+        (WebCore::FrameView::paintContents): Do not paint if we are inside view size adjustment.
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::scrollPositionChangedViaPlatformWidget): Added. Checks whether we need to defer
+        painting, and calls virtual scrollPositionChangedViaPlatformWidgetImpl if we do not.
+        (WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Added.
+        (WebCore::ScrollView::handleDeferredScrollUpdateAfterContentSizeChange): Added.
+        (WebCore::ScrollView::scrollTo): If we should defer painting, cache the
+        the scroll delta and apply it after the layout is complete.
+        (WebCore::ScrollView::completeUpdatesAfterScrollTo): Split off part of 'scrollTo' into its own method
+        so we can reuse it in handleDeferredScrollUpdateAfterContentSizeChange.
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::shouldDeferScrollUpdateAfterContentSizeChange): Added.
+
 2015-01-16  Ada Chan  <adac...@apple.com>
 
         HTMLMediaElement::isPlayingAudio() should return false if the element is explicitly muted by script.

Modified: trunk/Source/WebCore/WebCore.exp.in (178660 => 178661)


--- trunk/Source/WebCore/WebCore.exp.in	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/WebCore.exp.in	2015-01-19 20:02:01 UTC (rev 178661)
@@ -95,6 +95,7 @@
 __ZN7WebCore10ScrollView23setPaintsEntireContentsEb
 __ZN7WebCore10ScrollView23setScrollbarsSuppressedEbb
 __ZN7WebCore10ScrollView24windowResizerRectChangedEv
+__ZN7WebCore10ScrollView38scrollPositionChangedViaPlatformWidgetERKNS_8IntPointES3_
 __ZN7WebCore10ScrollView4hideEv
 __ZN7WebCore10ScrollView4showEv
 __ZN7WebCore10ScrollView5paintEPNS_15GraphicsContextERKNS_7IntRectE
@@ -1491,7 +1492,6 @@
 __ZN7WebCore9FrameView34setViewportSizeForCSSViewportUnitsENS_7IntSizeE
 __ZN7WebCore9FrameView37setScrollingPerformanceLoggingEnabledEb
 __ZN7WebCore9FrameView37updateLayoutAndStyleIfNeededRecursiveEv
-__ZN7WebCore9FrameView38scrollPositionChangedViaPlatformWidgetERKNS_8IntPointES3_
 __ZN7WebCore9FrameView39flushCompositingStateIncludingSubframesEv
 __ZN7WebCore9FrameView52disableLayerFlushThrottlingTemporarilyForInteractionEv
 __ZN7WebCore9FrameView6createERNS_5FrameE

Modified: trunk/Source/WebCore/page/FrameView.cpp (178660 => 178661)


--- trunk/Source/WebCore/page/FrameView.cpp	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/page/FrameView.cpp	2015-01-19 20:02:01 UTC (rev 178661)
@@ -1161,7 +1161,7 @@
     
     if (!allowSubtree && m_layoutRoot) {
         m_layoutRoot->markContainingBlocksForLayout(false);
-        m_layoutRoot = 0;
+        m_layoutRoot = nullptr;
     }
 
     ASSERT(frame().view() == this);
@@ -1358,7 +1358,9 @@
     layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_needsFullRepaint));
 
     updateCompositingLayersAfterLayout();
-    
+
+    m_layoutPhase = InPostLayerPositionsUpdatedAfterLayout;
+
     m_layoutCount++;
 
 #if PLATFORM(COCOA) || PLATFORM(WIN) || PLATFORM(GTK) || PLATFORM(EFL)
@@ -1378,6 +1380,8 @@
 
     updateCanBlitOnScrollRecursively();
 
+    handleDeferredScrollUpdateAfterContentSizeChange();
+
     if (document.hasListenerType(Document::OVERFLOWCHANGED_LISTENER))
         updateOverflowStatus(layoutWidth() < contentsWidth(), layoutHeight() < contentsHeight());
 
@@ -1409,6 +1413,11 @@
     --m_nestedLayoutCount;
 }
 
+bool FrameView::shouldDeferScrollUpdateAfterContentSizeChange()
+{
+    return (m_layoutPhase < InPostLayout) && (m_layoutPhase != OutsideLayout);
+}
+
 RenderBox* FrameView::embeddedContentBox() const
 {
     RenderView* renderView = this->renderView();
@@ -2097,7 +2106,7 @@
     frame().loader().client().didChangeScrollOffset();
 }
 
-void FrameView::scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition)
+void FrameView::scrollPositionChangedViaPlatformWidgetImpl(const IntPoint& oldPosition, const IntPoint& newPosition)
 {
     updateLayerPositionsAfterScrolling();
     updateCompositingLayersAfterScrolling();
@@ -2181,6 +2190,8 @@
 
 void FrameView::updateCompositingLayersAfterScrolling()
 {
+    ASSERT(m_layoutPhase >= InPostLayout || m_layoutPhase == OutsideLayout);
+
     if (!shouldUpdateCompositingLayersAfterScrolling())
         return;
 
@@ -3812,6 +3823,11 @@
 
 void FrameView::paintContents(GraphicsContext* context, const IntRect& dirtyRect)
 {
+    if (m_layoutPhase == InViewSizeAdjust)
+        return;
+
+    ASSERT(m_layoutPhase == InPostLayerPositionsUpdatedAfterLayout || m_layoutPhase == OutsideLayout);
+
 #ifndef NDEBUG
     bool fillWithRed;
     if (frame().document()->printing())

Modified: trunk/Source/WebCore/page/FrameView.h (178660 => 178661)


--- trunk/Source/WebCore/page/FrameView.h	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/page/FrameView.h	2015-01-19 20:02:01 UTC (rev 178661)
@@ -235,7 +235,6 @@
     virtual void setFixedVisibleContentRect(const IntRect&) override;
 #endif
     WEBCORE_EXPORT virtual void setScrollPosition(const IntPoint&) override;
-    WEBCORE_EXPORT void scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition);
     virtual void updateLayerPositionsAfterScrolling() override;
     virtual void updateCompositingLayersAfterScrolling() override;
     virtual bool requestScrollPositionUpdate(const IntPoint&) override;
@@ -542,6 +541,7 @@
         InLayout,
         InViewSizeAdjust,
         InPostLayout,
+        InPostLayerPositionsUpdatedAfterLayout,
     };
     LayoutPhase layoutPhase() const { return m_layoutPhase; }
 
@@ -558,6 +558,10 @@
 
     bool shouldUpdateCompositingLayersAfterScrolling() const;
 
+    virtual bool shouldDeferScrollUpdateAfterContentSizeChange() override;
+
+    virtual void scrollPositionChangedViaPlatformWidgetImpl(const IntPoint& oldPosition, const IntPoint& newPosition) override;
+
     void applyOverflowToViewport(RenderElement*, ScrollbarMode& hMode, ScrollbarMode& vMode);
     void applyPaginationToViewport();
 

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (178660 => 178661)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2015-01-19 20:02:01 UTC (rev 178661)
@@ -457,10 +457,41 @@
     scrollTo(newOffset);
 }
 
+void ScrollView::scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition)
+{
+    // We should not attempt to actually modify (paint) platform widgets if the layout phase
+    // is not complete. Instead, defer the scroll event until the layout finishes.
+    if (shouldDeferScrollUpdateAfterContentSizeChange()) {
+        // We only care about the most recent scroll position change request
+        m_deferredScrollPositions = std::make_unique<std::pair<IntPoint, IntPoint>>(std::make_pair(oldPosition, newPosition));
+        return;
+    }
+
+    scrollPositionChangedViaPlatformWidgetImpl(oldPosition, newPosition);
+}
+
+void ScrollView::handleDeferredScrollUpdateAfterContentSizeChange()
+{
+    ASSERT(!shouldDeferScrollUpdateAfterContentSizeChange());
+
+    if (!m_deferredScrollDelta && !m_deferredScrollPositions)
+        return;
+
+    ASSERT(static_cast<bool>(m_deferredScrollDelta) != static_cast<bool>(m_deferredScrollPositions));
+
+    if (m_deferredScrollDelta)
+        completeUpdatesAfterScrollTo(*m_deferredScrollDelta);
+    else if (m_deferredScrollPositions)
+        scrollPositionChangedViaPlatformWidgetImpl(m_deferredScrollPositions->first, m_deferredScrollPositions->second);
+    
+    m_deferredScrollDelta = nullptr;
+    m_deferredScrollPositions = nullptr;
+}
+
 void ScrollView::scrollTo(const IntSize& newOffset)
 {
     IntSize scrollDelta = newOffset - m_scrollOffset;
-    if (scrollDelta == IntSize())
+    if (scrollDelta.isZero())
         return;
     m_scrollOffset = newOffset;
 
@@ -473,6 +504,19 @@
         return;
     }
 #endif
+    // We should not attempt to actually modify layer contents if the layout phase
+    // is not complete. Instead, defer the scroll event until the layout finishes.
+    if (shouldDeferScrollUpdateAfterContentSizeChange()) {
+        ASSERT(!m_deferredScrollDelta);
+        m_deferredScrollDelta = std::make_unique<IntSize>(scrollDelta);
+        return;
+    }
+
+    completeUpdatesAfterScrollTo(scrollDelta);
+}
+
+void ScrollView::completeUpdatesAfterScrollTo(const IntSize& scrollDelta)
+{
     updateLayerPositionsAfterScrolling();
     scrollContents(scrollDelta);
     updateCompositingLayersAfterScrolling();

Modified: trunk/Source/WebCore/platform/ScrollView.h (178660 => 178661)


--- trunk/Source/WebCore/platform/ScrollView.h	2015-01-19 19:45:24 UTC (rev 178660)
+++ trunk/Source/WebCore/platform/ScrollView.h	2015-01-19 20:02:01 UTC (rev 178661)
@@ -379,6 +379,8 @@
 
     virtual bool isScrollView() const override { return true; }
 
+    WEBCORE_EXPORT void scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition);
+
 protected:
     ScrollView();
 
@@ -417,10 +419,18 @@
     float platformTopContentInset() const;
     void platformSetTopContentInset(float);
 
+    virtual void handleDeferredScrollUpdateAfterContentSizeChange();
+
+    virtual bool shouldDeferScrollUpdateAfterContentSizeChange() { return false; }
+
+    virtual void scrollPositionChangedViaPlatformWidgetImpl(const IntPoint&, const IntPoint&) { }
+
 private:
     virtual IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
     WEBCORE_EXPORT IntRect unobscuredContentRectInternal(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
 
+    void completeUpdatesAfterScrollTo(const IntSize& scrollDelta);
+
     RefPtr<Scrollbar> m_horizontalScrollbar;
     RefPtr<Scrollbar> m_verticalScrollbar;
     ScrollbarMode m_horizontalScrollbarMode;
@@ -450,6 +460,10 @@
     IntSize m_fixedLayoutSize;
     IntSize m_contentsSize;
 
+    std::unique_ptr<IntSize> m_deferredScrollDelta; // Needed for WebKit scrolling
+    std::unique_ptr<std::pair<IntPoint, IntPoint>> m_deferredScrollPositions; // Needed for platform widget scrolling
+
+
     int m_scrollbarsAvoidingResizer;
     bool m_scrollbarsSuppressed;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to