RenderBlock::simplifiedLayout should pop LayoutStateMaintainer when early returns.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Aug 2015 20:27:58 +0000 (20:27 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Aug 2015 20:27:58 +0000 (20:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148031

Reviewed by Simon Fraser.

LayoutStateMaintainer push/pop calls need to be balanced to ensure layout consistency.

Unable to make a test case for this.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::simplifiedLayout):
* rendering/RenderView.h:
(WebCore::LayoutStateMaintainer::~LayoutStateMaintainer): ASSERT the state properly.
(WebCore::LayoutStateMaintainer::push):
(WebCore::LayoutStateMaintainer::pop):
(WebCore::LayoutStateMaintainer::didPush):
(WebCore::LayoutStateMaintainer::LayoutStateMaintainer): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderView.h

index 47a18fe..6442409 100644 (file)
@@ -1,3 +1,23 @@
+2015-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        RenderBlock::simplifiedLayout should pop LayoutStateMaintainer when early returns.
+        https://bugs.webkit.org/show_bug.cgi?id=148031
+
+        Reviewed by Simon Fraser.
+
+        LayoutStateMaintainer push/pop calls need to be balanced to ensure layout consistency.
+
+        Unable to make a test case for this.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::simplifiedLayout):
+        * rendering/RenderView.h:
+        (WebCore::LayoutStateMaintainer::~LayoutStateMaintainer): ASSERT the state properly.
+        (WebCore::LayoutStateMaintainer::push):
+        (WebCore::LayoutStateMaintainer::pop):
+        (WebCore::LayoutStateMaintainer::didPush):
+        (WebCore::LayoutStateMaintainer::LayoutStateMaintainer): Deleted.
+
 2015-08-14  Simon Fraser  <simon.fraser@apple.com>
 
         Remove a few includes from RenderObject.h
index be7b280..585b650 100644 (file)
@@ -1205,9 +1205,10 @@ bool RenderBlock::simplifiedLayout()
         return false;
 
     LayoutStateMaintainer statePusher(view(), *this, locationOffset(), hasTransform() || hasReflection() || style().isFlippedBlocksWritingMode());
-    
-    if (needsPositionedMovementLayout() && !tryLayoutDoingPositionedMovementOnly())
+    if (needsPositionedMovementLayout() && !tryLayoutDoingPositionedMovementOnly()) {
+        statePusher.pop();
         return false;
+    }
 
     // Lay out positioned descendants or objects that just need to recompute overflow.
     if (needsSimplifiedNormalFlowLayout())
index 948109d..c335111 100644 (file)
@@ -387,9 +387,6 @@ public:
     explicit LayoutStateMaintainer(RenderView& view, RenderBox& root, LayoutSize offset, bool disableState = false, LayoutUnit pageHeight = 0, bool pageHeightChanged = false)
         : m_view(view)
         , m_disabled(disableState)
-        , m_didStart(false)
-        , m_didEnd(false)
-        , m_didCreateLayoutState(false)
     {
         push(root, offset, pageHeight, pageHeightChanged);
     }
@@ -397,50 +394,47 @@ public:
     // Constructor to maybe push later.
     explicit LayoutStateMaintainer(RenderView& view)
         : m_view(view)
-        , m_disabled(false)
-        , m_didStart(false)
-        , m_didEnd(false)
-        , m_didCreateLayoutState(false)
     {
     }
 
     ~LayoutStateMaintainer()
     {
-        ASSERT(m_didStart == m_didEnd);   // if this fires, it means that someone did a push(), but forgot to pop().
+        ASSERT(!m_didCallPush || m_didCallPush == m_didCallPop);
     }
 
     void push(RenderBox& root, LayoutSize offset, LayoutUnit pageHeight = 0, bool pageHeightChanged = false)
     {
-        ASSERT(!m_didStart);
+        ASSERT(!m_didCallPush);
+        m_didCallPush = true;
         // We push state even if disabled, because we still need to store layoutDelta
-        m_didCreateLayoutState = m_view.pushLayoutState(root, offset, pageHeight, pageHeightChanged);
-        if (m_disabled && m_didCreateLayoutState)
+        m_didPushLayoutState = m_view.pushLayoutState(root, offset, pageHeight, pageHeightChanged);
+        if (!m_didPushLayoutState)
+            return;
+        if (m_disabled)
             m_view.disableLayoutState();
-        m_didStart = true;
     }
 
     void pop()
     {
-        if (m_didStart) {
-            ASSERT(!m_didEnd);
-            if (m_didCreateLayoutState) {
-                m_view.popLayoutState();
-                if (m_disabled)
-                    m_view.enableLayoutState();
-            }
-            
-            m_didEnd = true;
-        }
+        ASSERT(!m_didCallPop);
+        m_didCallPop = true;
+        if (!m_didCallPush)
+            return;
+        if (!m_didPushLayoutState)
+            return;
+        m_view.popLayoutState();
+        if (m_disabled)
+            m_view.enableLayoutState();
     }
 
-    bool didPush() const { return m_didStart; }
+    bool didPush() const { return m_didCallPush; }
 
 private:
     RenderView& m_view;
-    bool m_disabled : 1;        // true if the offset and clip part of layoutState is disabled
-    bool m_didStart : 1;        // true if we did a push or disable
-    bool m_didEnd : 1;          // true if we popped or re-enabled
-    bool m_didCreateLayoutState : 1; // true if we actually made a layout state.
+    bool m_disabled { false };
+    bool m_didCallPush { false };
+    bool m_didCallPop { false };
+    bool m_didPushLayoutState { false };
 };
 
 class LayoutStateDisabler {