- Revision
- 238431
- Author
- [email protected]
- Date
- 2018-11-21 15:25:13 -0800 (Wed, 21 Nov 2018)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (238430 => 238431)
--- trunk/Source/WebCore/ChangeLog 2018-11-21 23:13:22 UTC (rev 238430)
+++ trunk/Source/WebCore/ChangeLog 2018-11-21 23:25:13 UTC (rev 238431)
@@ -1,5 +1,26 @@
2018-11-21 Zalan Bujtas <[email protected]>
+ [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 <[email protected]>
+
[LFC][IFC] Horizontal margins should be considered as non-breakable space
https://bugs.webkit.org/show_bug.cgi?id=191894
Modified: trunk/Source/WebCore/layout/LayoutFormattingState.cpp (238430 => 238431)
--- trunk/Source/WebCore/layout/LayoutFormattingState.cpp 2018-11-21 23:13:22 UTC (rev 238430)
+++ trunk/Source/WebCore/layout/LayoutFormattingState.cpp 2018-11-21 23:25:13 UTC (rev 238431)
@@ -45,18 +45,14 @@
WTF_MAKE_ISO_ALLOCATED_IMPL(LayoutState);
-LayoutState::LayoutState()
+LayoutState::LayoutState(const Container& initialContainingBlock, const LayoutSize& containerSize)
+ : m_initialContainingBlock(makeWeakPtr(initialContainingBlock))
{
-}
+ // LayoutState is always initiated with the ICB.
+ ASSERT(!initialContainingBlock.parent());
+ ASSERT(initialContainingBlock.establishesBlockFormattingContext());
-void LayoutState::initializeRoot(const Container& root, const LayoutSize& containerSize)
-{
- ASSERT(root.establishesFormattingContext());
-
- 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({ });
@@ -63,11 +59,11 @@
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()
Modified: trunk/Source/WebCore/layout/LayoutFormattingState.h (238430 => 238431)
--- trunk/Source/WebCore/layout/LayoutFormattingState.h 2018-11-21 23:13:22 UTC (rev 238430)
+++ trunk/Source/WebCore/layout/LayoutFormattingState.h 2018-11-21 23:25:13 UTC (rev 238431)
@@ -49,9 +49,9 @@
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.
@@ -58,9 +58,8 @@
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 @@
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;
Modified: trunk/Source/WebCore/layout/Verification.cpp (238430 => 238431)
--- trunk/Source/WebCore/layout/Verification.cpp 2018-11-21 23:13:22 UTC (rev 238430)
+++ trunk/Source/WebCore/layout/Verification.cpp 2018-11-21 23:25:13 UTC (rev 238431)
@@ -321,12 +321,12 @@
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();
Modified: trunk/Source/WebCore/page/FrameViewLayoutContext.cpp (238430 => 238431)
--- trunk/Source/WebCore/page/FrameViewLayoutContext.cpp 2018-11-21 23:13:22 UTC (rev 238430)
+++ trunk/Source/WebCore/page/FrameViewLayoutContext.cpp 2018-11-21 23:25:13 UTC (rev 238431)
@@ -56,8 +56,7 @@
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);
