Attachment 358991 Details for Bug 193381 – Patch

[patch] Patch

bug-193381-20190112084302.patch (text/plain), 15.00 KB, created by zalan on 2019-01-12 08:43:15 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-12 08:43:15 PST

Size: 15.00 KB

patch

obsolete

>Subversion Revision: 239899
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f56c34fe22173dcc0f9c50a9bc3e421f85dc07f5..0565a88b76be1d64e5b3d3e96704ef577e43eaa3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2019-01-12  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Move formatting context creation from FormattingState to LayoutState
>+        https://bugs.webkit.org/show_bug.cgi?id=193381
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        layoutState().createFormattingStateForFormattingRootIfNeeded(root).createFormattingContext(root) is not only mouthful
>+        but also feels unintuitive. Use layoutState().createFormattingContext(root) instead.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::FormattingContext):
>+        (WebCore::Layout::FormattingContext::~FormattingContext):
>+        (WebCore::Layout::FormattingContext::layoutOutOfFlowDescendants const):
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::shrinkToFitWidth):
>+        * layout/FormattingState.h:
>+        * layout/LayoutState.cpp:
>+        (WebCore::Layout::LayoutState::layoutFormattingContextSubtree):
>+        (WebCore::Layout::LayoutState::createFormattingContext):
>+        * layout/LayoutState.h:
>+        (WebCore::Layout::LayoutState::deregisterFormattingContext):
>+        (WebCore::Layout::LayoutState::registerFormattingContext):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
>+        (WebCore::Layout::BlockFormattingContext::instrinsicWidthConstraints const):
>+        * layout/blockformatting/BlockFormattingState.cpp:
>+        (WebCore::Layout::BlockFormattingState::createFormattingContext): Deleted.
>+        * layout/blockformatting/BlockFormattingState.h:
>+        * layout/inlineformatting/InlineFormattingContext.cpp:
>+        (WebCore::Layout::InlineFormattingContext::layoutFormattingContextRoot const):
>+        * layout/inlineformatting/InlineFormattingState.cpp:
>+        (WebCore::Layout::InlineFormattingState::createFormattingContext): Deleted.
>+        * layout/inlineformatting/InlineFormattingState.h:
>+
> 2019-01-12  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Move estimatedMarginBefore flag from state/display box to BlockFormattingContext
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index a767e93e95f70e73ca838484abdc8a58c0fe4ebf..d5a83ed15d1b576d2602782c0886ba7a8431f574 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -47,10 +47,16 @@ FormattingContext::FormattingContext(const Box& formattingContextRoot, Formattin
>     : m_root(makeWeakPtr(formattingContextRoot))
>     , m_formattingState(formattingState)
> {
>+#ifndef NDEBUG
>+    layoutState().registerFormattingContext(*this);
>+#endif
> }
> 
> FormattingContext::~FormattingContext()
> {
>+#ifndef NDEBUG
>+    layoutState().deregisterFormattingContext(*this);
>+#endif
> }
> 
> FormattingState& FormattingContext::formattingState() const
>@@ -154,7 +160,7 @@ void FormattingContext::layoutOutOfFlowDescendants(const Box& layoutBox) const
>         computeBorderAndPadding(layoutBox);
>         computeOutOfFlowHorizontalGeometry(layoutBox);
> 
>-        layoutState.createFormattingStateForFormattingRootIfNeeded(layoutBox).createFormattingContext(layoutBox)->layout();
>+        layoutState.createFormattingContext(layoutBox)->layout();
> 
>         computeOutOfFlowVerticalGeometry(layoutBox);
>         layoutOutOfFlowDescendants(layoutBox);
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 75e7b7ee69c658e01fe4d96b68d420e63364aa6d..deabb87b27851cecdbac83604809bc6059850fc2 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -238,8 +238,7 @@ LayoutUnit FormattingContext::Geometry::shrinkToFitWidth(LayoutState& layoutStat
> 
>     // Then the shrink-to-fit width is: min(max(preferred minimum width, available width), preferred width).
>     auto availableWidth = layoutState.displayBoxForLayoutBox(*formattingRoot.containingBlock()).width();
>-    auto& formattingState = layoutState.createFormattingStateForFormattingRootIfNeeded(formattingRoot);
>-    auto instrinsicWidthConstraints = formattingState.createFormattingContext(formattingRoot)->instrinsicWidthConstraints();
>+    auto instrinsicWidthConstraints = layoutState.createFormattingContext(formattingRoot)->instrinsicWidthConstraints();
>     return std::min(std::max(instrinsicWidthConstraints.minimum, availableWidth), instrinsicWidthConstraints.maximum);
> }
> 
>diff --git a/Source/WebCore/layout/FormattingState.h b/Source/WebCore/layout/FormattingState.h
>index 4a839769f1aa1641d6780d2cbb47af4f19ec250f..0c34c70a22262b9787c4b52b499f99e2c5e70406 100644
>--- a/Source/WebCore/layout/FormattingState.h
>+++ b/Source/WebCore/layout/FormattingState.h
>@@ -46,8 +46,6 @@ class FormattingState {
> public:
>     virtual ~FormattingState();
> 
>-    virtual std::unique_ptr<FormattingContext> createFormattingContext(const Box& formattingContextRoot) = 0;
>-
>     FloatingState& floatingState() const { return m_floatingState; }
> 
>     void markNeedsLayout(const Box&, StyleDiff);
>diff --git a/Source/WebCore/layout/LayoutState.cpp b/Source/WebCore/layout/LayoutState.cpp
>index fb644e72ff51782921d39f52f27930b3e31243eb..b4765d288fa971c7b7c124cfe771be52a52f5097 100644
>--- a/Source/WebCore/layout/LayoutState.cpp
>+++ b/Source/WebCore/layout/LayoutState.cpp
>@@ -76,8 +76,7 @@ void LayoutState::updateLayout()
> void LayoutState::layoutFormattingContextSubtree(const Box& layoutRoot)
> {
>     RELEASE_ASSERT(layoutRoot.establishesFormattingContext());
>-    auto& formattingState = createFormattingStateForFormattingRootIfNeeded(layoutRoot);
>-    auto formattingContext = formattingState.createFormattingContext(layoutRoot);
>+    auto formattingContext = createFormattingContext(layoutRoot);
>     formattingContext->layout();
>     formattingContext->layoutOutOfFlowDescendants(layoutRoot);
> }
>@@ -151,6 +150,20 @@ FormattingState& LayoutState::createFormattingStateForFormattingRootIfNeeded(con
>     CRASH();
> }
> 
>+std::unique_ptr<FormattingContext> LayoutState::createFormattingContext(const Box& formattingContextRoot)
>+{
>+    ASSERT(formattingContextRoot.establishesFormattingContext());
>+    if (formattingContextRoot.establishesInlineFormattingContext())
>+        return std::make_unique<InlineFormattingContext>(formattingContextRoot, createFormattingStateForFormattingRootIfNeeded(formattingContextRoot));
>+
>+    if (formattingContextRoot.establishesBlockFormattingContext()) {
>+        ASSERT(formattingContextRoot.establishesBlockFormattingContextOnly());
>+        return std::make_unique<BlockFormattingContext>(formattingContextRoot, createFormattingStateForFormattingRootIfNeeded(formattingContextRoot));
>+    }
>+
>+    CRASH();
>+}
>+
> }
> }
> 
>diff --git a/Source/WebCore/layout/LayoutState.h b/Source/WebCore/layout/LayoutState.h
>index 28446cf9add3e79869861c9cc79c6985bcd9f7ae..26159214f007dce1d36fcda6a48cf09794990fb2 100644
>--- a/Source/WebCore/layout/LayoutState.h
>+++ b/Source/WebCore/layout/LayoutState.h
>@@ -47,6 +47,7 @@ namespace Layout {
> enum class StyleDiff;
> class Box;
> class Container;
>+class FormattingContext;
> class FormattingState;
> 
> // LayoutState is the entry point for layout. It takes the initial containing block which acts as the root of the layout context.
>@@ -78,6 +79,12 @@ public:
>     bool hasFormattingState(const Box& formattingRoot) const { return m_formattingStates.contains(&formattingRoot); }
>     FormattingState& createFormattingStateForFormattingRootIfNeeded(const Box& formattingRoot);
> 
>+    std::unique_ptr<FormattingContext> createFormattingContext(const Box& formattingContextRoot);
>+#ifndef NDEBUG
>+    void registerFormattingContext(const FormattingContext&);
>+    void deregisterFormattingContext(const FormattingContext& formattingContext) { m_formattingContextList.remove(&formattingContext); }
>+#endif
>+
>     Display::Box& displayBoxForLayoutBox(const Box& layoutBox) const;
>     bool hasDisplayBox(const Box& layoutBox) const { return m_layoutToDisplayBox.contains(&layoutBox); }
> 
>@@ -92,10 +99,22 @@ private:
>     WeakPtr<const Container> m_initialContainingBlock;
>     HashSet<const Container*> m_formattingContextRootListForLayout;
>     HashMap<const Box*, std::unique_ptr<FormattingState>> m_formattingStates;
>+#ifndef NDEBUG
>+    HashSet<const FormattingContext*> m_formattingContextList;
>+#endif
>     mutable HashMap<const Box*, std::unique_ptr<Display::Box>> m_layoutToDisplayBox;
>     bool m_inQuirksMode { false };
> };
> 
>+#ifndef NDEBUG
>+inline void LayoutState::registerFormattingContext(const FormattingContext& formattingContext)
>+{
>+    // Multiple formatting contexts of the same root within a layout frame indicates defective layout logic.
>+    ASSERT(!m_formattingContextList.contains(&formattingContext));
>+    m_formattingContextList.add(&formattingContext);
>+}
>+#endif
>+
> }
> }
> #endif
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 15ac3d5b7632b2448bc5e3d89a29b43c0bb4a0e2..2faf75e4b14b43d8cff863b77c7e8c05df45083f 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -134,7 +134,7 @@ void BlockFormattingContext::layoutFormattingContextRoot(FloatingContext& floati
> 
>     precomputeVerticalPositionForFormattingRootIfNeeded(layoutBox);
>     // Swich over to the new formatting context (the one that the root creates).
>-    auto formattingContext = layoutState().createFormattingStateForFormattingRootIfNeeded(layoutBox).createFormattingContext(layoutBox);
>+    auto formattingContext = layoutState().createFormattingContext(layoutBox);
>     formattingContext->layout();
> 
>     // Come back and finalize the root's geometry.
>@@ -436,7 +436,7 @@ FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsic
>                 if (!Geometry::instrinsicWidthConstraintsNeedChildrenWidth(childBox))
>                     instrinsicWidthConstraints = Geometry::instrinsicWidthConstraints(layoutState, childBox);
>                 else if (childBox.establishesFormattingContext())
>-                    instrinsicWidthConstraints = layoutState.createFormattingStateForFormattingRootIfNeeded(childBox).createFormattingContext(childBox)->instrinsicWidthConstraints();
>+                    instrinsicWidthConstraints = layoutState.createFormattingContext(childBox)->instrinsicWidthConstraints();
>                 formattingState.setInstrinsicWidthConstraints(childBox, instrinsicWidthConstraints);
> 
>                 queue.removeLast();
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingState.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingState.cpp
>index 41ac997d6134700ad21a97d5ebb0c885468b2e23..35d8058b787c15e2b008c12337b00661fd611861 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingState.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingState.cpp
>@@ -46,12 +46,6 @@ BlockFormattingState::~BlockFormattingState()
> {
> }
> 
>-std::unique_ptr<FormattingContext> BlockFormattingState::createFormattingContext(const Box& formattingContextRoot)
>-{
>-    ASSERT(formattingContextRoot.establishesBlockFormattingContext());
>-    return std::make_unique<BlockFormattingContext>(formattingContextRoot, *this);
>-}
>-
> }
> }
> #endif
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingState.h b/Source/WebCore/layout/blockformatting/BlockFormattingState.h
>index b28ef695f148c3f91afabd838c98b7776df436ac..78536ad75a64f2069d45d35407a19ff4eb03d8cb 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingState.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingState.h
>@@ -41,8 +41,6 @@ public:
>     BlockFormattingState(Ref<FloatingState>&&, LayoutState&);
>     virtual ~BlockFormattingState();
> 
>-    std::unique_ptr<FormattingContext> createFormattingContext(const Box& formattingContextRoot) override;
>-
>     void setPositiveAndNegativeVerticalMargin(const Box& layoutBox, PositiveAndNegativeVerticalMargin verticalMargin) { m_positiveAndNegativeVerticalMargin.set(&layoutBox, verticalMargin); }
>     bool hasPositiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.contains(&layoutBox); }
>     PositiveAndNegativeVerticalMargin positiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.get(&layoutBox); }
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>index e7e72b79aae85d41d9b10cc585ec029cadf2b2dc..408d7876c332a3ba00a0cd4a0df39f4e1ededf8a 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>@@ -375,7 +375,7 @@ void InlineFormattingContext::layoutFormattingContextRoot(const Box& root) const
>     computeBorderAndPadding(root);
>     computeWidthAndMargin(root);
>     // Swich over to the new formatting context (the one that the root creates).
>-    auto formattingContext = layoutState().createFormattingStateForFormattingRootIfNeeded(root).createFormattingContext(root);
>+    auto formattingContext = layoutState().createFormattingContext(root);
>     formattingContext->layout();
>     // Come back and finalize the root's height and margin.
>     computeHeightAndMargin(root);
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingState.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingState.cpp
>index bae15888b499d9c872c6fe6cb71263adf2212cbb..87b6c95c1545d645cdbd4b0546aec186cf3ccf5d 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingState.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingState.cpp
>@@ -44,12 +44,6 @@ InlineFormattingState::~InlineFormattingState()
> {
> }
> 
>-std::unique_ptr<FormattingContext> InlineFormattingState::createFormattingContext(const Box& formattingContextRoot)
>-{
>-    ASSERT(formattingContextRoot.establishesInlineFormattingContext());
>-    return std::make_unique<InlineFormattingContext>(formattingContextRoot, *this);
>-}
>-
> }
> }
> #endif
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingState.h b/Source/WebCore/layout/inlineformatting/InlineFormattingState.h
>index a4872eb6249fde2019f712c739ee0974dbecc98b..662a83c3ebc51ba7b0e3c4eaccf51844e9ed659c 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingState.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingState.h
>@@ -43,8 +43,6 @@ public:
>     InlineFormattingState(Ref<FloatingState>&&, LayoutState&);
>     virtual ~InlineFormattingState();
> 
>-    std::unique_ptr<FormattingContext> createFormattingContext(const Box& formattingContextRoot) override;
>-
>     InlineContent& inlineContent() { return m_inlineContent; }
>     InlineItem* lastInlineItem() const { return m_inlineContent.isEmpty() ? nullptr : m_inlineContent.last().get(); }
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193381: 358991