[New Multicolumn] Rebalance properly when child content changes.

Source/core/rendering/RenderMultiColumnSet.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 66 matching lines @@
 {
     RenderBlockFlow* multicolBlock = multiColumnBlockFlow();
     LayoutUnit contentLogicalTop = logicalTop() - multicolBlock->borderBefore() - multicolBlock->paddingBefore();
 
     height -= contentLogicalTop;
     return max(height, LayoutUnit(1)); // Let's avoid zero height, as that would probably cause an infinite amount of columns to be created.
 }
 
 LayoutUnit RenderMultiColumnSet::pageLogicalTopForOffset(LayoutUnit offset) const
 {
-    LayoutUnit portionLogicalTop = (isHorizontalWritingMode() ? flowThreadPortionRect().y() : flowThreadPortionRect().x());
     unsigned columnIndex = columnIndexAtOffset(offset, AssumeNewColumns);
-    return portionLogicalTop + columnIndex * computedColumnHeight();
+    return logicalTopInFlowThread() + columnIndex * computedColumnHeight();
 }
 
 void RenderMultiColumnSet::setAndConstrainColumnHeight(LayoutUnit newHeight)
 {
     m_computedColumnHeight = newHeight;
     if (m_computedColumnHeight > m_maxColumnHeight)
         m_computedColumnHeight = m_maxColumnHeight;
     // FIXME: the height may also be affected by the enclosing pagination context, if any.
 }
 
@@ skipping 99 matching lines @@
     }
     LayoutUnit newColumnHeight = calculateColumnHeight(initial);
     setAndConstrainColumnHeight(newColumnHeight);
 
     // After having calculated an initial column height, the multicol container typically needs at
     // least one more layout pass with a new column height, but if a height was specified, we only
     // need to do this if we think that we need less space than specified. Conversely, if we
     // determined that the columns need to be as tall as the specified height of the container, we
     // have already laid it out correctly, and there's no need for another pass.
 
+    // We can get rid of the content runs now, if we haven't already done so. They are only needed
+    // to calculate the initial balanced column height. In fact, we have to get rid of them before
+    // the next layout pass, since each pass will rebuild this.
+    m_contentRuns.clear();
+
     if (m_computedColumnHeight == oldColumnHeight)
         return false; // No change. We're done.
 
     m_minSpaceShortage = RenderFlowThread::maxLogicalHeight();
-    m_contentRuns.clear();
     return true; // Need another pass.
 }
 
 void RenderMultiColumnSet::recordSpaceShortage(LayoutUnit spaceShortage)
 {
     if (spaceShortage >= m_minSpaceShortage)
         return;
 
     // The space shortage is what we use as our stretch amount. We need a positive number here in
     // order to get anywhere.
@@ skipping 17 matching lines @@
     LayoutUnit minimumContentLogicalWidth = colCount * computedColumnWidth() + (colCount - 1) * colGap;
     LayoutUnit currentContentLogicalWidth = contentLogicalWidth();
     LayoutUnit delta = max(LayoutUnit(), minimumContentLogicalWidth - currentContentLogicalWidth);
     if (!delta)
         return;
 
     // Increase our logical width by the delta.
     setLogicalWidth(logicalWidth() + delta);
 }
 
-void RenderMultiColumnSet::prepareForLayout()
+void RenderMultiColumnSet::prepareForLayout(bool initial)

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

We should replace the first-pass boolean logic with an enum while touching this
code!

/me doesn't like boolean parameter

(Also this is a coding style violation, boolean parameters should have a verb in
them (the coding style says 'Precede boolean values with words like "is" and
"did".'))

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

Done.

I did the second best thing - removed the parameter. :)
It's now resetColumnHeight(), and it's only called in the initial layout phase,
so no need for the parameter.

 {
     RenderBlockFlow* multicolBlock = multiColumnBlockFlow();
-    RenderStyle* multicolStyle = multicolBlock->style();
 
     // Set box logical top.
     ASSERT(!previousSiblingBox() || !previousSiblingBox()->isRenderMultiColumnSet()); // FIXME: multiple set not implemented; need to examine previous set to calculate the correct logical top.
-    setLogicalTop(multicolBlock->borderBefore() + multicolBlock->paddingBefore());
+    setLogicalTop(multicolBlock->borderAndPaddingBefore());
 
-    // Set box width.
-    updateLogicalWidth();
-
+    if (initial) {
+        m_maxColumnHeight = calculateMaxColumnHeight();
+        updateLogicalWidth();

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

It's weird that prepareForLayout actually does what layout requires...
Especially since this is only run for the initial layout.

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

Done.

All it really needed from updateLogicalWidth() was to update the column count
and width. It's now done from the flow thread rather than here (since the method
is now named resetColumnHeight()). I got rid of the updateLogicalWidth()
override while I was at it.

+    }
     if (multiColumnFlowThread()->requiresBalancing()) {
-        // Set maximum column height. We will not stretch beyond this.
-        m_maxColumnHeight = RenderFlowThread::maxLogicalHeight();
-        if (!multicolStyle->logicalHeight().isAuto()) {
-            m_maxColumnHeight = multicolBlock->computeContentLogicalHeight(multicolStyle->logicalHeight(), -1);
-            if (m_maxColumnHeight == -1)
-                m_maxColumnHeight = RenderFlowThread::maxLogicalHeight();
-        }
-        if (!multicolStyle->logicalMaxHeight().isUndefined()) {
-            LayoutUnit logicalMaxHeight = multicolBlock->computeContentLogicalHeight(multicolStyle->logicalMaxHeight(), -1);
-            if (logicalMaxHeight != -1 && m_maxColumnHeight > logicalMaxHeight)
-                m_maxColumnHeight = logicalMaxHeight;
-        }
-        m_maxColumnHeight = heightAdjustedForSetOffset(m_maxColumnHeight);
-        m_computedColumnHeight = 0; // Restart balancing.
+        if (initial)
+            m_computedColumnHeight = 0;
     } else {
         setAndConstrainColumnHeight(heightAdjustedForSetOffset(multiColumnFlowThread()->columnHeightAvailable()));
     }
 
-    m_contentRuns.clear();
+    // Content runs are only needed in the initial layout pass, in order to find an initial column
+    // height, and should have been deleted afterwards. We're about to rebuild the content runs, so
+    // the list needs to be empty.
+    ASSERT(m_contentRuns.isEmpty());
 
     // Nuke previously stored minimum column height. Contents may have changed for all we know.
     m_minimumColumnHeight = 0;
+
+    setNeedsLayoutAndFullRepaint();

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

There is no branch in prepareForLayout, which means that we will always force a
layout on all multi-column set. Do we really need that?

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

No, that looks very unnecessary, apart from being a nice way to hide repaint
bugs, of course. :)

