Bug 407095. Reflow in more situations where horizontal scrollbar appears or disappears. May hurt Tp, let's watch. r+sr=dbaron,a=damon

layout/generic/nsGfxScrollFrame.cpp

@@ -336,8 +336,8 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowState* aState,
   }
 
   if (aAssumeVScroll != aState->mReflowedContentsWithVScrollbar ||
-      ((mInner.mScrolledFrame->GetStateBits() & NS_FRAME_CONTAINS_RELATIVE_HEIGHT) &&
-       aAssumeHScroll != aState->mReflowedContentsWithHScrollbar)) {
+      (aAssumeHScroll != aState->mReflowedContentsWithHScrollbar &&
+       ScrolledContentDependsOnHeight(aState))) {
     nsresult rv = ReflowScrolledFrame(aState, aAssumeHScroll, aAssumeVScroll,
                                       aKidMetrics, PR_FALSE);
     if (NS_FAILED(rv)) {
@@ -421,6 +421,17 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowState* aState,
   return PR_TRUE;
 }
 
+PRBool
+nsHTMLScrollFrame::ScrolledContentDependsOnHeight(ScrollReflowState* aState)
+{
+  // Return true if ReflowScrolledFrame is going to do something different
+  // based on the presence of a horizontal scrollbar.
+  return (mInner.mScrolledFrame->GetStateBits() & NS_FRAME_CONTAINS_RELATIVE_HEIGHT) ||
+    aState->mReflowState.ComputedHeight() != NS_UNCONSTRAINEDSIZE ||
+    aState->mReflowState.mComputedMinHeight > 0 ||
+    aState->mReflowState.mComputedMaxHeight != NS_UNCONSTRAINEDSIZE;
+}
+
 nsresult
 nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState* aState,
                                        PRBool aAssumeHScroll,
@@ -508,6 +519,16 @@ nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState* aState,
   return rv;
 }
 
+PRBool
+nsHTMLScrollFrame::GuessHScrollbarNeeded(const ScrollReflowState& aState)
+{
+  if (aState.mStyles.mHorizontal != NS_STYLE_OVERFLOW_AUTO)
+    // no guessing required
+    return aState.mStyles.mHorizontal == NS_STYLE_OVERFLOW_SCROLL;
+
+  return mInner.mHasHorizontalScrollbar;
+}
+
 PRBool
 nsHTMLScrollFrame::GuessVScrollbarNeeded(const ScrollReflowState& aState)
 {
@@ -565,7 +586,7 @@ nsHTMLScrollFrame::ReflowContents(ScrollReflowState* aState,
                                   const nsHTMLReflowMetrics& aDesiredSize)
 {
   nsHTMLReflowMetrics kidDesiredSize(aDesiredSize.mFlags);
-  nsresult rv = ReflowScrolledFrame(aState, mInner.mHasHorizontalScrollbar,
+  nsresult rv = ReflowScrolledFrame(aState, GuessHScrollbarNeeded(*aState),
       GuessVScrollbarNeeded(*aState), &kidDesiredSize, PR_TRUE);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -607,22 +628,17 @@ nsHTMLScrollFrame::ReflowContents(ScrollReflowState* aState,
   // Do this first because changing the vertical scrollbar setting is expensive,
   // forcing a reflow always.
 
-  // First try a layout without a horizontal scrollbar, then with, except that
-  // if RELATIVE_HEIGHT is set then for performance we should try the status quo
-  // first. If RELATIVE_HEIGHT is not set then trying no horizontal scrollbar
-  // first is almost free.
-  PRBool firstHScrollbarState =
-    (mInner.mScrolledFrame->GetStateBits() & NS_FRAME_CONTAINS_RELATIVE_HEIGHT)
-        ? aState->mReflowedContentsWithHScrollbar : PR_FALSE;
-  if (TryLayout(aState, &kidDesiredSize, firstHScrollbarState,
+  // Try leaving the horizontal scrollbar unchanged first. This will be more
+  // efficient.
+  if (TryLayout(aState, &kidDesiredSize, aState->mReflowedContentsWithHScrollbar,
                 aState->mReflowedContentsWithVScrollbar, PR_FALSE, &rv))
     return NS_OK;
-  if (TryLayout(aState, &kidDesiredSize, !firstHScrollbarState,
+  if (TryLayout(aState, &kidDesiredSize, !aState->mReflowedContentsWithHScrollbar,
                 aState->mReflowedContentsWithVScrollbar, PR_FALSE, &rv))
     return NS_OK;
 
   // OK, now try toggling the vertical scrollbar. The performance advantage
-  // of trying the status-quo horizontal scrollbar state for RELATIVE_HEIGHT cases
+  // of trying the status-quo horizontal scrollbar state
   // does not exist here (we'll have to reflow due to the vertical scrollbar
   // change), so always try no horizontal scrollbar first.
   PRBool newVScrollbarState = !aState->mReflowedContentsWithVScrollbar;

layout/generic/nsGfxScrollFrame.h

@@ -266,6 +266,7 @@ public:
                    nsHTMLReflowMetrics* aKidMetrics,
                    PRBool aAssumeVScroll, PRBool aAssumeHScroll,
                    PRBool aForce, nsresult* aResult);
+  PRBool ScrolledContentDependsOnHeight(ScrollReflowState* aState);
   nsresult ReflowScrolledFrame(ScrollReflowState* aState,
                                PRBool aAssumeHScroll,
                                PRBool aAssumeVScroll,
@@ -388,6 +389,7 @@ protected:
   void SetSuppressScrollbarUpdate(PRBool aSuppress) {
     mInner.mSupppressScrollbarUpdate = aSuppress;
   }
+  PRBool GuessHScrollbarNeeded(const ScrollReflowState& aState);
   PRBool GuessVScrollbarNeeded(const ScrollReflowState& aState);
   nsSize GetScrollPortSize() const
   {

layout/reftests/bugs/407095-1-ref.html

@@ -0,0 +1,9 @@
+<DOCTYPE html>
+
+<html><body style="width: 400px;">
+
+<div style="overflow:scroll; background: yellow; height: 200px;">
+     
+</div>
+
+</body></html>

layout/reftests/bugs/407095-1.html

@@ -0,0 +1,9 @@
+<DOCTYPE html>
+
+<html><body style="width: 400px;" onload="document.documentElement.appendChild(document.body)">
+
+<div style="overflow:scroll; background: yellow; height: 200px;">
+     
+</div>
+
+</body></html>

layout/reftests/bugs/reftest.list

@@ -687,6 +687,7 @@ random == 403134-1.html 403134-1-ref.html # bug 405377
 == 407016-1-b.html 407016-1-ref.html
 == 407016-2.html 407016-2-ref.html
 == 407078-1.html 407078-1-ref.html
+== 407095-1.html 407095-1-ref.html
 == 407111-1.html 407111-1-ref.html
 == 407227-1.html 407227-1-ref.html
 == 407937-1.html 407937-1-ref.html