Bug 857324: Make column set reflow continue without balancing rather than restarting if computed height is exceeded. [r=mats]

layout/generic/nsColumnSetFrame.cpp

@@ -301,6 +301,35 @@ nsColumnSetFrame::ChooseColumnStrategy(const nsHTMLReflowState& aReflowState,
   return config;
 }
 
+bool
+nsColumnSetFrame::ReflowColumns(nsHTMLReflowMetrics& aDesiredSize,
+                                const nsHTMLReflowState& aReflowState,
+                                nsReflowStatus& aReflowStatus,
+                                ReflowConfig& aConfig,
+                                bool aLastColumnUnbounded,
+                                nsCollapsingMargin* aCarriedOutBottomMargin,
+                                ColumnBalanceData& aColData)
+{
+  bool feasible = ReflowChildren(aDesiredSize, aReflowState,
+                                 aReflowStatus, aConfig, aLastColumnUnbounded,
+                                 aCarriedOutBottomMargin, aColData);
+
+  if (aColData.mHasExcessHeight) {
+    aConfig = ChooseColumnStrategy(aReflowState, true);
+    MOZ_ASSERT(aReflowState.ComputedHeight() != NS_INTRINSICSIZE,
+                 "computed height cannot be of intrinsic size if we have "
+                 "excess height from a previous reflow");
+
+    // We need to reflow our children again because our size may have changed.
+    // Return value doesn't matter when !mIsBalancing.
+    ReflowChildren(aDesiredSize, aReflowState, aReflowStatus,
+                   aConfig, aLastColumnUnbounded,
+                   aCarriedOutBottomMargin, aColData);
+  }
+
+  return feasible;
+}
+
 // XXX copied from nsBlockFrame, should this be moved to nsContainerFrame?
 static void
 PlaceFrameView(nsIFrame* aFrame)
@@ -617,14 +646,18 @@ nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&     aDesiredSize,
         kidNextInFlow->RemoveStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
       }
 
