[LFC] Do not use virtual methods to construct floating/formatting states.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jul 2018 18:47:01 +0000 (18:47 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jul 2018 18:47:01 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187875

Reviewed by Antti Koivisto.

LayoutContext::establishedFormattingState() does not require FormattingContext anymore only the root of the context.

* layout/FormattingContext.cpp:
(WebCore::Layout::FormattingContext::layoutOutOfFlowDescendants const):
* layout/FormattingContext.h:
* layout/LayoutContext.cpp:
(WebCore::Layout::LayoutContext::layoutFormattingContextSubtree):
(WebCore::Layout::LayoutContext::formattingStateForBox const):
(WebCore::Layout::LayoutContext::establishedFormattingState):
* layout/LayoutContext.h:
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
(WebCore::Layout::BlockFormattingContext::instrinsicWidthConstraints const):
(WebCore::Layout::BlockFormattingContext::createFormattingState const): Deleted.
(WebCore::Layout::BlockFormattingContext::createOrFindFloatingState const): Deleted.
* layout/blockformatting/BlockFormattingContext.h:
* layout/inlineformatting/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::createFormattingState const): Deleted.
(WebCore::Layout::InlineFormattingContext::createOrFindFloatingState const): Deleted.
* layout/inlineformatting/InlineFormattingContext.h:

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

Source/WebCore/ChangeLog
Source/WebCore/layout/FormattingContext.cpp
Source/WebCore/layout/FormattingContext.h
Source/WebCore/layout/LayoutContext.cpp
Source/WebCore/layout/LayoutContext.h
Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContext.h
Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
Source/WebCore/layout/inlineformatting/InlineFormattingContext.h

index 37e1380..66ac5f3 100644 (file)
@@ -1,5 +1,33 @@
 2018-07-21  Zalan Bujtas  <zalan@apple.com>
 
+        [LFC] Do not use virtual methods to construct floating/formatting states.
+        https://bugs.webkit.org/show_bug.cgi?id=187875
+
+        Reviewed by Antti Koivisto.
+
+        LayoutContext::establishedFormattingState() does not require FormattingContext anymore only the root of the context.
+
+        * layout/FormattingContext.cpp:
+        (WebCore::Layout::FormattingContext::layoutOutOfFlowDescendants const):
+        * layout/FormattingContext.h:
+        * layout/LayoutContext.cpp:
+        (WebCore::Layout::LayoutContext::layoutFormattingContextSubtree):
+        (WebCore::Layout::LayoutContext::formattingStateForBox const):
+        (WebCore::Layout::LayoutContext::establishedFormattingState):
+        * layout/LayoutContext.h:
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
+        (WebCore::Layout::BlockFormattingContext::instrinsicWidthConstraints const):
+        (WebCore::Layout::BlockFormattingContext::createFormattingState const): Deleted.
+        (WebCore::Layout::BlockFormattingContext::createOrFindFloatingState const): Deleted.
+        * layout/blockformatting/BlockFormattingContext.h:
+        * layout/inlineformatting/InlineFormattingContext.cpp:
+        (WebCore::Layout::InlineFormattingContext::createFormattingState const): Deleted.
+        (WebCore::Layout::InlineFormattingContext::createOrFindFloatingState const): Deleted.
+        * layout/inlineformatting/InlineFormattingContext.h:
+
+2018-07-21  Zalan Bujtas  <zalan@apple.com>
+
         [LFC][BFC] Do not collapse top/bottom margin with first/last inflow child from a non-block formatting context.
         https://bugs.webkit.org/show_bug.cgi?id=187867
 
index 2b2d747..f9604ea 100644 (file)
@@ -130,12 +130,11 @@ void FormattingContext::layoutOutOfFlowDescendants(LayoutContext& layoutContext,
 
         ASSERT(layoutBox.establishesFormattingContext());
         auto formattingContext = layoutContext.formattingContext(layoutBox);
-        auto& establishedFormattingState = layoutContext.establishedFormattingState(layoutBox, *formattingContext);
 
         computeBorderAndPadding(layoutContext, layoutBox, displayBox);
         computeOutOfFlowHorizontalGeometry(layoutContext, layoutBox, displayBox);
 
-        formattingContext->layout(layoutContext, establishedFormattingState);
+        formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox));
 
         computeOutOfFlowVerticalGeometry(layoutContext, layoutBox, displayBox);
         layoutOutOfFlowDescendants(layoutContext, layoutBox);
