[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;
-    for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet()) {
-        if (relayoutChildren || columnSet->needsLayout()) {
-            if (!m_inBalancingPass)
-                columnSet->prepareForLayout();
-            shouldInvalidateRegions = true;
-        }
-    }
-
-    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();
+    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
+        // 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;
     }
 
-    layoutIfNeeded();
+    for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet())
+        columnSet->prepareForLayout(!m_inBalancingPass);

[Julien - ping for review, 2014/05/20 17:03:04]

We should rename this function to something better as we touch this code. It's
unclear to me what name would encompass what this function is doing though.

[mstensho (USE GERRIT), 2014/05/21 09:02:33]

This function prepares a column set for flow thread layout.
The flow thread is laid out first (and it's working on the sets during this
process, before they have actually been laid out). Afterwards, the sets are laid
out.
prepareForFlowThreadLayout() is more descriptive, but maybe we could come up
with something more cohesive than that?

[mstensho (USE GERRIT), 2014/05/22 15:07:18]

Done.

Ended up with resetColumnHeight(), which is only called in the initial pass.

+
+    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))
+            needsRelayout = true;
+        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;
 }
 
 }