I must have been somewhat careless when I did this. Now that it's called
blablablaAndFullRepaint(), it's more obvious that it's not cheap.

We can probably even get away with using the somewhat milder
setChildNeedsLayout(MarkOnlyThis).

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

Done.

We now call setChildNeedsLayout(MarkOnlyThis), and only in the initial layout
phase, and even then only if column height changed.

 }
 
 void RenderMultiColumnSet::expandToEncompassFlowThreadContentsIfNeeded()
 {
     ASSERT(multiColumnFlowThread()->lastMultiColumnSet() == this);
     LayoutRect rect(flowThreadPortionRect());
 
     // Get the offset within the flow thread in its block progression direction. Then get the
     // flow thread's remaining logical height including its overflow and expand our rect
     // to encompass that remaining height and overflow. The idea is that we will generate
     // additional columns and pages to hold that overflow, since people do write bad
     // content like <body style="height:0px"> in multi-column layouts.
     bool isHorizontal = flowThread()->isHorizontalWritingMode();
     LayoutUnit logicalTopOffset = isHorizontal ? rect.y() : rect.x();
     LayoutRect layoutRect = flowThread()->layoutOverflowRect();
     LayoutUnit logicalHeightWithOverflow = (isHorizontal ? layoutRect.maxY() : layoutRect.maxX()) - logicalTopOffset;
     setFlowThreadPortionRect(LayoutRect(rect.x(), rect.y(), isHorizontal ? rect.width() : logicalHeightWithOverflow, isHorizontal ? logicalHeightWithOverflow : rect.height()));
 }
 
 void RenderMultiColumnSet::layout()
 {
     RenderRegion::layout();
 
-    if (!nextSiblingMultiColumnSet()) {
-        // This is the last set, i.e. the last region. Seize the opportunity to validate them.
-        multiColumnFlowThread()->validateRegions();
-    }
+    // At this point the logical top of the column set is known. Update maximum column height
+    // (multicol height may be constrained, and in order to calculate it, we need to know our top).
+    m_maxColumnHeight = calculateMaxColumnHeight();
 }
 
 void RenderMultiColumnSet::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
 {
     computedValues.m_extent = m_computedColumnHeight;
     computedValues.m_position = logicalTop;
 }
 
+LayoutUnit RenderMultiColumnSet::calculateMaxColumnHeight() const
+{
+    RenderBlockFlow* multicolBlock = multiColumnBlockFlow();
+    RenderStyle* multicolStyle = multicolBlock->style();
+    LayoutUnit availableHeight = multiColumnFlowThread()->columnHeightAvailable();
+    LayoutUnit maxColumnHeight = availableHeight ? availableHeight : RenderFlowThread::maxLogicalHeight();
+    if (!multicolStyle->logicalMaxHeight().isUndefined()) {
+        LayoutUnit logicalMaxHeight = multicolBlock->computeContentLogicalHeight(multicolStyle->logicalMaxHeight(), -1);
+        if (logicalMaxHeight != -1 && maxColumnHeight > logicalMaxHeight)
+            maxColumnHeight = logicalMaxHeight;
+    }
+    return heightAdjustedForSetOffset(maxColumnHeight);
+}
+
 LayoutUnit RenderMultiColumnSet::columnGap() const
 {
     RenderBlockFlow* parentBlock = multiColumnBlockFlow();
     if (parentBlock->style()->hasNormalColumnGap())
         return parentBlock->style()->fontDescription().computedPixelSize(); // "1em" is recommended as the normal gap setting. Matches <p> margins.
     return parentBlock->style()->columnGap();
 }
 
 unsigned RenderMultiColumnSet::columnCount() const
 {
@@ skipping 299 matching lines @@
         fragments.append(fragment);
     }
 }
 
 const char* RenderMultiColumnSet::renderName() const
 {
     return "RenderMultiColumnSet";
 }
 
 }