Bug 1809630 - Make thin scrollbars on Windows have a reasonable minimum size. r=tnikkel,layout-reviewers

The way the code was set up before this patch ends up causing minimum
scrollbar sizes to be 0x0 for thin (non-overlay) scrollbars.

This is rather problematic, since it means that we would always try to
place the scrollbar even if it doesn't fit (think of an element with
height: 0).

This causes a lot of extra reflow, which with very complex layouts is
even worse, because the extra scrollframe reflows cause us to miss the
flex caches, causing O(n^2) performance.

Add assertions to make sure we never end up with a zero minimum
scrollbar size, and change the size computation to match for both
thin and thick scrollbars.

Differential Revision: https://phabricator.services.mozilla.com/D166756

layout/generic/nsGfxScrollFrame.cpp

@@ -442,6 +442,19 @@ ScrollReflowInput::ScrollReflowInput(nsHTMLScrollFrame* aFrame,
 
     GetScrollbarMetrics(mBoxState, hScrollbarBox, &mHScrollbarMinSize,
                         &mHScrollbarPrefSize);
+
+    // A zero minimum size is a bug with non-overlay scrollbars. That
+    // means we'll always try to place the scrollbar, even if it will ultimately
+    // not fit, see bug 1809630. XUL collapsing is the exception because the
+    // front-end uses it.
+    MOZ_ASSERT(aFrame->PresContext()->UseOverlayScrollbars() ||
+                   hScrollbarBox->IsXULCollapsed() ||
+                   (mHScrollbarMinSize.width && mHScrollbarMinSize.height),
+               "Shouldn't have a zero horizontal min-scrollbar-size");
+    MOZ_ASSERT(mHScrollbarPrefSize.width >= mHScrollbarMinSize.width &&
+                   mHScrollbarPrefSize.height >= mHScrollbarMinSize.height,
+               "Scrollbar pref size should be >= min size");
+
   } else {
     mHScrollbar = ShowScrollbar::Never;
     mHScrollbarAllowedForScrollingVVInsideLV = false;
@@ -452,6 +465,15 @@ ScrollReflowInput::ScrollReflowInput(nsHTMLScrollFrame* aFrame,
 
     GetScrollbarMetrics(mBoxState, vScrollbarBox, &mVScrollbarMinSize,
                         &mVScrollbarPrefSize);
+
+    // See above.
+    MOZ_ASSERT(aFrame->PresContext()->UseOverlayScrollbars() ||
+                   vScrollbarBox->IsXULCollapsed() ||
+                   (mVScrollbarMinSize.width && mVScrollbarMinSize.height),
+               "Shouldn't have a zero vertical min-size");
+    MOZ_ASSERT(mVScrollbarPrefSize.width >= mVScrollbarMinSize.width &&
+                   mVScrollbarPrefSize.height >= mVScrollbarMinSize.height,
+               "Scrollbar pref size should be >= min size");
   } else {
     mVScrollbar = ShowScrollbar::Never;
     mVScrollbarAllowedForScrollingVVInsideLV = false;

widget/ScrollbarDrawingWin.cpp

@@ -33,11 +33,6 @@ LayoutDeviceIntSize ScrollbarDrawingWin::GetMinimumWidgetSize(
     case StyleAppearance::ScrollbarHorizontal:
     case StyleAppearance::ScrollbarthumbVertical:
     case StyleAppearance::ScrollbarthumbHorizontal: {
-      if ((aAppearance == StyleAppearance::ScrollbarHorizontal ||
-           aAppearance == StyleAppearance::ScrollbarVertical) &&
-          !aPresContext->UseOverlayScrollbars()) {
-        return LayoutDeviceIntSize{};
-      }
       // TODO: for short scrollbars it could be nice if the thumb could shrink
       // under this size.
       auto sizes = GetScrollbarSizes(aPresContext, aFrame);
@@ -46,8 +41,22 @@ LayoutDeviceIntSize ScrollbarDrawingWin::GetMinimumWidgetSize(
           aAppearance == StyleAppearance::ScrollbarthumbHorizontal ||
           aAppearance == StyleAppearance::ScrollbarbuttonLeft ||
           aAppearance == StyleAppearance::ScrollbarbuttonRight;
-      const auto size = isHorizontal ? sizes.mHorizontal : sizes.mVertical;
-      return LayoutDeviceIntSize{size, size};
+      const auto relevantSize =
+          isHorizontal ? sizes.mHorizontal : sizes.mVertical;
+      auto size = LayoutDeviceIntSize{relevantSize, relevantSize};
+      if (aAppearance == StyleAppearance::ScrollbarHorizontal ||
+          aAppearance == StyleAppearance::ScrollbarVertical) {
+        // Always reserve some space in the right direction. Historically we've
+        // reserved 3 times the size in the other axis (one for the thumb, two
+        // for the buttons). We do this even when painting thin scrollbars just
+        // for consistency, though there just one would probably do.
+        if (isHorizontal) {
+          size.width *= 3;
+        } else {
+          size.height *= 3;
+        }
+      }
+      return size;
     }
     default:
       return LayoutDeviceIntSize{};