[LFC] LayoutState should always be initialized with the initial conta…

…ining block.

https://bugs.webkit.org/show_bug.cgi?id=191896

Reviewed by Antti Koivisto.

There should always be only one LayoutState per layout tree (it does not mean that layout always starts at the ICB).
The ICB is a special formatting context root because it does not have a parent formatting context. All the other formatting contexts
first need to be laid out (partially at least e.g margin) in their parent formatting context.
Having a non-null parent formatting context as root could lead to undefined behaviour.

* layout/LayoutFormattingState.cpp:
(WebCore::Layout::LayoutState::LayoutState):
(WebCore::Layout::LayoutState::initializeRoot): Deleted.
* layout/LayoutFormattingState.h:
* layout/Verification.cpp:
(WebCore::Layout::LayoutState::verifyAndOutputMismatchingLayoutTree const):
* page/FrameViewLayoutContext.cpp:
(WebCore::layoutUsingFormattingContext):

Canonical link: https://commits.webkit.org/206601@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238431 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,24 @@
+2018-11-21  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC] LayoutState should always be initialized with the initial containing block.
+        https://bugs.webkit.org/show_bug.cgi?id=191896
+
+        Reviewed by Antti Koivisto.
+
+        There should always be only one LayoutState per layout tree (it does not mean that layout always starts at the ICB).
+        The ICB is a special formatting context root because it does not have a parent formatting context. All the other formatting contexts
+        first need to be laid out (partially at least e.g margin) in their parent formatting context.
+        Having a non-null parent formatting context as root could lead to undefined behaviour.
+
+        * layout/LayoutFormattingState.cpp:
+        (WebCore::Layout::LayoutState::LayoutState):
+        (WebCore::Layout::LayoutState::initializeRoot): Deleted.
+        * layout/LayoutFormattingState.h:
+        * layout/Verification.cpp:
+        (WebCore::Layout::LayoutState::verifyAndOutputMismatchingLayoutTree const):
+        * page/FrameViewLayoutContext.cpp:
+        (WebCore::layoutUsingFormattingContext):
+
 2018-11-21  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Horizontal margins should be considered as non-breakable space

Source/WebCore/layout/LayoutFormattingState.cpp

@@ -45,29 +45,25 @@ namespace Layout {
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(LayoutState);
 
-LayoutState::LayoutState()
+LayoutState::LayoutState(const Container& initialContainingBlock, const LayoutSize& containerSize)
+    : m_initialContainingBlock(makeWeakPtr(initialContainingBlock))
 {
-}
-
-void LayoutState::initializeRoot(const Container& root, const LayoutSize& containerSize)
-{
-    ASSERT(root.establishesFormattingContext());
+    // LayoutState is always initiated with the ICB.
+    ASSERT(!initialContainingBlock.parent());
+    ASSERT(initialContainingBlock.establishesBlockFormattingContext());
 
-    m_root = makeWeakPtr(root);
-    auto& displayBox = displayBoxForLayoutBox(root);
-
-    // FIXME: m_root could very well be a formatting context root with ancestors and resolvable border and padding (as opposed to the topmost root)
+    auto& displayBox = displayBoxForLayoutBox(initialContainingBlock);
     displayBox.setHorizontalMargin({ });
     displayBox.setHorizontalNonComputedMargin({ });
     displayBox.setVerticalMargin({ });
     displayBox.setVerticalNonCollapsedMargin({ });
     displayBox.setBorder({ });
     displayBox.setPadding({ });
+    displayBox.setTopLeft({ });
     displayBox.setContentBoxHeight(containerSize.height());
     displayBox.setContentBoxWidth(containerSize.width());
-    displayBox.setTopLeft({ });
 
-    m_formattingContextRootListForLayout.add(&root);
+    m_formattingContextRootListForLayout.add(&initialContainingBlock);
 }
 
 void LayoutState::updateLayout()

Source/WebCore/layout/LayoutFormattingState.h

@@ -49,18 +49,17 @@ class Box;
 class Container;
 class FormattingState;
 
-// LayoutState is the entry point for layout. It takes a (formatting root)container which acts as the root of the layout context.
+// LayoutState is the entry point for layout. It takes the initial containing block which acts as the root of the layout context.
 // LayoutState::layout() generates the display tree for the root container's subtree (it does not run layout on the root though).
-// Note, while the root container is suppposed to be the entry point for the initial layout, it does not necessarily need to be the entry point of any
+// Note, while the initial containing block is entry point for the initial layout, it does not necessarily need to be the entry point of any
 // subsequent layouts (subtree layout). A non-initial, subtree layout could be initiated on multiple formatting contexts.
 // Each formatting context has an entry point for layout, which potenitally means multiple entry points per layout frame.
 // LayoutState also holds the formatting states. They cache formatting context specific data to enable performant incremental layouts.
 class LayoutState {
     WTF_MAKE_ISO_ALLOCATED(LayoutState);
 public:
-    LayoutState();
+    LayoutState(const Container& initialContainingBlock, const LayoutSize&);
 
-    void initializeRoot(const Container&, const LayoutSize&);
     void updateLayout();
     void styleChanged(const Box&, StyleDiff);
     void setInQuirksMode(bool inQuirksMode) { m_inQuirksMode = inQuirksMode; }
@@ -86,9 +85,10 @@ class LayoutState {
     void verifyAndOutputMismatchingLayoutTree(const RenderView&) const;
 
 private:
+    const Container& initialContainingBlock() const { return *m_initialContainingBlock; }
     void layoutFormattingContextSubtree(const Box&);
 
-    WeakPtr<const Container> m_root;
+    WeakPtr<const Container> m_initialContainingBlock;
     HashSet<const Container*> m_formattingContextRootListForLayout;
     HashMap<const Box*, std::unique_ptr<FormattingState>> m_formattingStates;
     mutable HashMap<const Box*, std::unique_ptr<Display::Box>> m_layoutToDisplayBox;

Source/WebCore/layout/Verification.cpp

@@ -321,12 +321,12 @@ static bool verifyAndOutputSubtree(TextStream& stream, const LayoutState& contex
 void LayoutState::verifyAndOutputMismatchingLayoutTree(const RenderView& renderView) const
 {
     TextStream stream;
-    auto mismatchingGeometry = verifyAndOutputSubtree(stream, *this, renderView, *m_root.get());
+    auto mismatchingGeometry = verifyAndOutputSubtree(stream, *this, renderView, initialContainingBlock());
     if (!mismatchingGeometry)
         return;
 #if ENABLE(TREE_DEBUGGING)
     showRenderTree(&renderView);
-    showLayoutTree(*m_root.get(), this);
+    showLayoutTree(initialContainingBlock(), this);
 #endif
     WTFLogAlways("%s", stream.release().utf8().data());
     ASSERT_NOT_REACHED();

Source/WebCore/page/FrameViewLayoutContext.cpp

@@ -56,8 +56,7 @@ namespace WebCore {
 static void layoutUsingFormattingContext(const RenderView& renderView)
 {
     auto initialContainingBlock = Layout::TreeBuilder::createLayoutTree(renderView);
-    auto layoutState = std::make_unique<Layout::LayoutState>();
-    layoutState->initializeRoot(*initialContainingBlock, renderView.size());
+    auto layoutState = std::make_unique<Layout::LayoutState>(*initialContainingBlock, renderView.size());
     layoutState->setInQuirksMode(renderView.document().inQuirksMode());
     layoutState->updateLayout();
     layoutState->verifyAndOutputMismatchingLayoutTree(renderView);