Bug 1308876 - Fix ColumnSet to reflow a non-dirty column when the blo…

…ck-size has shrunk and the column might need to push some children to the next column. r=dholbert

This fixes (confirmed by testing locally) a regression in
layout/reftests/w3c-css/received/css-multicol-1/multicol-nested-margin-004.xht
resulting from the primary patch in this bug, which tends to make frames
dirty less often.  The problem with that test is that (at least in a
simplified form), in the final reflow of the inner ColumnSet in the
first column of the outer ColumnSet, the inner ColumnSet chooses not to
reflow its first column, thus leaving that first column having a height
that is too large for the inner ColumnSet to fit in the first column of
the outer ColumnSet, causing the entire inner ColumnSet (rather than
just part of it) to be pushed to the next column.

I believe this existing incremental reflow code just doesn't make sense.
The code I'm modifying dates back primarily to:
c237520 (October 2004, initial columns implementation)
ee070ec (March 2005)
31e3540 (November 2006)

The first thing that doesn't make sense is the condition modified at the
end of this patch:
  (!reflowNext && (skipIncremental || skipResizeBSizeShrink))
There's simply no reason that that || isn't required to be an &&, as far
as I can tell.  Even if we don't need to reflow due to any of the
standard incremental reflow conditions, we can need to reflow because
the block size is shrinking and the column no longer fits.

Note that things were already OK when we required reflow due to
NS_SUBTREE_DIRTY(this), because of the way shrinkingBSizeOnly was
initialized using !NS_SUBTREE_DIRTY(this), thus excluding such cases
from the optimization.

The rest of the patch falls out of turning the || into an && in an
efficient way (i.e., without the extra !NS_SUBTREE_DIRTY(this) test, and
avoiding doing extra tests that we know we're not going to need by
coalescing all the incremental reflow tests into a single variable).

I tested that this patch passes try on its own (on 64-bit Linux debug):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a279023fb7e8f3349d5ecbfb95807d6b097cdbcb

MozReview-Commit-ID: BD3ofmWN5Wl

layout/generic/nsColumnSetFrame.cpp

@@ -605,8 +605,7 @@ nsColumnSetFrame::ReflowChildren(ReflowOutput&     aDesiredSize,
   bool allFit = true;
   WritingMode wm = GetWritingMode();
   bool isRTL = !wm.IsBidiLTR();
-  bool shrinkingBSizeOnly = !NS_SUBTREE_DIRTY(this) &&
-    mLastBalanceBSize > aConfig.mColMaxBSize;
+  bool shrinkingBSize = mLastBalanceBSize > aConfig.mColMaxBSize;
 
 #ifdef DEBUG_roc
   printf("*** Doing column reflow pass: mLastBalanceBSize=%d, mColMaxBSize=%d, RTL=%d\n"
@@ -695,22 +694,22 @@ nsColumnSetFrame::ReflowChildren(ReflowOutput&     aDesiredSize,
     // boundary, but if so, too bad, this optimization is defeated.)
     // We want scrollable overflow here since this is a calculation that
     // affects layout.
-    bool skipResizeBSizeShrink = false;
-    if (shrinkingBSizeOnly) {
+    if (skipIncremental && shrinkingBSize) {
       switch (wm.GetBlockDir()) {
       case WritingMode::eBlockTB:
-        if (child->GetScrollableOverflowRect().YMost() <= aConfig.mColMaxBSize) {
-          skipResizeBSizeShrink = true;
+        if (child->GetScrollableOverflowRect().YMost() > aConfig.mColMaxBSize) {
+          skipIncremental = false;
         }
         break;
       case WritingMode::eBlockLR:
-        if (child->GetScrollableOverflowRect().XMost() <= aConfig.mColMaxBSize) {
-          skipResizeBSizeShrink = true;
+        if (child->GetScrollableOverflowRect().XMost() > aConfig.mColMaxBSize) {
+          skipIncremental = false;
         }
         break;
       case WritingMode::eBlockRL:
         // XXX not sure how to handle this, so for now just don't attempt
         // the optimization
+        skipIncremental = false;
         break;
       default:
         NS_NOTREACHED("unknown block direction");
@@ -719,7 +718,7 @@ nsColumnSetFrame::ReflowChildren(ReflowOutput&     aDesiredSize,
     }
 
     nscoord childContentBEnd = 0;
-    if (!reflowNext && (skipIncremental || skipResizeBSizeShrink)) {
+    if (!reflowNext && skipIncremental) {
       // This child does not need to be reflowed, but we may need to move it
       MoveChildTo(child, childOrigin, wm, containerSize);
 
@@ -737,8 +736,8 @@ nsColumnSetFrame::ReflowChildren(ReflowOutput&     aDesiredSize,
       }
       childContentBEnd = nsLayoutUtils::CalculateContentBEnd(wm, child);
 #ifdef DEBUG_roc
-      printf("*** Skipping child #%d %p (incremental %d, resize block-size shrink %d): status = %d\n",
-             columnCount, (void*)child, skipIncremental, skipResizeBSizeShrink, aStatus);
+      printf("*** Skipping child #%d %p (incremental %d): status = %d\n",
+             columnCount, (void*)child, skipIncremental, aStatus);
 #endif
     } else {
       LogicalSize availSize(wm, aConfig.mColISize, aConfig.mColMaxBSize);