Attachment 347984 Details for Bug 188912 – Patch

[patch] Patch

bug-188912-20180823191918.patch (text/plain), 12.82 KB, created by zalan on 2018-08-23 19:19:18 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-23 19:19:18 PDT

Size: 12.82 KB

patch

obsolete

>Subversion Revision: 235228
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f2d9feb7dc124859d254b517dbeba0b171c6d538..1c7d8646dc8a37395e2503065639f2c6666e8136 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-08-23  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Simplify FloatingState::FloatItem class
>+        https://bugs.webkit.org/show_bug.cgi?id=188912
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Let's remove some redundant code now that FloatingState::FloatItem is not used for incoming floats anymore.
>+
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::LayoutContext::verifyAndOutputMismatchingLayoutTree const):
>+        * layout/floats/FloatBox.cpp:
>+        (WebCore::Layout::FloatBox::resetVertically):
>+        * layout/floats/FloatingContext.cpp:
>+        (WebCore::Layout::FloatingPair::left const):
>+        (WebCore::Layout::FloatingPair::right const):
>+        (WebCore::Layout::FloatingPair::intersects const):
>+        (WebCore::Layout::previousFloatingIndex):
>+        (WebCore::Layout::Iterator::operator++):
>+        (WebCore::Layout::Iterator::set):
>+        * layout/floats/FloatingState.cpp:
>+        (WebCore::Layout::FloatingState::FloatItem::FloatItem):
>+        (WebCore::Layout::FloatingState::remove):
>+        (WebCore::Layout::FloatingState::bottom const):
>+        * layout/floats/FloatingState.h:
>+        (WebCore::Layout::FloatingState::FloatItem::operator== const):
>+        (WebCore::Layout::FloatingState::FloatItem::isLeftPositioned const):
>+        (WebCore::Layout::FloatingState::FloatItem::rectWithMargin const):
>+        (WebCore::Layout::FloatingState::FloatItem::bottom const):
>+        (WebCore::Layout::FloatingState::leftBottom const):
>+        (WebCore::Layout::FloatingState::rightBottom const):
>+        (WebCore::Layout::FloatingState::bottom const):
>+        (WebCore::Layout::FloatingState::FloatItem::inFormattingContext const):
>+        (WebCore::Layout::FloatingState::FloatItem::layoutBox const): Deleted.
>+        (WebCore::Layout::FloatingState::FloatItem::containingBlock const): Deleted.
>+        (WebCore::Layout::FloatingState::FloatItem::displayBox const): Deleted.
>+        (WebCore::Layout::FloatingState::FloatItem::containingBlockDisplayBox const): Deleted.
>+
> 2018-08-23  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Decouple the incoming floats and floats already placed in the list
>diff --git a/Source/WebCore/layout/floats/FloatBox.cpp b/Source/WebCore/layout/floats/FloatBox.cpp
>index d4010ce02032bf67f4d03f4e307e8bdebe4516ed..a3815b7dc647e424a3ee14ff61a5311f438e044c 100644
>--- a/Source/WebCore/layout/floats/FloatBox.cpp
>+++ b/Source/WebCore/layout/floats/FloatBox.cpp
>@@ -86,7 +86,7 @@ void FloatBox::resetVertically()
>     // Take the static position (where the box would go if it wasn't floating) and adjust it with the last float.
>     auto top = m_absoluteDisplayBox.rectWithMargin().top();
>     if (auto lastFloat = m_floatingState.last())
>-        top = std::max(top, lastFloat->displayBox().rectWithMargin().top());
>+        top = std::max(top, lastFloat->rectWithMargin().top());
>     top += m_absoluteDisplayBox.marginTop();
> 
>     m_absoluteDisplayBox.setTop(top);
>diff --git a/Source/WebCore/layout/floats/FloatingContext.cpp b/Source/WebCore/layout/floats/FloatingContext.cpp
>index 5e462d49c2ce36f36017aba7b809fa4df7b40d61..e6d584718e9764a6aeac9ec1cb40b490fb0de635 100644
>--- a/Source/WebCore/layout/floats/FloatingContext.cpp
>+++ b/Source/WebCore/layout/floats/FloatingContext.cpp
>@@ -64,8 +64,8 @@ class Iterator;
> class FloatingPair {
> public:
>     bool isEmpty() const { return !m_leftIndex && !m_rightIndex; }
>-    const Display::Box* left() const;
>-    const Display::Box* right() const;
>+    const FloatingState::FloatItem* left() const;
>+    const FloatingState::FloatItem* right() const;
>     bool intersects(const Display::Box::Rect&) const;
>     PositionInContextRoot verticalPosition() const { return m_verticalPosition; }
>     std::optional<PositionInContextRoot> horiztonalPosition(Float) const;
>@@ -254,27 +254,27 @@ FloatingPair::FloatingPair(const FloatingState::FloatList& floats)
> {
> }
> 
>-const Display::Box* FloatingPair::left() const
>+const FloatingState::FloatItem* FloatingPair::left() const
> {
>     if (!m_leftIndex)
>         return nullptr;
> 
>-    ASSERT(m_floats[*m_leftIndex].layoutBox().isLeftFloatingPositioned());
>-    return &m_floats[*m_leftIndex].displayBox();
>+    ASSERT(m_floats[*m_leftIndex].isLeftPositioned());
>+    return &m_floats[*m_leftIndex];
> }
> 
>-const Display::Box* FloatingPair::right() const
>+const FloatingState::FloatItem* FloatingPair::right() const
> {
>     if (!m_rightIndex)
>         return nullptr;
> 
>-    ASSERT(m_floats[*m_rightIndex].layoutBox().isRightFloatingPositioned());
>-    return &m_floats[*m_rightIndex].displayBox();
>+    ASSERT(!m_floats[*m_rightIndex].isLeftPositioned());
>+    return &m_floats[*m_rightIndex];
> }
> 
> bool FloatingPair::intersects(const Display::Box::Rect& candidateRect) const
> {
>-    auto intersects = [&](const Display::Box* floating, Float floatingType) {
>+    auto intersects = [&](const FloatingState::FloatItem* floating, Float floatingType) {
>         if (!floating)
>             return false;
> 
>@@ -348,8 +348,8 @@ inline static std::optional<unsigned> previousFloatingIndex(Float floatingType,
>     RELEASE_ASSERT(currentIndex <= floats.size());
> 
>     while (currentIndex) {
>-        auto& floating = floats[--currentIndex].layoutBox();
>-        if ((floatingType == Float::Left && floating.isLeftFloatingPositioned()) || (floatingType == Float::Right && floating.isRightFloatingPositioned()))
>+        auto& floating = floats[--currentIndex];
>+        if ((floatingType == Float::Left && floating.isLeftPositioned()) || (floatingType == Float::Right && !floating.isLeftPositioned()))
>             return currentIndex;
>     }
> 
>@@ -371,7 +371,7 @@ Iterator& Iterator::operator++()
>         if (!currentIndex)
>             return { };
> 
>-        auto currentBottom = m_floats[currentIndex].displayBox().rectWithMargin().bottom();
>+        auto currentBottom = m_floats[currentIndex].rectWithMargin().bottom();
> 
>         std::optional<unsigned> index = currentIndex;
>         while (true) {
>@@ -379,7 +379,7 @@ Iterator& Iterator::operator++()
>             if (!index)
>                 return { };
> 
>-            if (m_floats[*index].displayBox().rectWithMargin().bottom() > currentBottom)
>+            if (m_floats[*index].rectWithMargin().bottom() > currentBottom)
>                 return index;
>         }
> 
>@@ -443,7 +443,7 @@ void Iterator::set(PositionInContextRoot verticalPosition)
>             if (!index)
>                 return { };
> 
>-            auto bottom = m_floats[*index].displayBox().rectWithMargin().bottom();
>+            auto bottom = m_floats[*index].rectWithMargin().bottom();
>             // Is this floating intrusive on this position?
>             if (bottom > verticalPosition)
>                 return index;
>@@ -455,8 +455,8 @@ void Iterator::set(PositionInContextRoot verticalPosition)
>     m_current.m_leftIndex = findFloatingBelow(Float::Left);
>     m_current.m_rightIndex = findFloatingBelow(Float::Right);
> 
>-    ASSERT(!m_current.m_leftIndex || (*m_current.m_leftIndex < m_floats.size() && m_floats[*m_current.m_leftIndex].layoutBox().isLeftFloatingPositioned()));
>-    ASSERT(!m_current.m_rightIndex || (*m_current.m_rightIndex < m_floats.size() && m_floats[*m_current.m_rightIndex].layoutBox().isRightFloatingPositioned()));
>+    ASSERT(!m_current.m_leftIndex || (*m_current.m_leftIndex < m_floats.size() && m_floats[*m_current.m_leftIndex].isLeftPositioned()));
>+    ASSERT(!m_current.m_rightIndex || (*m_current.m_rightIndex < m_floats.size() && !m_floats[*m_current.m_rightIndex].isLeftPositioned()));
> }
> 
> bool Iterator::operator==(const Iterator& other) const
>diff --git a/Source/WebCore/layout/floats/FloatingState.cpp b/Source/WebCore/layout/floats/FloatingState.cpp
>index a82ff8ad2b862bd15cdee75cf69cb2210077b386..31d52c3ef57da6dd00662b3503ab868d8d660534 100644
>--- a/Source/WebCore/layout/floats/FloatingState.cpp
>+++ b/Source/WebCore/layout/floats/FloatingState.cpp
>@@ -41,9 +41,7 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(FloatingState);
> 
> FloatingState::FloatItem::FloatItem(const Box& layoutBox, const FloatingState& floatingState)
>     : m_layoutBox(makeWeakPtr(const_cast<Box&>(layoutBox)))
>-    , m_containingBlock(makeWeakPtr(const_cast<Container&>(*layoutBox.containingBlock())))
>     , m_absoluteDisplayBox(FormattingContext::mapBoxToAncestor(floatingState.layoutContext(), layoutBox, downcast<Container>(floatingState.root())))
>-    , m_containingBlockAbsoluteDisplayBox(FormattingContext::mapBoxToAncestor(floatingState.layoutContext(), *m_containingBlock, downcast<Container>(floatingState.root())))
> {
> }
> 
>@@ -71,7 +69,7 @@ static bool belongsToThisFloatingContext(const Box& layoutBox, const Box& floati
> void FloatingState::remove(const Box& layoutBox)
> {
>     for (size_t index = 0; index < m_floats.size(); ++index) {
>-        if (&m_floats[index].layoutBox() == &layoutBox) {
>+        if (m_floats[index] == layoutBox) {
>             m_floats.remove(index);
>             return;
>         }
>@@ -101,14 +99,14 @@ std::optional<LayoutUnit> FloatingState::bottom(const Box& formattingContextRoot
>     std::optional<LayoutUnit> bottom;
>     for (auto& floatItem : m_floats) {
>         // Ignore floats from other formatting contexts when the floating state is inherited.
>-        if (&formattingContextRoot != &floatItem.layoutBox().formattingContextRoot())
>+        if (!floatItem.inFormattingContext(formattingContextRoot))
>             continue;
> 
>-        if ((type == Clear::Left && !floatItem.layoutBox().isLeftFloatingPositioned())
>-            || (type == Clear::Right && !floatItem.layoutBox().isRightFloatingPositioned()))
>+        if ((type == Clear::Left && !floatItem.isLeftPositioned())
>+            || (type == Clear::Right && floatItem.isLeftPositioned()))
>             continue;
> 
>-        auto floatsBottom = floatItem.displayBox().rectWithMargin().bottom();
>+        auto floatsBottom = floatItem.rectWithMargin().bottom();
>         if (bottom) {
>             bottom = std::max(*bottom, floatsBottom);
>             continue;
>diff --git a/Source/WebCore/layout/floats/FloatingState.h b/Source/WebCore/layout/floats/FloatingState.h
>index 77d7130c30abcf7d35378a1f5b1795d2ebe72dd7..8f5f5c33f0e93ed92f7429d2e64f5cb9d1e26502 100644
>--- a/Source/WebCore/layout/floats/FloatingState.h
>+++ b/Source/WebCore/layout/floats/FloatingState.h
>@@ -28,6 +28,8 @@
> #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
> 
> #include "DisplayBox.h"
>+#include "LayoutBox.h"
>+#include "LayoutContainer.h"
> #include <wtf/IsoMalloc.h>
> #include <wtf/Ref.h>
> #include <wtf/WeakPtr.h>
>@@ -36,8 +38,6 @@ namespace WebCore {
> 
> namespace Layout {
> 
>-class Box;
>-class Container;
> class FormattingState;
> class LayoutContext;
> 
>@@ -62,18 +62,17 @@ public:
>     public:
>         FloatItem(const Box&, const FloatingState&);
> 
>-        const Box& layoutBox() const { return *m_layoutBox; }
>-        const Container& containingBlock() const { return *m_containingBlock; }
>+        bool operator==(const Box& layoutBox) const { return m_layoutBox.get() == &layoutBox; }
> 
>-        const Display::Box& displayBox() const { return m_absoluteDisplayBox; }
>-        const Display::Box& containingBlockDisplayBox() const { return m_containingBlockAbsoluteDisplayBox; }
>+        bool isLeftPositioned() const { return m_layoutBox->isLeftFloatingPositioned(); }
>+        bool inFormattingContext(const Box&) const;
>+
>+        Display::Box::Rect rectWithMargin() const { return m_absoluteDisplayBox.rectWithMargin(); }
>+        PositionInContextRoot bottom() const { return m_absoluteDisplayBox.bottom(); }
> 
>     private:
>         WeakPtr<Box> m_layoutBox;
>-        WeakPtr<Container> m_containingBlock;
>-
>         Display::Box m_absoluteDisplayBox;
>-        Display::Box m_containingBlockAbsoluteDisplayBox;
>     };
>     using FloatList = Vector<FloatItem>;
>     const FloatList& floats() const { return m_floats; }
>@@ -94,19 +93,28 @@ private:
> 
> inline std::optional<LayoutUnit> FloatingState::leftBottom(const Box& formattingContextRoot) const
> { 
>+    ASSERT(formattingContextRoot.establishesFormattingContext()); 
>     return bottom(formattingContextRoot, Clear::Left);
> }
> 
> inline std::optional<LayoutUnit> FloatingState::rightBottom(const Box& formattingContextRoot) const
> {
>+    ASSERT(formattingContextRoot.establishesFormattingContext()); 
>     return bottom(formattingContextRoot, Clear::Right);
> }
> 
> inline std::optional<LayoutUnit> FloatingState::bottom(const Box& formattingContextRoot) const
> {
>+    ASSERT(formattingContextRoot.establishesFormattingContext()); 
>     return bottom(formattingContextRoot, Clear::Both);
> }
> 
>+inline bool FloatingState::FloatItem::inFormattingContext(const Box& formattingContextRoot) const
>+{
>+    ASSERT(formattingContextRoot.establishesFormattingContext()); 
>+    return &m_layoutBox->formattingContextRoot() == &formattingContextRoot;
>+}
>+
> }
> }
> #endif

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188912: 347984