[New Multicolumn] Actual support for layout of column-span:all.

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 14 matching lines @@
 
 #include "config.h"
 #include "core/rendering/RenderMultiColumnFlowThread.h"
 
 #include "core/rendering/RenderMultiColumnSet.h"
 #include "core/rendering/RenderMultiColumnSpannerSet.h"
 
 namespace blink {
 
 RenderMultiColumnFlowThread::RenderMultiColumnFlowThread()
-    : m_columnCount(1)
+    : m_lastSetWorkedOn(0)
+    , m_columnCount(1)
     , m_columnHeightAvailable(0)
     , m_inBalancingPass(false)
     , m_needsColumnHeightsRecalculation(false)
     , m_progressionIsInline(true)
 {
     setFlowThreadState(InsideInFlowThread);
 }
 
 RenderMultiColumnFlowThread::~RenderMultiColumnFlowThread()
 {
@@ skipping 109 matching lines @@
         // Just before the multicol container (our parent RenderBlockFlow) finishes laying out, it
         // will call recalculateColumnHeights() on us unconditionally, but we only want that method
         // to do any work if we actually laid out the flow thread. Otherwise, the balancing
         // machinery would kick in needlessly, and trigger additional layout passes. Furthermore, we
         // actually depend on a proper flowthread layout pass in order to do balancing, since it's
         // flowthread layout that sets up content runs.
         m_needsColumnHeightsRecalculation = false;
         return;
     }
 
-    for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet()) {
-        if (!m_inBalancingPass) {
-            // This is the initial layout pass. We need to reset the column height, because contents
-            // typically have changed.
+    if (!m_inBalancingPass) {
+        // This is the initial layout pass. We need to reset the column height, because contents
+        // typically have changed.
+        for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet())
             columnSet->resetColumnHeight();
-        }
     }
 
     invalidateRegions();
-    m_needsColumnHeightsRecalculation = heightIsAuto();
+    m_needsColumnHeightsRecalculation = true;

[Julien - ping for review, 2014/11/17 23:47:11]

That seems overly broad as you will force recomputing the column heights in any
cases. Shouldn't we only redo column height measurement if there is a column
spanner or the height is auto? If so, we should probably fix this or at least
put a FIXME here.

[mstensho (USE GERRIT), 2014/11/18 13:29:26]

Done.

You're right. There's no need for this if there's only one set and that set
isn't to be balanced.

     layout();
 }
 
 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;
@@ skipping 124 matching lines @@
     m_spannerMap.clear();
     // Detach all column sets from the flow thread. Cannot destroy them at this point, since they
     // are siblings of this object, and there may be pointers to this object's sibling somewhere
     // further up on the call stack.
     for (RenderMultiColumnSet* columnSet = firstMultiColumnSet(); columnSet; columnSet = columnSet->nextSiblingMultiColumnSet())
         columnSet->detachRegion();
     multiColumnBlockFlow()->resetMultiColumnFlowThread();
     RenderFlowThread::willBeRemovedFromTree();
 }
 
