Compute the normal flow list on the fly

Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp

Index: Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp
diff --git a/Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp b/Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp
index c767e84978dea41a9c1e5ad0d880cd5785b196e4..e6c440c50038a1663534bb0bfb0546da03bd44f1 100644
--- a/Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp
+++ b/Source/core/paint/DeprecatedPaintLayerStackingNodeIterator.cpp
@@ -31,10 +31,21 @@
 #include "config.h"
 #include "core/paint/DeprecatedPaintLayerStackingNodeIterator.h"
 
+// FIXME: We should build our primitive on top of
+// DeprecatedLayerStackingNode and remove this include.
+#include "core/paint/DeprecatedPaintLayer.h"
 #include "core/paint/DeprecatedPaintLayerStackingNode.h"
 
 namespace blink {
 
+DeprecatedPaintLayerStackingNodeIterator::DeprecatedPaintLayerStackingNodeIterator(const DeprecatedPaintLayerStackingNode& root, unsigned whichChildren)
+    : m_root(root)
+    , m_remainingChildren(whichChildren)
+    , m_index(0)
+{
+    m_currentNormalFlowChild = root.layer()->firstChild();
+}
+
 DeprecatedPaintLayerStackingNode* DeprecatedPaintLayerStackingNodeIterator::next()
 {
     if (m_remainingChildren & NegativeZOrderChildren) {
@@ -47,11 +58,16 @@ DeprecatedPaintLayerStackingNode* DeprecatedPaintLayerStackingNodeIterator::next
     }
 
     if (m_remainingChildren & NormalFlowChildren) {
-        Vector<DeprecatedPaintLayerStackingNode*>* normalFlowList = m_root.normalFlowList();
-        if (normalFlowList && m_index < normalFlowList->size())
-            return normalFlowList->at(m_index++);
+        for (; m_currentNormalFlowChild; m_currentNormalFlowChild = m_currentNormalFlowChild->nextSibling()) {
+            if (!m_currentNormalFlowChild->stackingNode()->isTreatedAsStackingContextForPainting() && !m_currentNormalFlowChild->isReflection()) {
+                DeprecatedPaintLayer* normalFlowChild = m_currentNormalFlowChild;
+                m_currentNormalFlowChild = m_currentNormalFlowChild->nextSibling();
+                return normalFlowChild->stackingNode();
+            }
+        }
 
-        m_index = 0;
+        // We reset the iterator in case we reuse it.
+        m_currentNormalFlowChild = m_root.layer()->firstChild();

[chrishtr, 2015/06/24 23:07:55]

Is this functionality actually used? I don't think stl iterators work this way.

[Julien - ping for review, 2015/06/24 23:13:57]

It's not used no. For consistency with the other cases that reset m_index to 0,
I reset this value.

If you prefer I can skip resetting it.

[chrishtr, 2015/06/24 23:36:30]

It seems weird as written. Because you can call next() one more time after
getting a null return value, it'll loop around. Maybe set it to nullptr and
ASSERT that next() is never called again?

[Julien - ping for review, 2015/06/25 17:16:24]

It won't loop around as we remove normal flow children from m_remainingChildren
the line below. Where did you see that next() would return nullptr then a value?
(that would be a definite bug!)

It's more like if you re-add normal the flow list to the iterator, it would just
work (tm). This is not possible right now though so it's a moot point.

[chrishtr, 2015/06/25 17:20:49]

If you call next() and it's already at the end, then won't line 83 execute and
return nullptr?

[chrishtr, 2015/06/25 20:38:19]

I see now. (Julien explained offline). LGTM

         m_remainingChildren &= ~NormalFlowChildren;
     }
 
@@ -79,9 +95,13 @@ DeprecatedPaintLayerStackingNode* DeprecatedPaintLayerStackingNodeReverseIterato
     }
 
     if (m_remainingChildren & NormalFlowChildren) {
-        Vector<DeprecatedPaintLayerStackingNode*>* normalFlowList = m_root.normalFlowList();
-        if (normalFlowList && m_index >= 0)
-            return normalFlowList->at(m_index--);
+        for (; m_currentNormalFlowChild; m_currentNormalFlowChild = m_currentNormalFlowChild->previousSibling()) {
+            if (!m_currentNormalFlowChild->stackingNode()->isTreatedAsStackingContextForPainting() && !m_currentNormalFlowChild->isReflection()) {
+                DeprecatedPaintLayer* normalFlowChild = m_currentNormalFlowChild;
+                m_currentNormalFlowChild = m_currentNormalFlowChild->previousSibling();
+                return normalFlowChild->stackingNode();
+            }
+        }
 
         m_remainingChildren &= ~NormalFlowChildren;
         setIndexToLastItem();
@@ -112,13 +132,8 @@ void DeprecatedPaintLayerStackingNodeReverseIterator::setIndexToLastItem()
     }
 
     if (m_remainingChildren & NormalFlowChildren) {
-        Vector<DeprecatedPaintLayerStackingNode*>* normalFlowList = m_root.normalFlowList();
-        if (normalFlowList) {
-            m_index = normalFlowList->size() - 1;
-            return;
-        }
-
-        m_remainingChildren &= ~NormalFlowChildren;
+        m_currentNormalFlowChild = m_root.layer()->lastChild();
+        return;
     }
 
     if (m_remainingChildren & PositiveZOrderChildren) {