Setting up OrderIterator shouldn't require an extra Vector
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 May 2014 07:17:52 +0000 (07:17 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 May 2014 07:17:52 +0000 (07:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=119061

Reviewed by Darin Adler.

From Blink r153971 by <jchaffraix@chromium.org>

This patches reuse the OrderIterator's Vector. It provides a helper class OrderIteratorPopulator, used for
manipulating the Vector directly. Which allows to consolidate the code into a single implementation across
flexbox and grid.

No new tests, already covered by current tests.

* rendering/OrderIterator.cpp:
(WebCore::OrderIteratorPopulator::~OrderIteratorPopulator): Reset OrderIterator and call
removeDuplicatedOrderValues().
(WebCore::OrderIteratorPopulator::removeDuplicatedOrderValues): Sorts the Vector and removes the duplicated
order values.
(WebCore::OrderIteratorPopulator::collectChild): Collect order value information from child.
(WebCore::OrderIterator::setOrderValues): Deleted.
* rendering/OrderIterator.h:
(WebCore::OrderIteratorPopulator::OrderIteratorPopulator): Add helper class to manipulate OrderValues Vector.
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock): Remove unneeded code related to old OrderValues vector.
(WebCore::RenderFlexibleBox::prepareOrderIteratorAndMargins): Populate OrderIterator using collectChild().
(WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Deleted.
* rendering/RenderFlexibleBox.h: Rename computeMainAxisPreferredSizes() to prepareOrderIteratorAndMargins().
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::populateExplicitGridAndOrderIterator): Populate OrderIterator using collectChild().

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/OrderIterator.cpp
Source/WebCore/rendering/OrderIterator.h
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h
Source/WebCore/rendering/RenderGrid.cpp

index a7ea312..5e17dee 100644 (file)
@@ -1,3 +1,35 @@
+2014-05-27  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        Setting up OrderIterator shouldn't require an extra Vector
+        https://bugs.webkit.org/show_bug.cgi?id=119061
+
+        Reviewed by Darin Adler.
+
+        From Blink r153971 by <jchaffraix@chromium.org>
+
+        This patches reuse the OrderIterator's Vector. It provides a helper class OrderIteratorPopulator, used for
+        manipulating the Vector directly. Which allows to consolidate the code into a single implementation across
+        flexbox and grid.
+
+        No new tests, already covered by current tests.
+
+        * rendering/OrderIterator.cpp:
+        (WebCore::OrderIteratorPopulator::~OrderIteratorPopulator): Reset OrderIterator and call
+        removeDuplicatedOrderValues().
+        (WebCore::OrderIteratorPopulator::removeDuplicatedOrderValues): Sorts the Vector and removes the duplicated
+        order values.
+        (WebCore::OrderIteratorPopulator::collectChild): Collect order value information from child.
+        (WebCore::OrderIterator::setOrderValues): Deleted.
+        * rendering/OrderIterator.h:
+        (WebCore::OrderIteratorPopulator::OrderIteratorPopulator): Add helper class to manipulate OrderValues Vector.
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutBlock): Remove unneeded code related to old OrderValues vector.
+        (WebCore::RenderFlexibleBox::prepareOrderIteratorAndMargins): Populate OrderIterator using collectChild().
+        (WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Deleted.
+        * rendering/RenderFlexibleBox.h: Rename computeMainAxisPreferredSizes() to prepareOrderIteratorAndMargins().
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::populateExplicitGridAndOrderIterator): Populate OrderIterator using collectChild().
+
 2014-05-26  Tim Horton  <timothy_horton@apple.com>
 
         [wk2] RemoteLayerBackingStore front buffers should be purgeable when unparented
index e09924f..a389867 100644 (file)
@@ -32,8 +32,7 @@
 #include "config.h"
 #include "OrderIterator.h"
 
-#include "RenderFlexibleBox.h"
-#include "RenderGrid.h"
+#include "RenderBox.h"
 
 namespace WebCore {
 
@@ -45,18 +44,6 @@ OrderIterator::OrderIterator(RenderBox& containerBox)
     reset();
 }
 
-void OrderIterator::setOrderValues(OrderValues&& orderValues)
-{
-    reset();
-    m_orderValues = std::move(orderValues);
-    if (m_orderValues.size() < 2)
-        return;
-
-    std::sort(m_orderValues.begin(), m_orderValues.end());
-    auto nextElement = std::unique(m_orderValues.begin(), m_orderValues.end());
-    m_orderValues.shrinkCapacity(nextElement - m_orderValues.begin());
-}
-
 RenderBox* OrderIterator::first()
 {
     reset();
@@ -94,4 +81,35 @@ void OrderIterator::reset()
     m_orderIndex = cInvalidIndex;
 }
 
+OrderIteratorPopulator::OrderIteratorPopulator(OrderIterator& iterator)
+    : m_iterator(iterator)
+{
+    // Note that we don't release the memory here, we only invalidate the size
+    // This avoids unneeded reallocation if the size ends up not changing.
+    m_iterator.m_orderValues.shrink(0);
+}
+
+OrderIteratorPopulator::~OrderIteratorPopulator()
+{
+    m_iterator.reset();
+
+    if (m_iterator.m_orderValues.size() > 1)
+        removeDuplicatedOrderValues();
+}
+
+void OrderIteratorPopulator::removeDuplicatedOrderValues()
+{
+    auto& orderValues = m_iterator.m_orderValues;
+
+    std::sort(orderValues.begin(), orderValues.end());
+    auto nextElement = std::unique(orderValues.begin(), orderValues.end());
+    orderValues.shrinkCapacity(nextElement - orderValues.begin());
+}
+
+void OrderIteratorPopulator::collectChild(const RenderBox& child)
+{
+    m_iterator.m_orderValues.append(child.style().order());
+}
+
+
 } // namespace WebCore
