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

Source/core/rendering/RenderMultiColumnSpannerSet.cpp

Index: Source/core/rendering/RenderMultiColumnSpannerSet.cpp
diff --git a/Source/core/rendering/RenderMultiColumnSpannerSet.cpp b/Source/core/rendering/RenderMultiColumnSpannerSet.cpp
index 69ae4f36398f84e73fb69e9fb029b7a511a12baf..7d60c1b6241276e2ee6f02aafc8daba1609a2336 100644
--- a/Source/core/rendering/RenderMultiColumnSpannerSet.cpp
+++ b/Source/core/rendering/RenderMultiColumnSpannerSet.cpp
@@ -28,6 +28,17 @@ bool RenderMultiColumnSpannerSet::recalculateColumnHeight(BalancedHeightCalculat
     return false;
 }
 
+void RenderMultiColumnSpannerSet::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
+{
+    computedValues.m_extent = m_rendererInFlowThread->logicalHeight();
+    computedValues.m_position = logicalTop;
+}
+
+LayoutUnit RenderMultiColumnSpannerSet::pageLogicalHeight() const
+{
+    return m_rendererInFlowThread->logicalHeight();

[Julien - ping for review, 2014/11/26 21:09:01]

Should we ASSERT that m_rendererInFlowThread doesn't need layout?

More conceptually, it doesn't seem correct to return the logical height of the
content for the page logical height. It's not a paginated value at all.

[mstensho (USE GERRIT), 2014/11/27 21:20:29]

Good idea. Done. But then we obviously have to avoid calling this method before
and during layout. I had to add some guards against column spanners here and
there now, including the ones I removed in patch set 8.
Indeed. This is another unfortunate consequence of RenderMultiColumnSpannerSet
inheriting from RenderMultiColumnSet, and that spanners are part of the flow
thread at all.

This is the weakness of my current proposed implementation of column-span:all;
laying it out inside of the flow thread. My first proposal [1] teleported the
spanner from its DOM position in the tree to becoming a sibling of the column
sets, so that it wouldn't be part of the flow thread at all. Then we wouldn't
have this problem (but we'd have some questionable teleportation magic instead).

[1] https://codereview.chromium.org/296413007/

[Julien - ping for review, 2014/12/03 23:02:22]

Both patches seem as complicated at the end of the day. I still like this a bit
more as it produces a tree that's closer to the DOM.

[mstensho (USE GERRIT), 2014/12/10 13:48:19]

I've found a third way, which I dislike even less. As mentioned in another
comment, if we fix spanner->container() to return the multicol container, we
need to change quite a few things, and for the better, at that. The main benefit
is that we don't have to lay out spanners inside the flow thread and be careful
to walk around out any kind of fragmentation code that could break the spanner.
Margin handling will be easier (when we get to that) too.

See top of comment for new CLs.

+}
+
 const char* RenderMultiColumnSpannerSet::renderName() const
 {
     return "RenderMultiColumnSpannerSet";