index 1f2c150..de4edf7 100644 (file)
@@ -52,8 +52,6 @@ public:
 
     virtual void layout(LayoutContext&, FormattingState&) const = 0;
     void layoutOutOfFlowDescendants(LayoutContext&, const Box&) const;
-    virtual std::unique_ptr<FormattingState> createFormattingState(Ref<FloatingState>&&, const LayoutContext&) const = 0;
-    virtual Ref<FloatingState> createOrFindFloatingState(LayoutContext&) const = 0;
 
     struct InstrinsicWidthConstraints {
         LayoutUnit minimum;
index 8c44ea6..8ca9c9b 100644 (file)
@@ -81,8 +81,7 @@ void LayoutContext::layoutFormattingContextSubtree(const Box& layoutRoot)
 {
     RELEASE_ASSERT(layoutRoot.establishesFormattingContext());
     auto formattingContext = this->formattingContext(layoutRoot);
-    auto& formattingState = establishedFormattingState(layoutRoot, *formattingContext);
-    formattingContext->layout(*this, formattingState);
+    formattingContext->layout(*this, establishedFormattingState(layoutRoot));
     formattingContext->layoutOutOfFlowDescendants(*this, layoutRoot);
 }
 
@@ -119,11 +118,31 @@ FormattingState& LayoutContext::formattingStateForBox(const Box& layoutBox) cons
     return *m_formattingStates.get(&root);
 }
 
-FormattingState& LayoutContext::establishedFormattingState(const Box& formattingContextRoot, const FormattingContext& context)
+FormattingState& LayoutContext::establishedFormattingState(const Box& formattingRoot)
 {
-    return *m_formattingStates.ensure(&formattingContextRoot, [this, &context] {
-        return context.createFormattingState(context.createOrFindFloatingState(*this), *this);
-    }).iterator->value;
+    if (formattingRoot.establishesInlineFormattingContext()) {
+        return *m_formattingStates.ensure(&formattingRoot, [&] {
+
+            // If the block container box that initiates this inline formatting context also establishes a block context, the floats outside of the formatting root
+            // should not interfere with the content inside.
+            // <div style="float: left"></div><div style="overflow: hidden"> <- is a non-intrusive float, because overflow: hidden triggers new block formatting context.</div>
+            if (formattingRoot.establishesBlockFormattingContext())
+                return std::make_unique<InlineFormattingState>(FloatingState::create(), *this);
+
+            // Otherwise, the formatting context inherits the floats from the parent formatting context.
+            // Find the formatting state in which this formatting root lives, not the one it creates and use its floating state.
+            return std::make_unique<InlineFormattingState>(formattingStateForBox(formattingRoot).floatingState(), *this);
+        }).iterator->value;
+    }
+
+    if (formattingRoot.establishesBlockFormattingContext()) {
+        return *m_formattingStates.ensure(&formattingRoot, [&] {
+
+            // Block formatting context always establishes a new floating state.
+            return std::make_unique<BlockFormattingState>(FloatingState::create(), *this);
+        }).iterator->value;
+    }
+    CRASH();
 }
 
 std::unique_ptr<FormattingContext> LayoutContext::formattingContext(const Box& formattingContextRoot)
index 9928d55..dc4cdb0 100644 (file)
@@ -74,10 +74,11 @@ public:
     void markNeedsUpdate(const Box&, OptionSet<UpdateType>);
     bool needsUpdate(const Box&) const;
 
-    FormattingState& formattingStateForBox(const Box&) const;
-    FormattingState& establishedFormattingState(const Box& formattingContextRoot, const FormattingContext&);
     std::unique_ptr<FormattingContext> formattingContext(const Box& formattingContextRoot);
 
+    FormattingState& formattingStateForBox(const Box&) const;
+    FormattingState& establishedFormattingState(const Box& formattingRoot);
+
     Display::Box& createDisplayBox(const Box&);
     Display::Box* displayBoxForLayoutBox(const Box& layoutBox) const { return m_layoutToDisplayBox.get(&layoutBox); }
 
index 8e86ab3..1933224 100644 (file)
@@ -141,8 +141,7 @@ void BlockFormattingContext::layoutFormattingContextRoot(LayoutContext& layoutCo
 
     // Swich over to the new formatting context (the one that the root creates).
     auto formattingContext = layoutContext.formattingContext(layoutBox);
-    auto& establishedFormattingState = layoutContext.establishedFormattingState(layoutBox, *formattingContext);
-    formattingContext->layout(layoutContext, establishedFormattingState);
+    formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox));
 
     // Come back and finalize the root's geometry.
     FloatingContext(formattingState.floatingState()).computePosition(layoutBox, displayBox);
