Bug 1475033 part 2 - Add a ShowScrollbar enum to be used in ScrollReflowInput. r?mstange draft
authorXidorn Quan <me@upsuper.org>
Fri, 03 Aug 2018 17:44:15 +1000
changeset 829837 e1d6f590fef8dda4db589bc16f74ff21e7e6badf
parent 829836 08831f306cf1b495e4a647746d58eac0e7e7e3d9
child 829838 ccf885306efbdfc969ce066b389d6f7c2c2c9918
child 829845 8f341de50b2eb819b5790d94748f18c0a2f80a9a
child 829851 08584543bcfaeda6fc946adda61cc04894c20617
push id118795
push userxquan@mozilla.com
push dateThu, 16 Aug 2018 22:58:08 +0000
reviewersmstange
bugs1475033
milestone63.0a1
Bug 1475033 part 2 - Add a ShowScrollbar enum to be used in ScrollReflowInput. r?mstange Overflow properties have two semantics nowadays: 1. controlling whether the scrollbar should be shown; 2. controlling whether the content is scrollable. However, with the scrollbar-width property being added, scrollability and presence of scrollbar no longer binds together. This change attempts to draw a boundary between value of overflow and presence of scrollbar by making it clear that for ScrollReflowInput, we only care about whether scrollbar should be shown. This should make it easier to reason about further changes involving presence of scrollbar. MozReview-Commit-ID: 2E964z0SkW4
layout/generic/nsGfxScrollFrame.cpp
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -208,20 +208,42 @@ nsHTMLScrollFrame::GetSplittableType() c
 /**
  HTML scrolling implementation
 
  All other things being equal, we prefer layouts with fewer scrollbars showing.
 */
 
 namespace mozilla {
 
+enum class ShowScrollbar : uint8_t
+{
+  Auto,
+  Always,
+  Never,
+};
+
+static ShowScrollbar
+ShouldShowScrollbar(uint8_t aOverflow)
+{
+  switch (aOverflow) {
+    case NS_STYLE_OVERFLOW_SCROLL:
+      return ShowScrollbar::Always;
+    case NS_STYLE_OVERFLOW_HIDDEN:
+      return ShowScrollbar::Never;
+    default:
+    case NS_STYLE_OVERFLOW_AUTO:
+      return ShowScrollbar::Auto;
+  }
+}
+
 struct MOZ_STACK_CLASS ScrollReflowInput {
   const ReflowInput& mReflowInput;
   nsBoxLayoutState mBoxState;
-  ScrollStyles mStyles;
+  ShowScrollbar mHScrollbar;
+  ShowScrollbar mVScrollbar;
   nsMargin mComputedBorder;
 
   // === Filled in by ReflowScrolledFrame ===
   nsOverflowAreas mContentsOverflowAreas;
   MOZ_INIT_OUTSIDE_CTOR
   bool mReflowedContentsWithHScrollbar;
   MOZ_INIT_OUTSIDE_CTOR
   bool mReflowedContentsWithVScrollbar;
@@ -232,22 +254,26 @@ struct MOZ_STACK_CLASS ScrollReflowInput
   // Whether we decided to show the horizontal scrollbar
   MOZ_INIT_OUTSIDE_CTOR
   bool mShowHScrollbar;
   // Whether we decided to show the vertical scrollbar
   MOZ_INIT_OUTSIDE_CTOR
   bool mShowVScrollbar;
 
   ScrollReflowInput(nsIScrollableFrame* aFrame,
-                    const ReflowInput& aReflowInput) :
-    mReflowInput(aReflowInput),
+                    const ReflowInput& aReflowInput)
+  : mReflowInput(aReflowInput)
     // mBoxState is just used for scrollbars so we don't need to
     // worry about the reflow depth here
-    mBoxState(aReflowInput.mFrame->PresContext(), aReflowInput.mRenderingContext, 0),
-    mStyles(aFrame->GetScrollStyles()) {
+  , mBoxState(aReflowInput.mFrame->PresContext(),
+              aReflowInput.mRenderingContext)
+  {
+    ScrollStyles styles = aFrame->GetScrollStyles();
+    mHScrollbar = ShouldShowScrollbar(styles.mHorizontal);
+    mVScrollbar = ShouldShowScrollbar(styles.mVertical);
   }
 };
 
 } // namespace mozilla
 
 // XXXldb Can this go away?
 static nsSize ComputeInsideBorderSize(ScrollReflowInput* aState,
                                       const nsSize& aDesiredInsideBorderSize)