+bool RenderMultiColumnFlowThread::isColumnSpanner(const RenderObject* descendant) const
+{
+    ASSERT(descendant->isDescendantOf(this));
+    return m_spannerMap.get(descendant);
+}
+
+bool RenderMultiColumnFlowThread::isInsideColumnSpanner(const RenderObject* descendant) const
+{
+    ASSERT(descendant->isDescendantOf(this));
+    return containingColumnSpannerSet(descendant);
+}
+
+LayoutUnit RenderMultiColumnFlowThread::enterColumnSpannerBeforeLayout(RenderBox* renderer, LayoutUnit logicalTop, SubtreeLayoutScope& layoutScope)
+{
+    ASSERT(renderer->isDescendantOf(this));
+    RenderMultiColumnSpannerSet* spannerSet = m_spannerMap.get(renderer);
+    ASSERT(spannerSet);

[Julien - ping for review, 2014/11/17 23:47:11]

The 2 ASSERTs above could be replaced by:

ASSERT(isColumnSpanner(renderer));

[mstensho (USE GERRIT), 2014/11/18 13:29:26]

Done.

+
+    // FIXME: it's really only necessary to mark the spanner set for layout when the height of
+    // |renderer| changes.
+    spannerSet->setChildNeedsLayout(MarkOnlyThis, &layoutScope);
+
+    RenderMultiColumnSet* previousSet = spannerSet->previousSiblingMultiColumnSet();
+    if (!previousSet) {
+        // The first set is entered at the beginning of flow thread layout. If the first set happens
+        // to be a spanner, we have nothing more to do here.
+        return LayoutUnit();
+    }
+
+    RenderBlock* cb = renderer->containingBlock();
+    LayoutUnit logicalTopInFlowThread = cb->offsetFromLogicalTopOfFirstPage() + logicalTop;
+    LayoutUnit adjustment;
+    if (!previousSet->isRenderMultiColumnSpannerSet() && previousSet->pageLogicalHeight()) {
+        // Pad flow thread offset to a column boundary, so that contents that's supposed to come
+        // after the spanner (or the spanner itself) don't bleed into the column preceding the
+        // spanner.
+        LayoutUnit columnLogicalTopInFlowThread = previousSet->pageLogicalTopForOffset(logicalTopInFlowThread);
+        if (columnLogicalTopInFlowThread != logicalTopInFlowThread) {
+            adjustment = columnLogicalTopInFlowThread + previousSet->pageLogicalHeight() - logicalTopInFlowThread;
+            logicalTopInFlowThread += adjustment;
+        }
+    }
+
+    if (!previousSet->isRenderMultiColumnSpannerSet())
+        previousSet->endFlow(logicalTopInFlowThread);
+    spannerSet->beginFlow(logicalTopInFlowThread);
+
+    m_lastSetWorkedOn = spannerSet;
+    return adjustment;
+}
+
+void RenderMultiColumnFlowThread::leaveColumnSpannerAfterLayout(RenderBox* renderer, LayoutUnit logicalBottom)
+{
+    ASSERT(m_lastSetWorkedOn == m_spannerMap.get(renderer));
+
+    RenderBlock* cb = renderer->containingBlock();
+    LayoutUnit logicalBottomInFlowThread = cb->offsetFromLogicalTopOfFirstPage() + logicalBottom;
+    m_lastSetWorkedOn->endFlow(logicalBottomInFlowThread);
+    RenderMultiColumnSet* nextSet = m_lastSetWorkedOn->nextSiblingMultiColumnSet();
+    if (nextSet) {
+        m_lastSetWorkedOn = nextSet;
+        if (!m_lastSetWorkedOn->isRenderMultiColumnSpannerSet())
+            m_lastSetWorkedOn->beginFlow(logicalBottomInFlowThread);
+    }
+}
+
 void RenderMultiColumnFlowThread::flowThreadDescendantWasInserted(RenderObject* descendant)
 {
     // Go through the subtree that was just inserted and create column sets (needed by regular
     // column content) and spanner sets (one needed by each spanner).
     for (RenderObject* renderer = descendant; renderer; renderer = renderer->nextInPreOrder(descendant)) {
         if (containingColumnSpannerSet(renderer))
             continue; // Inside a column spanner set. Nothing to do, then.
         if (descendantIsValidColumnSpanner(renderer)) {
             // This renderer is a spanner, so it needs to establish a spanner set.
             createAndInsertSpannerSet(toRenderBox(renderer));
@@ skipping 15 matching lines @@
 
 void RenderMultiColumnFlowThread::updateLogicalWidth()
 {
     LayoutUnit columnWidth;
     calculateColumnCountAndWidth(columnWidth, m_columnCount);
     setLogicalWidth(columnWidth);
 }
 
 void RenderMultiColumnFlowThread::layout()
 {
+    ASSERT(!m_lastSetWorkedOn);
+    m_lastSetWorkedOn = firstMultiColumnSet();
+    if (m_lastSetWorkedOn)
+        m_lastSetWorkedOn->beginFlow(LayoutUnit());
     RenderFlowThread::layout();
-    if (RenderMultiColumnSet* lastSet = lastMultiColumnSet())
+    if (RenderMultiColumnSet* lastSet = lastMultiColumnSet()) {

[Julien - ping for review, 2014/11/17 23:47:11]

Are we not expecting that m_lastSetWorkedOn to be lastMultiColumnSet() at this
point?

[mstensho (USE GERRIT), 2014/11/18 13:29:26]

Done.

Adding that assertion caused some tests to crash [1]. In order to fix that, I
needed to steal more code from my (no longer so) huge patch (that I've been
breaking into pieces for review). We need to remove spanner sets when a spanner
is removed, or the spanner map will contain dangling pointers (in the test the
pointer happened to be re-used for another (non-spanner) renderer, which caused
flowThreadDescendantWasInserted() to incorrectly believe that a inserted
renderer was a spanner).

I uploaded a separate patch for this particular issue, instead of lumping it
together with a bunch of trivial fixes for the other issues you raised.

[1] One such failing test:
virtual/regionbasedmulticol/fast/multicol/span/generated-child-split-flow-crash.html

+        lastSet->endFlow(logicalHeight());
         lastSet->expandToEncompassFlowThreadContentsIfNeeded();
+    }
+    m_lastSetWorkedOn = 0;
 }
 
 void RenderMultiColumnFlowThread::setPageBreak(LayoutUnit offset, LayoutUnit spaceShortage)
 {
     // Only positive values are interesting (and allowed) here. Zero space shortage may be reported
     // when we're at the top of a column and the element has zero height. Ignore this, and also
     // ignore any negative values, which may occur when we set an early break in order to honor
     // widows in the next column.
     if (spaceShortage <= 0)
         return;
 
     if (RenderMultiColumnSet* multicolSet = columnSetAtBlockOffset(offset))
         multicolSet->recordSpaceShortage(spaceShortage);
 }
 
 void RenderMultiColumnFlowThread::updateMinimumPageHeight(LayoutUnit offset, LayoutUnit minHeight)
 {
     if (RenderMultiColumnSet* multicolSet = columnSetAtBlockOffset(offset))
         multicolSet->updateMinimumColumnHeight(minHeight);
 }
 
-RenderMultiColumnSet* RenderMultiColumnFlowThread::columnSetAtBlockOffset(LayoutUnit /*offset*/) const
+RenderMultiColumnSet* RenderMultiColumnFlowThread::columnSetAtBlockOffset(LayoutUnit offset) const
 {
-    // For now there's only one column set, so this is easy:
-    return firstMultiColumnSet();
+    if (m_lastSetWorkedOn) {
+        // Layout in progress. We are calculating the set heights as we speak, so the column set range
+        // information is not up-to-date.
+        // FIXME: need to check if m_lastSetWorkedOn contains the offset, and if it doesn't, we need
+        // to locate the right set.

[Julien - ping for review, 2014/11/17 23:47:11]

This FIXME is actionable but it's not super clear for an outsider why this is
desirable or which use cases it fixes.

[mstensho (USE GERRIT), 2014/11/18 13:29:26]

Done.

I'll just remove the FIXME. There are no tests that currently depend on this,
including those that I've planned that are not part of this CL.

It's a bit unclear to me whether there are legitimate reasons for dealing with
this. It will typically upset the balancing machinery if we revisit a column set
that we have already finished.

In RenderBlockFlow::adjustBlockChildForPagination() (which is called when
finishing layout of a child) there's a call to pageLogicalHeightForOffset() that
leads to this method, and the offset will typically be outside the bounds of the
current set if the child block in question contains a column spanner:

<div style="-webkit-columns:2;">
    <div id="child" style="height:100px;">
        <div style="-webkit-column:span:all;"></div>
    </div>
</div>

We try to find the available height at the top of the child block, which isn't
in the same column set as the bottom, and record space shortage to the balancer
based on that. Need to revisit this at some point, if it turns out to matter
enough.

+        return m_lastSetWorkedOn;
+    }
+
+    ASSERT(!m_regionsInvalidated);
+    if (offset <= 0)
+        return m_multiColumnSetList.isEmpty() ? 0 : m_multiColumnSetList.first();
+
+    MultiColumnSetSearchAdapter adapter(offset);
+    m_multiColumnSetIntervalTree.allOverlapsWithAdapter<MultiColumnSetSearchAdapter>(adapter);
+
+    // If no set was found, the offset is in the flow thread overflow.
+    if (!adapter.result() && !m_multiColumnSetList.isEmpty())
+        return m_multiColumnSetList.last();
+    return adapter.result();
 }
 
 bool RenderMultiColumnFlowThread::addForcedRegionBreak(LayoutUnit offset, RenderObject* /*breakChild*/, bool /*isBefore*/, LayoutUnit* offsetBreakAdjustment)
 {
     if (RenderMultiColumnSet* multicolSet = columnSetAtBlockOffset(offset)) {
         multicolSet->addContentRun(offset);
         if (offsetBreakAdjustment)
             *offsetBreakAdjustment = pageLogicalHeightForOffset(offset) ? pageRemainingLogicalHeightForOffset(offset, IncludePageBoundary) : LayoutUnit();
         return true;
     }
     return false;
 }
 
 bool RenderMultiColumnFlowThread::isPageLogicalHeightKnown() const
 {
     if (RenderMultiColumnSet* columnSet = lastMultiColumnSet())
         return columnSet->pageLogicalHeight();
     return false;
 }
 
 }