index e47c850..52f3f11 100644 (file)
@@ -41,10 +41,9 @@ class RenderBox;
 
 class OrderIterator {
 public:
-    OrderIterator(RenderBox&);
+    friend class OrderIteratorPopulator;
 
-    typedef Vector<int, 1> OrderValues;
-    void setOrderValues(OrderValues&&);
+    explicit OrderIterator(RenderBox&);
 
     RenderBox* currentChild() const { return m_currentChild; }
     RenderBox* first();
@@ -55,10 +54,24 @@ private:
 
     RenderBox& m_containerBox;
     RenderBox* m_currentChild;
-    OrderValues m_orderValues;
+
+    Vector<int, 1> m_orderValues;
     int m_orderIndex;
 };
 
+class OrderIteratorPopulator {
+public:
+    OrderIteratorPopulator(OrderIterator&);
+    ~OrderIteratorPopulator();
+
+    void collectChild(const RenderBox&);
+
+private:
+    void removeDuplicatedOrderValues();
+
+    OrderIterator& m_iterator;
+};
+
 } // namespace WebCore
 
 #endif //  OrderIterator_h
index 3b3ce23..daecbfd 100644 (file)
@@ -275,13 +275,11 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren, LayoutUnit)
 
     dirtyForLayoutFromPercentageHeightDescendants();
 
-    Vector<LineContext> lineContexts;
-    OrderIterator::OrderValues orderValues;
-    computeMainAxisPreferredSizes(orderValues);
-    m_orderIterator.setOrderValues(std::move(orderValues));
+    prepareOrderIteratorAndMargins();
 
     ChildFrameRects oldChildRects;
     appendChildFrameRects(oldChildRects);
+    Vector<LineContext> lineContexts;
     layoutFlexItems(relayoutChildren, lineContexts);
 
     updateLogicalHeight();
@@ -834,16 +832,12 @@ LayoutUnit RenderFlexibleBox::computeChildMarginValue(const Length& margin)
     return minimumValueForLength(margin, availableSize);
 }
 
-void RenderFlexibleBox::computeMainAxisPreferredSizes(OrderIterator::OrderValues& orderValues)
+void RenderFlexibleBox::prepareOrderIteratorAndMargins()
 {
-    ASSERT(orderValues.isEmpty());
+    OrderIteratorPopulator populator(m_orderIterator);
 
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        // Avoid growing the vector for the common-case default value of 0. This optimizes the most common case which is
-        // one or a few values with the default order 0
-        int order = child->style().order();
-        if (orderValues.isEmpty() || orderValues.last() != order)
-            orderValues.append(order);
+        populator.collectChild(*child);
 
         if (child->isOutOfFlowPositioned())
             continue;
index 91b547a..35852c9 100644 (file)
@@ -137,7 +137,7 @@ private:
     LayoutUnit marginBoxAscentForChild(RenderBox&);
 
     LayoutUnit computeChildMarginValue(const Length& margin);
-    void computeMainAxisPreferredSizes(OrderIterator::OrderValues&);
+    void prepareOrderIteratorAndMargins();
     LayoutUnit adjustChildSizeForMinAndMax(RenderBox&, LayoutUnit childSize);
     bool computeNextFlexLine(OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, double& totalFlexGrow, double& totalWeightedFlexShrink, LayoutUnit& minMaxAppliedMainAxisExtent, bool& hasInfiniteLineLength);
 
index da36c29..75e8beb 100644 (file)
@@ -698,17 +698,12 @@ void RenderGrid::placeItemsOnGrid()
 
 void RenderGrid::populateExplicitGridAndOrderIterator()
 {
-    // FIXME: We should find a way to share OrderValues's initialization code with RenderFlexibleBox.
-    OrderIterator::OrderValues orderValues;
+    OrderIteratorPopulator populator(m_orderIterator);
     size_t maximumRowIndex = std::max<size_t>(1, explicitGridRowCount());
     size_t maximumColumnIndex = std::max<size_t>(1, explicitGridColumnCount());
 
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        // Avoid growing the vector for the common-case default value of 0. This optimizes the most common case which is
-        // one or a few values with the default order 0
-        int order = child->style().order();
-        if (orderValues.isEmpty() || orderValues.last() != order)
-            orderValues.append(order);
+        populator.collectChild(*child);
 
         // This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
         std::unique_ptr<GridSpan> rowPositions = resolveGridPositionsFromStyle(child, ForRows);
@@ -725,8 +720,6 @@ void RenderGrid::populateExplicitGridAndOrderIterator()
     m_grid.grow(maximumRowIndex);
     for (size_t i = 0; i < m_grid.size(); ++i)
         m_grid[i].grow(maximumColumnIndex);
-
-    m_orderIterator.setOrderValues(std::move(orderValues));
 }
 
 void RenderGrid::placeSpecifiedMajorAxisItemsOnGrid(const Vector<RenderBox*>& autoGridItems)