[css-grid] Avoid storing pointers to orthogonal grid items

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 446 matching lines @@
   // In orthogonal flow cases column track's size is determined by using the
   // computed row track's size, which it was estimated during the first cycle of
   // the sizing algorithm.
   // Hence we need to repeat computeUsedBreadthOfGridTracks for both, columns
   // and rows, to determine the final values.
   // TODO (lajava): orthogonal flows is just one of the cases which may require
   // a new cycle of the sizing algorithm; there may be more. In addition, not
   // all the cases with orthogonal flows require this extra cycle; we need a
   // more specific condition to detect whether child's min-content contribution
   // has changed or not.
-  if (!m_orthogonalChildren.isEmpty()) {
+  if (m_hasAnyOrthogonalChildren) {
     computeTrackSizesForDefiniteSize(ForColumns, sizingData,
                                      availableSpaceForColumns);
     computeTrackSizesForDefiniteSize(ForRows, sizingData,
                                      availableSpaceForRows);
   }
 }
 
 void LayoutGrid::layoutBlock(bool relayoutChildren) {
   ASSERT(needsLayout());
 
   if (!relayoutChildren && simplifiedLayout())
     return;
 
   SubtreeLayoutScope layoutScope(*this);
 
   {
     // LayoutState needs this deliberate scope to pop before updating scroll
     // information (which may trigger relayout).
     LayoutState state(*this, locationOffset());
 
     LayoutSize previousSize = size();
 
     // We need to clear both own and containingBlock override sizes to
     // ensure we get the same result when grid's intrinsic size is
     // computed again in the updateLogicalWidth call bellow.
     if (sizesLogicalWidthToFitContent(styleRef().logicalWidth()) ||
         styleRef().logicalWidth().isIntrinsicOrAuto()) {
-      // We do cache orthogonal items during the placeItemsOnGrid call, which is
-      // executed later. However, we are
-      // only interested on running this logic when we are performing a
-      // relayout, hence we have already cached
-      // the orthogonal items.
-      for (auto* child : m_orthogonalChildren) {
-        if (child->isOutOfFlowPositioned())
+      for (auto* child = firstChildBox(); child;
+           child = child->nextInFlowSiblingBox()) {
+        if (child->isOutOfFlowPositioned() || !isOrthogonalChild(*child))

[cbiesinger, 2016/10/17 12:16:45]

Since you use nextInFlowSiblingBox, do you need the isOutOfFlowPositioned check?

[jfernandez, 2016/10/17 12:37:37]

You're right. I thought the same, but since we were already using it in our 
grid item positioned logic, I blindly trusted our previous implementation.

What I'm going to do is to do a patch to fix both cases.

[Manuel Rego, 2016/10/17 12:40:42]

Note that we need to check that firstChildBox() is not out of flow.
So or we implement a firstInFlowChildBox() or we probably simply use
nextSiblingBox() and keep the if here.

[jfernandez, 2016/10/17 12:43:41]

That's true. I think we should change both and just use nextSiblingBox 
instead.

           continue;
         child->clearOverrideSize();
         child->clearContainingBlockOverrideSize();
         child->forceLayout();
       }
     }
 
     updateLogicalWidth();
     m_hasDefiniteLogicalHeight = hasDefiniteLogicalHeight();
 
@@ skipping 1413 matching lines @@
         computeAutoRepeatTracksCount(ForColumns, sizingOperation);
   m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows, sizingOperation);
 
   populateExplicitGridAndOrderIterator();
 
   // We clear the dirty bit here as the grid sizes have been updated.
   m_gridIsDirty = false;
 
   Vector<LayoutBox*> autoMajorAxisAutoGridItems;
   Vector<LayoutBox*> specifiedMajorAxisAutoGridItems;
-  m_orthogonalChildren.shrink(0);
+  m_hasAnyOrthogonalChildren = false;
   for (LayoutBox* child = m_orderIterator.first(); child;
        child = m_orderIterator.next()) {
     if (child->isOutOfFlowPositioned())
       continue;
 
-    if (isOrthogonalChild(*child))
-      m_orthogonalChildren.append(child);
+    m_hasAnyOrthogonalChildren =
+        m_hasAnyOrthogonalChildren || isOrthogonalChild(*child);
 
     GridArea area = cachedGridArea(*child);
     if (!area.rows.isIndefinite())
       area.rows.translate(abs(m_smallestRowStart));
     if (!area.columns.isIndefinite())
       area.columns.translate(abs(m_smallestColumnStart));
     m_gridItemArea.set(child, area);
 
     if (area.rows.isIndefinite() || area.columns.isIndefinite()) {
       GridSpan majorAxisPositions =
@@ skipping 1374 matching lines @@
   if (!m_gridItemArea.isEmpty())
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 bool LayoutGrid::cachedHasDefiniteLogicalHeight() const {
   SECURITY_DCHECK(m_hasDefiniteLogicalHeight);
   return m_hasDefiniteLogicalHeight.value();
 }
 
 }  // namespace blink