@@ -152,17 +151,6 @@ void BlockFormattingContext::layoutFormattingContextRoot(LayoutContext& layoutCo
     formattingContext->layoutOutOfFlowDescendants(layoutContext, layoutBox);
 }
 
-std::unique_ptr<FormattingState> BlockFormattingContext::createFormattingState(Ref<FloatingState>&& floatingState, const LayoutContext& layoutContext) const
-{
-    return std::make_unique<BlockFormattingState>(WTFMove(floatingState), layoutContext);
-}
-
-Ref<FloatingState> BlockFormattingContext::createOrFindFloatingState(LayoutContext&) const
-{
-    // Block formatting context always establishes a new floating state.
-    return FloatingState::create();
-}
-
 void BlockFormattingContext::computeStaticPosition(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
 {
     displayBox.setTopLeft(Geometry::staticPosition(layoutContext, layoutBox));
@@ -237,7 +225,7 @@ FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsic
     if (auto* firstChild = downcast<Container>(layoutBox).firstInFlowOrFloatingChild())
         queue.append(firstChild);
 
-    auto& formattingStateForChildren = layoutBox.establishesFormattingContext() ? layoutContext.establishedFormattingState(layoutBox, *this) : formattingState;
+    auto& formattingStateForChildren = layoutBox.establishesFormattingContext() ? layoutContext.establishedFormattingState(layoutBox) : formattingState;
     while (!queue.isEmpty()) {
         while (true) {
             auto& childBox = *queue.last(); 
index bc845cf..4d4fb9e 100644 (file)
@@ -47,8 +47,6 @@ public:
     BlockFormattingContext(const Box& formattingContextRoot);
 
     void layout(LayoutContext&, FormattingState&) const override;
-    std::unique_ptr<FormattingState> createFormattingState(Ref<FloatingState>&&, const LayoutContext&) const override;
-    Ref<FloatingState> createOrFindFloatingState(LayoutContext&) const override;
 
 private:
     void layoutFormattingContextRoot(LayoutContext&, FormattingState&, const Box&, Display::Box&) const;
index dbe7bdf..1928fcb 100644 (file)
@@ -99,24 +99,6 @@ void InlineFormattingContext::layout(LayoutContext& layoutContext, FormattingSta
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[End] -> inline formatting context -> layout context(" << &layoutContext << ") formatting root(" << &root() << ")");
 }
 
-std::unique_ptr<FormattingState> InlineFormattingContext::createFormattingState(Ref<FloatingState>&& floatingState, const LayoutContext& layoutContext) const
-{
-    return std::make_unique<InlineFormattingState>(WTFMove(floatingState), layoutContext);
-}
-
-Ref<FloatingState> InlineFormattingContext::createOrFindFloatingState(LayoutContext& layoutContext) const
-{
-    // If the block container box that initiates this inline formatting context also establishes a block context, the floats outside of the formatting root
-    // should not interfere with the content inside.
-    // <div style="float: left"></div><div style="overflow: hidden"> <- is a non-intrusive float, because overflow: hidden triggers new block formatting context.</div>
-    if (root().establishesBlockFormattingContext())
-        return FloatingState::create();
-    // Otherwise, the formatting context inherits the floats from the parent formatting context.
-    // Find the formatting state in which this formatting root lives, not the one it creates (this) and use its floating state.
-    auto& formattingState = layoutContext.formattingStateForBox(root());
-    return formattingState.floatingState();
-}
-
 void InlineFormattingContext::computeStaticPosition(LayoutContext&, const Box&, Display::Box&) const
 {
 }
index 42a344e..d704335 100644 (file)
@@ -44,8 +44,6 @@ public:
     InlineFormattingContext(const Box& formattingContextRoot);
 
     void layout(LayoutContext&, FormattingState&) const override;
-    std::unique_ptr<FormattingState> createFormattingState(Ref<FloatingState>&&, const LayoutContext&) const override;
-    Ref<FloatingState> createOrFindFloatingState(LayoutContext&) const override;
 
 private:
     void computeStaticPosition(LayoutContext&, const Box&, Display::Box&) const override;