-
+      // XXX_jwir3:
-      if ((contentBottom > aReflowState.mComputedMaxHeight ||
+      // There are specific situations where contentBottom can exceed
+      // NS_INTRINSICSIZE, which causes a problem, e.g.
+      // in layout/generic/crashtests/479938-1.html
+      if (contentBottom <= NS_INTRINSICSIZE &&
+          (contentBottom > aReflowState.mComputedMaxHeight ||
            contentBottom > aReflowState.ComputedHeight()) &&
            aConfig.mBalanceColCount < INT32_MAX) {
-        // We overflowed vertically, but have not exceeded the number
+        // We overflowed vertically, but have not exceeded the number of
-        // of columns. If we're balancing, then we should try reverting
+        // columns. We're going to go into overflow columns now, so balancing
-        // to auto instead.
+        // no longer applies.
-        aColData.mShouldRevertToAuto = true;
+        aColData.mHasExcessHeight = true;
       }
 
       if (columnCount >= aConfig.mBalanceColCount) {
@@ -794,22 +827,11 @@ nsColumnSetFrame::Reflow(nsPresContext*           aPresContext,
   bool unboundedLastColumn = config.mIsBalancing && !nextInFlow;
   nsCollapsingMargin carriedOutBottomMargin;
   ColumnBalanceData colData;
-  colData.mShouldRevertToAuto = false;
+  colData.mHasExcessHeight = false;
 
-  // This loop exists in order to try balancing initially. If the balancing
+  bool feasible = ReflowColumns(aDesiredSize, aReflowState, aStatus, config,
-  // overflows, then we want to revert to column-fill: auto.
+                                unboundedLastColumn, &carriedOutBottomMargin,
-
+                                colData);
-  // Our loop invariant is: colData.mShouldRevertToAuto is true if and only
-  // if we've reflowed our children, and during the most recent reflow of
-  // children, we were balancing and we overflowed in the block direction.
-  do {
-    if (colData.mShouldRevertToAuto) {
-      config = ChooseColumnStrategy(aReflowState, true);
-      config.mIsBalancing = false;
-    }
-
-    bool feasible = ReflowChildren(aDesiredSize, aReflowState,
-      aStatus, config, unboundedLastColumn, &carriedOutBottomMargin, colData);
 
   if (config.mIsBalancing && !aPresContext->HasPendingInterrupt()) {
     nscoord availableContentHeight = GetAvailableContentHeight(aReflowState);
@@ -916,12 +938,18 @@ nsColumnSetFrame::Reflow(nsPresContext*           aPresContext,
 
       unboundedLastColumn = false;
       AddStateBits(NS_FRAME_IS_DIRTY);
-        feasible = ReflowChildren(aDesiredSize, aReflowState,
+      feasible = ReflowColumns(aDesiredSize, aReflowState, aStatus, config, false,
-                                  aStatus, config, false,
                                &carriedOutBottomMargin, colData);
+
+      if (!config.mIsBalancing) {
+        // Looks like we had excess height when balancing, so we gave up on
+        // trying to balance.
+        break;
+      }
     }
 
-      if (!feasible && !aPresContext->HasPendingInterrupt()) {
+    if (config.mIsBalancing && !feasible &&
+        !aPresContext->HasPendingInterrupt()) {
       // We may need to reflow one more time at the feasible height to
       // get a valid layout.
       bool skip = false;
@@ -939,15 +967,13 @@ nsColumnSetFrame::Reflow(nsPresContext*           aPresContext,
         // Otherwise we'd have to split, and it's not clear what we'd do with
         // that.
         AddStateBits(NS_FRAME_IS_DIRTY);
-          ReflowChildren(aDesiredSize, aReflowState, aStatus, config,
+        ReflowColumns(aDesiredSize, aReflowState, aStatus, config,
                       availableContentHeight == NS_UNCONSTRAINEDSIZE,
                       &carriedOutBottomMargin, colData);
       }
     }
   }
 
- } while (colData.mShouldRevertToAuto);
-
   if (aPresContext->HasPendingInterrupt() &&
       aReflowState.availableHeight == NS_UNCONSTRAINEDSIZE) {
     // In this situation, we might be lying about our reflow status, because

layout/generic/nsColumnSetFrame.h

@@ -137,13 +137,14 @@ protected:
     // The maximum "content height" of all columns that overflowed
     // their available height
     nscoord mMaxOverflowingHeight;
-    // Whether or not we should revert back to 'auto' setting for column-fill.
+    // This flag determines whether the last reflow of children exceeded the
-    // This happens if we overflow our columns such that we no longer have
+    // computed height of the column set frame. If so, we set the height to
-    // enough room to keep balancing.
+    // this maximum allowable height, and continue reflow without balancing.
-    bool mShouldRevertToAuto;
+    bool mHasExcessHeight;
+
     void Reset() {
       mMaxHeight = mSumHeight = mLastHeight = mMaxOverflowingHeight = 0;
-      mShouldRevertToAuto = false;
+      mHasExcessHeight = false;
     }
   };
 
@@ -154,6 +155,14 @@ protected:
    */
   void DrainOverflowColumns();
 
+  bool ReflowColumns(nsHTMLReflowMetrics& aDesiredSize,
+                     const nsHTMLReflowState& aReflowState,
+                     nsReflowStatus& aReflowStatus,
+                     ReflowConfig& aConfig,
+                     bool aLastColumnUnbounded,
+                     nsCollapsingMargin* aCarriedOutBottomMargin,
+                     ColumnBalanceData& aColData);
+
   /**
    * The basic reflow strategy is to call this function repeatedly to
    * obtain specific parameters that determine the layout of the

layout/reftests/columns/columnfill-auto-2-ref.html

@@ -0,0 +1,48 @@
+<!doctype html>
+<html>
+<head>
+<link rel="stylesheet" type="text/css" href="ahem.css" />
+<style>
+  html {
+    line-height: 1.5;
+    font-family: ahem;
+  }
+
+  body {
+    margin: 0;
+    padding: 0;
+  }
+
+  #test {
+    padding: 5px 0px 0px 10px;
+    position: absolute;
+    left: 500px;
+    right: 0px;
+    top: 0px;
+    bottom: 0px;
+    background-color: orange;
+    height: 120px;
+    -moz-column-width: 640px;
+    -moz-column-fill: balance;
+    -moz-column-gap: 0;
+    overflow-x: auto;
+    overflow-y: hidden;
+  }
+
+  #parent {
+    background-color: lightBlue;
+    position: absolute;
+    left: 0px;
+    bottom: 0px;
+    right: 0px;
+    height: 120px;
+    width: 70em;
+  }
+</style>
+</head>
+<body>
+  <div id="parent">
+    <div id="test">To Mrs. Aville, England St. Petersburgh, Dec. 11th, 17- You will rejoice to hear that no disaster has accompanied the commencement of an enterprise which you have regarded with such evil forebodings. I arrived here yesterday; and my first task is to assure my dear sister of my welfare, and increasing confidence in the success of my undertaking. I am already far north of London; and as I walk in the streets of Petersburgh. I feel a cold northern breeze play upon my cheeks, which braces my nerves, and fills me with delight.</div>
+  </div>
+</body>
+</html>

layout/reftests/columns/columnfill-auto-2.html

@@ -0,0 +1,48 @@
+<!doctype html>
+<html>
+<head>
+<link rel="stylesheet" type="text/css" href="ahem.css" />
+<style>
+  html {
+    line-height: 1.5;
+    font-family: ahem;
+  }
+
+  body {
+    margin: 0;
+    padding: 0;
+  }
+
+  #test {
+    padding: 5px 0px 0px 10px;
+    position: absolute;
+    left: 500px;
+    right: 0px;
+    top: 0px;
+    bottom: 0px;
+    background-color: orange;
+    height: 120px;
+    -moz-column-width: 640px;
+    -moz-column-fill: auto;
+    -moz-column-gap: 0;
+    overflow-x: auto;
+    overflow-y: hidden;
+  }
+
+  #parent {
+    background-color: lightBlue;
+    position: absolute;
+    left: 0px;
+    bottom: 0px;
+    right: 0px;
+    height: 120px;
+    width: 70em;
+  }
+</style>
+</head>
+<body>
+  <div id="parent">
+    <div id="test">To Mrs. Aville, England St. Petersburgh, Dec. 11th, 17- You will rejoice to hear that no disaster has accompanied the commencement of an enterprise which you have regarded with such evil forebodings. I arrived here yesterday; and my first task is to assure my dear sister of my welfare, and increasing confidence in the success of my undertaking. I am already far north of London; and as I walk in the streets of Petersburgh. I feel a cold northern breeze play upon my cheeks, which braces my nerves, and fills me with delight.</div>
+  </div>
+</body>
+</html>

layout/reftests/columns/reftest.list

@@ -19,6 +19,7 @@
 == column-box-alignment-rtl.html column-box-alignment-rtl-ref.html
 HTTP(..) == columnfill-balance.html columnfill-balance-ref.html
 HTTP(..) == columnfill-auto.html columnfill-auto-ref.html
+HTTP(..) == columnfill-auto-2.html columnfill-auto-2-ref.html
 skip-if(B2G) == columnrule-basic.html columnrule-basic-ref.html # bug 773482
 skip-if(B2G) == columnrule-complex.html columnrule-complex-ref.html # bug 773482
 != columnrule-linestyles.html columnrule-linestyles-notref.html