[css-grid] Clearing override sizes before running grid's layout logic.

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 434 matching lines @@
 
     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 through consecutive layout calls.
+        if (!m_gridIsDirty) {

[cbiesinger, 2016/09/26 14:56:38]

 && (sizesLogicalWidthToFitContent(style()->width())
    || width.isIntrinsic() || minWidth.isIntrinsic() || minHeight.isIntrinsic())

+            for (LayoutBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
+                if (child->isOutOfFlowPositioned())

[cbiesinger, 2016/09/26 14:56:38]

|| !hasOrthogonalFlow(child)

+                    continue;
+                child->clearOverrideSize();
+                child->clearContainingBlockOverrideSize();
+                layoutScope.setChildNeedsLayout(child);

[cbiesinger, 2016/09/23 16:57:09]

(I would use forceChildLayout() instead of this sequence)

I agree with Sergio, why force-layout everything here? Better to only layout if
you need; I don't think you necessarily need layout for any grid item? Flexbox
tries to only call layout when it is really needed and that's important for
performance.

[jfernandez, 2016/09/23 18:47:22]

I understand your point and I agree the current code can be seen as a
brute-force
approach, but as I commented below, I just wanted to spot the nature of the
issue
I'm trying to solve. So far, we have only found out cases with orthogonal items,
but
we are not sure if they are the only ones. 

Note that the problem happens when there are multiple layouts for the same
LayoutTree. There is no way to avoid laid out the orthogonal items after
clearing the override sizes
if we really want to execute the track sizing algo without depending on previous
values. 

+                child->layoutIfNeeded();

[svillar, 2016/09/23 15:42:28]

Why are you laying out the items even before placing them? I think the algorithm
will properly lay them out whenever needed.

Apart from that adding that extra layout seems overkill to me. Think about it,
one of the grid items might be another grid.

[jfernandez, 2016/09/23 18:47:22]

Well, that code address issues related to subsequent layouts; it won't be
executed the first time we laid out the grid and its children. That's why we
check m_isDirty. I'm sure we can find another way of doing this, but first we
need to agree on a valid approach to solve the real issue, which is the
different behavior of the grid layout logic when it's run multiple times. 

First of all, bear in mind that we only detected, so far, issues when there are
orthogonal grid items involved. We may focus this new code only on those cases,
but I wanted first
to do it generally, for the shake of discussing the problem.

As you know, we indeed need to laid out orthogonal grid items before we perform
the actual layout code of the grid container; otherwise we won't get the right
values
for the grid columns.

We we are doing here is to clear the override sizes, both local (for stretch)
and related
to the grid area (item's containing block). We do this in order to recompute it
again (remember that we are processing layout code twice) in a clean way.
However, we have
already laid out the grid items using the previous grid areas, so we must laid
out again
after clearing the override sizes; otherwise we will get a different result the
next time we
process the track sizing algorithm.
Yes, indeed. This would complicate the solution. As I said, we can address just
the
orthogonal case, but such nested grid item could be orthogonal as well. Hard to
say
how we should address that case.

+            }
+        }
+
         updateLogicalWidth();
         m_hasDefiniteLogicalHeight = hasDefiniteLogicalHeight();
 
         TextAutosizer::LayoutScope textAutosizerLayoutScope(this, &layoutScope);
 
         // TODO(svillar): we won't need to do this once the intrinsic width computation is isolated
         // from the LayoutGrid object state (it should not touch any attribute) (see crbug.com/627812)
         if (m_autoRepeatColumns && m_autoRepeatColumns != computeAutoRepeatTracksCount(ForColumns, TrackSizing))
             dirtyGrid();
         placeItemsOnGrid(TrackSizing);
@@ skipping 2202 matching lines @@
         GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 bool LayoutGrid::cachedHasDefiniteLogicalHeight() const
 {
     SECURITY_DCHECK(m_hasDefiniteLogicalHeight);
     return m_hasDefiniteLogicalHeight.value();
 }
 
 } // namespace blink