[FrameView::layout cleanup] Move scrollbars setup logic to a separate function
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 21:23:13 +0000 (21:23 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 21:23:13 +0000 (21:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178394
<rdar://problem/35031066>

Reviewed by Antti Koivisto.

Decouple scrollbars setup and the unrelated first-layout logic.
FIXME: find out why m_firstLayout depends on the subtree flag (I'd assume we issue full layout the very first time).

Covered by existing test cases.

* page/FrameView.cpp:
(WebCore::FrameView::adjustScrollbarsForLayout):
(WebCore::FrameView::layout):
* page/FrameView.h:

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h

index 7c69a8c..e247201 100644 (file)
@@ -1,3 +1,21 @@
+2017-10-19  Zalan Bujtas  <zalan@apple.com>
+
+        [FrameView::layout cleanup] Move scrollbars setup logic to a separate function
+        https://bugs.webkit.org/show_bug.cgi?id=178394
+        <rdar://problem/35031066>
+
+        Reviewed by Antti Koivisto.
+
+        Decouple scrollbars setup and the unrelated first-layout logic.
+        FIXME: find out why m_firstLayout depends on the subtree flag (I'd assume we issue full layout the very first time). 
+
+        Covered by existing test cases.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::adjustScrollbarsForLayout):
+        (WebCore::FrameView::layout):
+        * page/FrameView.h:
+
 2017-10-19  Tim Horton  <timothy_horton@apple.com>
 
         Expand r209943 to suppress paste during provisional navigation as well
index 8839a3e..e4e300e 100644 (file)
@@ -1340,6 +1340,28 @@ void FrameView::markRootOrBodyRendererDirty() const
         rootRenderer->setChildNeedsLayout();
 }
 
+void FrameView::adjustScrollbarsForLayout(bool isFirstLayout)
+{
+    ScrollbarMode hMode;
+    ScrollbarMode vMode;
+    calculateScrollbarModesForLayout(hMode, vMode);
+    if (isFirstLayout && !isLayoutNested()) {
+        setScrollbarsSuppressed(true);
+        // Set the initial vMode to AlwaysOn if we're auto.
+        if (vMode == ScrollbarAuto)
+            setVerticalScrollbarMode(ScrollbarAlwaysOn); // This causes a vertical scrollbar to appear.
+        // Set the initial hMode to AlwaysOff if we're auto.
+        if (hMode == ScrollbarAuto)
+            setHorizontalScrollbarMode(ScrollbarAlwaysOff); // This causes a horizontal scrollbar to disappear.
+        ASSERT(frame().page());
+        if (frame().page()->expectsWheelEventTriggers())
+            scrollAnimator().setWheelEventTestTrigger(frame().page()->testTrigger());
+        setScrollbarModes(hMode, vMode);
+        setScrollbarsSuppressed(false, true);
+    } else if (hMode != horizontalScrollbarMode() || vMode != verticalScrollbarMode())
+        setScrollbarModes(hMode, vMode);
+}
+
 void FrameView::layout(bool allowSubtreeLayout)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(!frame().document()->inRenderTreeUpdate());
@@ -1436,33 +1458,12 @@ void FrameView::layout(bool allowSubtreeLayout)
             if (m_firstLayout && !frame().ownerElement())
                 LOG(Layout, "FrameView %p elapsed time before first layout: %.3fs\n", this, document.timeSinceDocumentCreation().value());
 #endif
-            ScrollbarMode hMode;
-            ScrollbarMode vMode;    
-            calculateScrollbarModesForLayout(hMode, vMode);
-
-            if (m_firstLayout || (hMode != horizontalScrollbarMode() || vMode != verticalScrollbarMode())) {
-                if (m_firstLayout) {
-                    setScrollbarsSuppressed(true);
-
-                    m_firstLayout = false;
-                    m_firstLayoutCallbackPending = true;
-                    m_lastViewportSize = sizeForResizeEvent();
-                    m_lastZoomFactor = layoutRoot->style().zoom();
-
-                    // Set the initial vMode to AlwaysOn if we're auto.
-                    if (vMode == ScrollbarAuto)
-                        setVerticalScrollbarMode(ScrollbarAlwaysOn); // This causes a vertical scrollbar to appear.
-                    // Set the initial hMode to AlwaysOff if we're auto.
-                    if (hMode == ScrollbarAuto)
-                        setHorizontalScrollbarMode(ScrollbarAlwaysOff); // This causes a horizontal scrollbar to disappear.
-                    Page* page = frame().page();
-                    if (page && page->expectsWheelEventTriggers())
-                        scrollAnimator().setWheelEventTestTrigger(page->testTrigger());
-                    setScrollbarModes(hMode, vMode);
-                    setScrollbarsSuppressed(false, true);
-                } else
-                    setScrollbarModes(hMode, vMode);
+            if (m_firstLayout) {
+                m_lastViewportSize = sizeForResizeEvent();
+                m_lastZoomFactor = layoutRoot->style().zoom();
+                m_firstLayoutCallbackPending = true;
             }
+            adjustScrollbarsForLayout(m_firstLayout);
 
             auto oldSize = m_size;
             m_size = layoutSize();
@@ -1473,6 +1474,7 @@ void FrameView::layout(bool allowSubtreeLayout)
                     markRootOrBodyRendererDirty();
             }
             m_layoutPhase = InPreLayout;
+            m_firstLayout = false;
         }
 
         ASSERT(allowSubtreeLayout || !isSubtreeLayout);
index 8ac0e5f..a2711b0 100644 (file)
@@ -735,6 +735,8 @@ private:
     IntSize sizeForResizeEvent() const;
     void sendResizeEventIfNeeded();
 
+    void adjustScrollbarsForLayout(bool firstLayout);
+
     void handleDeferredScrollbarsUpdateAfterDirectionChange();
 
     void updateScrollableAreaSet();