May need to insert or remove column sets when out-of-flow objects get their containing block change…

Source/core/layout/LayoutMultiColumnFlowThread.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 736 matching lines @@
         // If there were no adjacent spanners, it has to mean that there's only one column set,
         // since it's only spanners that may cause creation of multiple sets.
         columnSetToRemove = firstMultiColumnSet();
         ASSERT(columnSetToRemove);
         ASSERT(!columnSetToRemove->nextSiblingMultiColumnSet());
     }
     ASSERT(columnSetToRemove);
     columnSetToRemove->destroy();
 }
 
+static inline bool needsToReinsertIntoFlowThread(const ComputedStyle& oldStyle, const ComputedStyle& newStyle)
+{
+    // If we've become (or are about to become) a container for absolutely positioned descendants,

[leviw_travelin_and_unemployed, 2015/07/14 19:43:42]

This comment seems to imply we only care if we're about to become a container
for abs pos descendants, but the code checks if we become or stop being a
container for abs pos.

[mstensho (USE GERRIT), 2015/07/14 19:49:25]

Done.

The code was right, the comment was inaccurate.

+    // we need to re-evaluate the need for column sets. There may be out-of-flow descendants
+    // further down that become part of the flow thread, or cease to be part of the flow thread,
+    // because of this change.
+    if (oldStyle.hasTransformRelatedProperty() != newStyle.hasTransformRelatedProperty())
+        return true;
+    return (oldStyle.hasInFlowPosition() && newStyle.position() == StaticPosition)
+        || (newStyle.hasInFlowPosition() && oldStyle.position() == StaticPosition);
+}
+
+static inline bool needsToRemoveFromFlowThread(const ComputedStyle& oldStyle, const ComputedStyle& newStyle)
+{
+    // If an in-flow descendant goes out-of-flow, we may have to remove column sets and spanner placeholders.
+    return (newStyle.hasOutOfFlowPosition() && !oldStyle.hasOutOfFlowPosition()) || needsToReinsertIntoFlowThread(oldStyle, newStyle);
+}
+
+static inline bool needsToInsertIntoFlowThread(const ComputedStyle& oldStyle, const ComputedStyle& newStyle)
+{
+    // If an out-of-flow descendant goes in-flow, we may have to insert column sets and spanner placeholders.
+    return (!newStyle.hasOutOfFlowPosition() && oldStyle.hasOutOfFlowPosition()) || needsToReinsertIntoFlowThread(oldStyle, newStyle);
+}
+
 void LayoutMultiColumnFlowThread::flowThreadDescendantStyleWillChange(LayoutObject* descendant, StyleDifference diff, const ComputedStyle& newStyle)
 {
-    // If an in-flow descendant goes out-of-flow, we may have to remove a column set.
     if (descendant->isText()) {
         // Text nodes inherit all properties from the parent node (including non-inheritable
         // ones). We don't care what its 'position' is. In fact, we _must_ ignore it, since the
         // parent may be the multicol container, and having that accidentally leaked into children
         // of the multicol is bad.
         return;
     }
-    if (newStyle.hasOutOfFlowPosition() && !styleRef().hasOutOfFlowPosition())
+    if (needsToRemoveFromFlowThread(descendant->styleRef(), newStyle))
         flowThreadDescendantWillBeRemoved(descendant);
 }
 
 void LayoutMultiColumnFlowThread::flowThreadDescendantStyleDidChange(LayoutObject* descendant, StyleDifference diff, const ComputedStyle& oldStyle)
 {
-    // If an out-of-flow descendant goes in-flow, we may have to insert a column set.
     if (descendant->isText()) {
         // Text nodes inherit all properties from the parent node (including non-inheritable
         // ones). We don't care what its 'position' is. In fact, we _must_ ignore it, since the
         // parent may be the multicol container, and having that accidentally leaked into children
         // of the multicol is bad.
         return;
     }
-    if (styleRef().hasOutOfFlowPosition())
-        return;
-
-    // We're not out of flow.
-    if (oldStyle.hasOutOfFlowPosition()) {
-        // ... but we used to be out of flow. So we might need to insert a column set (or
-        // spanner placeholder, in case this descendant is now a valid column spanner).
+    if (needsToInsertIntoFlowThread(oldStyle, descendant->styleRef())) {
         flowThreadDescendantWasInserted(descendant);
         return;
     }
     if (descendantIsValidColumnSpanner(descendant)) {
         // We went from being regular column content to becoming a spanner.
         ASSERT(!toLayoutBox(descendant)->spannerPlaceholder());
 
         // First remove this as regular column content. Note that this will walk the entire subtree
         // of |descendant|. There might be spanners there (which won't be spanners anymore, since
         // we're not allowed to nest spanners), whose placeholders must die.
@@ skipping 91 matching lines @@
 }
 
 bool LayoutMultiColumnFlowThread::isPageLogicalHeightKnown() const
 {
     if (LayoutMultiColumnSet* columnSet = lastMultiColumnSet())
         return columnSet->pageLogicalHeight();
     return false;
 }
 
 }