[New Multicolumn] Rebalance properly when child content changes.

Source/core/rendering/RenderMultiColumnFlowThread.cpp

 /*
  * Copyright (C) 2012 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 121 matching lines @@
     flipForWritingMode(flowThreadPoint);
     LayoutUnit blockOffset = isHorizontalWritingMode() ? flowThreadPoint.y() : flowThreadPoint.x();
     RenderRegion* renderRegion = regionAtBlockOffset(blockOffset);
     if (!renderRegion)
         return LayoutSize(0, 0);
     return toRenderMultiColumnSet(renderRegion)->flowThreadTranslationAtOffset(blockOffset);
 }
 
 void RenderMultiColumnFlowThread::layoutColumns(bool relayoutChildren, SubtreeLayoutScope& layoutScope)
 {
-    // Update the dimensions of our regions before we lay out the flow thread.
-    // FIXME: Eventually this is going to get way more complicated, and we will be destroying regions
-    // instead of trying to keep them around.
-    bool shouldInvalidateRegions = false;
+    if (relayoutChildren)
+        layoutScope.setChildNeedsLayout(this);
+
+    if (!needsLayout()) {
+        // At the end of multicol container (our parent RenderBlockFlow) layout, it will call
+        // recalculateColumnHeights() on us unconditionally, but if we won't lay out (e.g. because
+        // we have fixed width with layout already being up-to-date), we want to prevent
+        // recalculateColumnHeights() from doing any work, so that the column balancing machinery
+        // doesn't kick in and trigger additional unnecessary layout passes. Actually, it's not just
+        // a good idea in general to not waste time on balancing content that hasn't been re-laid

[Julien - ping for review, 2014/05/23 09:07:54]

I have a hard time parsing this sentence due to too many negation (it's NOT just
.... to NOT waste ... hasN'T been re-laid). It would be better to simplify the
sentence altogether.

[mstensho (USE GERRIT), 2014/05/23 11:46:09]

Done.

+        // out; we are actually required to guarantee this. The calculation of implicit breaks needs
+        // to be preceded by a proper layout pass, since it's layout that sets up content runs, and
+        // the runs get deleted right after every pass.
+        m_needsColumnHeightsRecalculation = false;
+        return;
+    }
+
     for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet()) {
-        if (relayoutChildren || columnSet->needsLayout()) {
-            if (!m_inBalancingPass)
-                columnSet->prepareForLayout();
-            shouldInvalidateRegions = true;
+        if (!m_inBalancingPass) {
+            // This is the initial layout pass. We need to reset the column height, because contents
+            // typically have changed.
+            columnSet->resetColumnHeight();
+
+            columnSet->setComputedColumnWidthAndCount(columnWidth(), columnCount());
         }
     }
 
-    if (shouldInvalidateRegions)
-        invalidateRegions();
-
-    if (relayoutChildren)
-        layoutScope.setChildNeedsLayout(this);
-
-    if (requiresBalancing()) {
-        // At the end of multicol layout, relayoutForPagination() is called unconditionally, but if
-        // no children are to be laid out (e.g. fixed width with layout already being up-to-date),
-        // we want to prevent it from doing any work, so that the column balancing machinery doesn't
-        // kick in and trigger additional unnecessary layout passes. Actually, it's not just a good
-        // idea in general to not waste time on balancing content that hasn't been re-laid out; we
-        // are actually required to guarantee this. The calculation of implicit breaks needs to be
-        // preceded by a proper layout pass, since it's layout that sets up content runs, and the
-        // runs get deleted right after every pass.
-        m_needsColumnHeightsRecalculation = shouldInvalidateRegions || needsLayout();
-    }
-
-    layoutIfNeeded();
+    invalidateRegions();
+    m_needsColumnHeightsRecalculation = requiresBalancing();
+    layout();
 }
 
 bool RenderMultiColumnFlowThread::computeColumnCountAndWidth()
 {
     RenderBlock* columnBlock = multiColumnBlockFlow();
     LayoutUnit oldColumnWidth = m_columnWidth;
 
     // Calculate our column width and column count.
     m_columnCount = 1;
     m_columnWidth = columnBlock->contentLogicalWidth();
@@ skipping 15 matching lines @@
     } else {
         m_columnCount = std::max<LayoutUnit>(std::min<LayoutUnit>(colCount, (availWidth + colGap) / (colWidth + colGap)), 1);
         m_columnWidth = ((availWidth + colGap) / m_columnCount) - colGap;
     }
 
     return m_columnWidth != oldColumnWidth;
 }
 
 bool RenderMultiColumnFlowThread::recalculateColumnHeights()
 {
+    // All column sets that needed layout have now been laid out, so we can finally validate them.
+    validateRegions();
+
     if (!m_needsColumnHeightsRecalculation)
         return false;
 
     // Column heights may change here because of balancing. We may have to do multiple layout
     // passes, depending on how the contents is fitted to the changed column heights. In most
     // cases, laying out again twice or even just once will suffice. Sometimes we need more
     // passes than that, though, but the number of retries should not exceed the number of
     // columns, unless we have a bug.
     bool needsRelayout = false;
     for (RenderMultiColumnSet* multicolSet = firstMultiColumnSet(); multicolSet; multicolSet = multicolSet->nextSiblingMultiColumnSet()) {
-        if (multicolSet->recalculateColumnHeight(!m_inBalancingPass)) {
+        if (multicolSet->recalculateColumnHeight(m_inBalancingPass ? RenderMultiColumnSet::StretchBySpaceShortage : RenderMultiColumnSet::GuessFromFlowThreadPortion))
+            needsRelayout = true;

[Julien - ping for review, 2014/05/23 09:07:54]

needsRelayout |= multicolSet->recalculateColumnHeight(m_inBalancingPass ?
RenderMultiColumnSet::StretchBySpaceShortage :
RenderMultiColumnSet::GuessFromFlowThreadPortion); ?

[mstensho (USE GERRIT), 2014/05/23 11:46:09]

Done.

+        if (needsRelayout) {
+            // Once a column set gets a new column height, that column set and all successive column
+            // sets need to be laid out over again, since their logical top will be affected by
+            // this, and therefore their column heights may change as well, at least if the multicol
+            // height is constrained.
             multicolSet->setChildNeedsLayout(MarkOnlyThis);
-            needsRelayout = true;
         }
     }
 
     if (needsRelayout)
         setChildNeedsLayout(MarkOnlyThis);
 
     m_inBalancingPass = needsRelayout;
     return needsRelayout;
 }
 
@@ skipping 82 matching lines @@
 }
 
 bool RenderMultiColumnFlowThread::isPageLogicalHeightKnown() const
 {
     if (RenderMultiColumnSet* columnSet = lastMultiColumnSet())
         return columnSet->computedColumnHeight();
     return false;
 }
 
 }