[LayoutState cleanup] Remove conditional push from RenderTableSection::calcRowLogical...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Nov 2017 20:22:10 +0000 (20:22 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Nov 2017 20:22:10 +0000 (20:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179493
<rdar://problem/35446631>

Reviewed by Antti Koivisto.

Pushing layout states is cheap and we do it for every container anyway.

Covered by existing tests.

* rendering/LayoutState.cpp:
(WebCore::LayoutStateMaintainer::LayoutStateMaintainer):
(WebCore::LayoutStateMaintainer::~LayoutStateMaintainer):
(WebCore::LayoutStateMaintainer::pop):
(WebCore::LayoutStateMaintainer::push): Deleted.
* rendering/LayoutState.h:
(WebCore::LayoutStateMaintainer::didPush const): Deleted.
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::calcRowLogicalHeight):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/LayoutState.cpp
Source/WebCore/rendering/LayoutState.h
Source/WebCore/rendering/RenderTableSection.cpp

index 91cfff3..0a474ac 100644 (file)
@@ -1,3 +1,25 @@
+2017-11-09  Zalan Bujtas  <zalan@apple.com>
+
+        [LayoutState cleanup] Remove conditional push from RenderTableSection::calcRowLogicalHeight
+        https://bugs.webkit.org/show_bug.cgi?id=179493
+        <rdar://problem/35446631>
+
+        Reviewed by Antti Koivisto.
+
+        Pushing layout states is cheap and we do it for every container anyway.
+
+        Covered by existing tests.
+
+        * rendering/LayoutState.cpp:
+        (WebCore::LayoutStateMaintainer::LayoutStateMaintainer):
+        (WebCore::LayoutStateMaintainer::~LayoutStateMaintainer):
+        (WebCore::LayoutStateMaintainer::pop):
+        (WebCore::LayoutStateMaintainer::push): Deleted.
+        * rendering/LayoutState.h:
+        (WebCore::LayoutStateMaintainer::didPush const): Deleted.
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::calcRowLogicalHeight):
+
 2017-11-09  Youenn Fablet  <youenn@apple.com>
 
         CachedResourceLoader::requestRawResource should not assert when destination is set in the context of a service worker
index 534e810..d887b92 100644 (file)
@@ -276,7 +276,9 @@ LayoutStateMaintainer::LayoutStateMaintainer(RenderBox& root, LayoutSize offset,
     : m_context(root.view().frameView().layoutContext())
     , m_paintOffsetCacheIsDisabled(disablePaintOffsetCache)
 {
-    push(root, offset, pageHeight, pageHeightChanged);
+    m_didPushLayoutState = m_context.pushLayoutState(root, offset, pageHeight, pageHeightChanged);
+    if (m_didPushLayoutState && m_paintOffsetCacheIsDisabled)
+        m_context.disablePaintOffsetCache();
 }
 
 LayoutStateMaintainer::LayoutStateMaintainer(LayoutContext& context)
@@ -286,30 +288,15 @@ LayoutStateMaintainer::LayoutStateMaintainer(LayoutContext& context)
 
 LayoutStateMaintainer::~LayoutStateMaintainer()
 {
-    // FIXME: Remove conditional push/pop.
-    if (m_didCallPush && !m_didCallPop)
+    // FIXME: Remove conditional pop.
+    if (!m_didCallPop)
         pop();
-    ASSERT(!m_didCallPush || m_didCallPush == m_didCallPop);
-}
-
-void LayoutStateMaintainer::push(RenderBox& root, LayoutSize offset, LayoutUnit pageHeight, bool pageHeightChanged)
-{
-    ASSERT(!m_didCallPush);
-    m_didCallPush = true;
-    // We push state even if disabled, because we still need to store layoutDelta
-    m_didPushLayoutState = m_context.pushLayoutState(root, offset, pageHeight, pageHeightChanged);
-    if (!m_didPushLayoutState)
-        return;
-    if (m_paintOffsetCacheIsDisabled)
-        m_context.disablePaintOffsetCache();
 }
 
 void LayoutStateMaintainer::pop()
 {
     ASSERT(!m_didCallPop);
     m_didCallPop = true;
-    if (!m_didCallPush)
-        return;
     if (!m_didPushLayoutState)
         return;
     m_context.popLayoutState();
index 17862dd..0a4ccf8 100644 (file)
@@ -144,14 +144,11 @@ public:
     explicit LayoutStateMaintainer(LayoutContext&);
     ~LayoutStateMaintainer();
 
-    void push(RenderBox& root, LayoutSize offset, LayoutUnit pageHeight = 0, bool pageHeightChanged = false);
     void pop();
-    bool didPush() const { return m_didCallPush; }
 
 private:
     LayoutContext& m_context;
     bool m_paintOffsetCacheIsDisabled { false };
-    bool m_didCallPush { false };
     bool m_didCallPop { false };
     bool m_didPushLayoutState { false };
 };
index c4ca554..d71d473 100644 (file)
@@ -278,7 +278,7 @@ LayoutUnit RenderTableSection::calcRowLogicalHeight()
     if (this == table()->topSection())
         spacing = table()->vBorderSpacing();
 
-    LayoutStateMaintainer statePusher(view().frameView().layoutContext());
+    LayoutStateMaintainer statePusher(*this, locationOffset(), hasTransform() || hasReflection() || style().isFlippedBlocksWritingMode());
 
     m_rowPos.resize(m_grid.size() + 1);
     m_rowPos[0] = spacing;
@@ -329,11 +329,6 @@ LayoutUnit RenderTableSection::calcRowLogicalHeight()
                 unsigned cellStartRow = cell->rowIndex();
 
                 if (cell->hasOverrideLogicalContentHeight()) {
-                    if (!statePusher.didPush()) {
-                        // Technically, we should also push state for the row, but since
-                        // rows don't push a coordinate transform, that's not necessary.
-                        statePusher.push(*this, locationOffset());
-                    }
                     cell->clearIntrinsicPadding();
                     cell->clearOverrideSize();
                     cell->setChildNeedsLayout(MarkOnlyThis);