@@ -325,18 +351,18 @@ GetScrollbarMetrics(nsBoxLayoutState& aS
  * @param aForce if true, then we just assume the layout is consistent.
  */
 bool
 nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState,
                              ReflowOutput* aKidMetrics,
                              bool aAssumeHScroll, bool aAssumeVScroll,
                              bool aForce)
 {
-  if ((aState->mStyles.mVertical == NS_STYLE_OVERFLOW_HIDDEN && aAssumeVScroll) ||
-      (aState->mStyles.mHorizontal == NS_STYLE_OVERFLOW_HIDDEN && aAssumeHScroll)) {
+  if ((aState->mVScrollbar == ShowScrollbar::Never && aAssumeVScroll) ||
+      (aState->mHScrollbar == ShowScrollbar::Never && aAssumeHScroll)) {
     NS_ASSERTION(!aForce, "Shouldn't be forcing a hidden scrollbar to show!");
     return false;
   }
 
   if (aAssumeVScroll != aState->mReflowedContentsWithVScrollbar ||
       (aAssumeHScroll != aState->mReflowedContentsWithHScrollbar &&
        ScrolledContentDependsOnHeight(aState))) {
     if (aAssumeHScroll != aState->mReflowedContentsWithHScrollbar) {
@@ -397,31 +423,31 @@ nsHTMLScrollFrame::TryLayout(ScrollReflo
 
   nsRect scrolledRect =
     mHelper.GetUnsnappedScrolledRectInternal(aState->mContentsOverflowAreas.ScrollableOverflow(),
                                              scrollPortSize);
   nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1);
 
   if (!aForce) {
     // If the style is HIDDEN then we already know that aAssumeHScroll is false
-    if (aState->mStyles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN) {
+    if (aState->mHScrollbar != ShowScrollbar::Never) {
       bool wantHScrollbar =
-        aState->mStyles.mHorizontal == NS_STYLE_OVERFLOW_SCROLL ||
+        aState->mHScrollbar == ShowScrollbar::Always ||
         scrolledRect.XMost() >= visualViewportSize.width + oneDevPixel ||
         scrolledRect.x <= -oneDevPixel;
       if (scrollPortSize.width < hScrollbarMinSize.width)
         wantHScrollbar = false;
       if (wantHScrollbar != aAssumeHScroll)
         return false;
     }
 
     // If the style is HIDDEN then we already know that aAssumeVScroll is false
-    if (aState->mStyles.mVertical != NS_STYLE_OVERFLOW_HIDDEN) {
+    if (aState->mVScrollbar != ShowScrollbar::Never) {
       bool wantVScrollbar =
-        aState->mStyles.mVertical == NS_STYLE_OVERFLOW_SCROLL ||
+        aState->mVScrollbar == ShowScrollbar::Always ||
         scrolledRect.YMost() >= visualViewportSize.height + oneDevPixel ||
         scrolledRect.y <= -oneDevPixel;
       if (scrollPortSize.height < vScrollbarMinSize.height)
         wantVScrollbar = false;
       if (wantVScrollbar != aAssumeVScroll)
         return false;
     }
   }
@@ -430,19 +456,19 @@ nsHTMLScrollFrame::TryLayout(ScrollReflo
     if (!mHelper.mIsRoot) {
       break;
     }
     // Check whether there is actually any overflow.
     nscoord scrolledWidth = scrolledRect.width + oneDevPixel;
     if (scrolledWidth <= scrollPortSize.width) {
       break;
     }
-    // Viewport scrollbar style is used below instead of aState->mStyles
-    // because the latter can be affected by various factors, while we
-    // only care about what the page itself specifies.
+    // Viewport scrollbar style is used below instead of aState because
+    // the latter can be affected by various factors, while we only
+    // care about what the page itself specifies.
     nsPresContext* pc = PresContext();
     ScrollStyles styles = pc->GetViewportScrollStylesOverride();
     if (styles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN) {
       break;
     }
     // Only top level content document is considered.
     nsIDocument* doc = pc->Document();
     if (!doc->IsTopLevelContentDocument()) {
@@ -631,29 +657,30 @@ nsHTMLScrollFrame::ReflowScrolledFrame(S
   aState->mContentsOverflowAreas = aMetrics->mOverflowAreas;
   aState->mReflowedContentsWithHScrollbar = aAssumeHScroll;
   aState->mReflowedContentsWithVScrollbar = aAssumeVScroll;
 }
 
 bool
 nsHTMLScrollFrame::GuessHScrollbarNeeded(const ScrollReflowInput& aState)
 {
-  if (aState.mStyles.mHorizontal != NS_STYLE_OVERFLOW_AUTO)
+  if (aState.mHScrollbar != ShowScrollbar::Auto) {
     // no guessing required
-    return aState.mStyles.mHorizontal == NS_STYLE_OVERFLOW_SCROLL;
-
+    return aState.mHScrollbar == ShowScrollbar::Always;
+  }
   return mHelper.mHasHorizontalScrollbar;
 }
 
 bool
 nsHTMLScrollFrame::GuessVScrollbarNeeded(const ScrollReflowInput& aState)
 {
-  if (aState.mStyles.mVertical != NS_STYLE_OVERFLOW_AUTO)
+  if (aState.mVScrollbar != ShowScrollbar::Auto) {
     // no guessing required
-    return aState.mStyles.mVertical == NS_STYLE_OVERFLOW_SCROLL;
+    return aState.mVScrollbar == ShowScrollbar::Always;
+  }
 
   // If we've had at least one non-initial reflow, then just assume
   // the state of the vertical scrollbar will be what we determined
   // last time.
   if (mHelper.mHadNonInitialReflow) {
     return mHelper.mHasVerticalScrollbar;
   }
 
@@ -718,18 +745,18 @@ nsHTMLScrollFrame::ReflowContents(Scroll
 
   // Detecting when we enter this special case is important for when
   // people design layouts that exactly fit the container "most of the
   // time".
 
   // XXX Is this check really sufficient to catch all the incremental cases
   // where the ideal case doesn't have a scrollbar?
   if ((aState->mReflowedContentsWithHScrollbar || aState->mReflowedContentsWithVScrollbar) &&
-      aState->mStyles.mVertical != NS_STYLE_OVERFLOW_SCROLL &&
-      aState->mStyles.mHorizontal != NS_STYLE_OVERFLOW_SCROLL) {
+      aState->mVScrollbar != ShowScrollbar::Always &&
+      aState->mHScrollbar != ShowScrollbar::Always) {
     nsSize insideBorderSize =
       ComputeInsideBorderSize(aState,
                               nsSize(kidDesiredSize.Width(), kidDesiredSize.Height()));
     nsRect scrolledRect =
       mHelper.GetUnsnappedScrolledRectInternal(kidDesiredSize.ScrollableOverflow(),
                                                insideBorderSize);
     if (nsRect(nsPoint(0, 0), insideBorderSize).Contains(scrolledRect)) {
       // Let's pretend we had no scrollbars coming in here
@@ -760,18 +787,18 @@ nsHTMLScrollFrame::ReflowContents(Scroll
     return;
   if (TryLayout(aState, &kidDesiredSize, true, newVScrollbarState, false))
     return;
 
   // OK, we're out of ideas. Try again enabling whatever scrollbars we can
   // enable and force the layout to stick even if it's inconsistent.
   // This just happens sometimes.
   TryLayout(aState, &kidDesiredSize,
-            aState->mStyles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN,
-            aState->mStyles.mVertical != NS_STYLE_OVERFLOW_HIDDEN,
+            aState->mHScrollbar != ShowScrollbar::Never,
+            aState->mVScrollbar != ShowScrollbar::Never,
             true);
 }
 
 void
 nsHTMLScrollFrame::PlaceScrollArea(ScrollReflowInput& aState,
                                    const nsPoint& aScrollPosition)
 {
   nsIFrame *scrolledFrame = mHelper.mScrolledFrame;
@@ -1032,20 +1059,22 @@ nsHTMLScrollFrame::Reflow(nsPresContext*
   DISPLAY_REFLOW(aPresContext, this, aReflowInput, aDesiredSize, aStatus);
   MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
 
   mHelper.HandleScrollbarStyleSwitching();
 
   ScrollReflowInput state(this, aReflowInput);
   // sanity check: ensure that if we have no scrollbar, we treat it
   // as hidden.
-  if (!mHelper.mVScrollbarBox || mHelper.mNeverHasVerticalScrollbar)
-    state.mStyles.mVertical = NS_STYLE_OVERFLOW_HIDDEN;
-  if (!mHelper.mHScrollbarBox || mHelper.mNeverHasHorizontalScrollbar)
-    state.mStyles.mHorizontal = NS_STYLE_OVERFLOW_HIDDEN;
+  if (!mHelper.mVScrollbarBox || mHelper.mNeverHasVerticalScrollbar) {
+    state.mVScrollbar = ShowScrollbar::Never;
+  }
+  if (!mHelper.mHScrollbarBox || mHelper.mNeverHasHorizontalScrollbar) {
+    state.mHScrollbar = ShowScrollbar::Never;
+  }
 
   //------------ Handle Incremental Reflow -----------------
   bool reflowHScrollbar = true;
   bool reflowVScrollbar = true;
   bool reflowScrollCorner = true;
   if (!aReflowInput.ShouldReflowAllKids()) {
     #define NEEDS_REFLOW(frame_) \
       ((frame_) && NS_SUBTREE_DIRTY(frame_))