Bug 1128102. Don't reconstruct the frame for scroll-snap-type or scroll-behavior style changes. r=emilio

The reason these used to have to reconstruct the frame was so that we called nsPresContext::UpdateViewportScrollStylesOverride to update nsPresContext::mViewportScrollStyles. That field used to store scroll behavior and scroll snap data, it no longer does.

In the current code mScrollBehavior is only checked here https://searchfox.org/mozilla-central/rev/d34f9713ae128a3138c2b70d8041a535f1049d19/layout/generic/nsGfxScrollFrame.cpp#8111 before we perform a new scroll, so no change is needed, any future scroll operation will look in the correct place.

scroll-snap-type is similar except we also store it on the FrameMetrics we send to the compositor so we need to repaint.

Note that even currently where we reconstruct the frame on scroll-snap-type changes we do not perform snapping if we move from none to mandatory or proximity, this is bug 1530253. So by removing the frame reconstruct we are not regressing that since we currently fail to do it.

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

layout/style/nsStyleStruct.cpp

@@ -2390,8 +2390,6 @@ nsChangeHint nsStyleDisplay::CalcDifference(
     const nsStyleDisplay& aNewData, const nsStylePosition& aOldPosition) const {
   if (mDisplay != aNewData.mDisplay || mContain != aNewData.mContain ||
       (mFloat == StyleFloat::None) != (aNewData.mFloat == StyleFloat::None) ||
-      mScrollBehavior != aNewData.mScrollBehavior ||
-      mScrollSnapType != aNewData.mScrollSnapType ||
       mTopLayer != aNewData.mTopLayer || mResize != aNewData.mResize) {
     return nsChangeHint_ReconstructFrame;
   }
@@ -2447,6 +2445,13 @@ nsChangeHint nsStyleDisplay::CalcDifference(
     // FIXME: Bug 1530253 Support re-snapping when scroll-snap-align changes.
     hint |= nsChangeHint_NeutralChange;
   }
+  if (mScrollSnapType != aNewData.mScrollSnapType) {
+    // FIXME: Bug 1530253 Support re-snapping when scroll-snap-type changes.
+    hint |= nsChangeHint_RepaintFrame;
+  }
+  if (mScrollBehavior != aNewData.mScrollBehavior) {
+    hint |= nsChangeHint_NeutralChange;
+  }
 
   if (mOverflowX != aNewData.mOverflowX || mOverflowY != aNewData.mOverflowY) {
     const bool isScrollable = IsScrollableOverflow();
@@ -2492,22 +2497,6 @@ nsChangeHint nsStyleDisplay::CalcDifference(
     }
   }
 
-  /* Note: When mScrollBehavior or mScrollSnapType are changed,
-   * nsChangeHint_NeutralChange is not sufficient to enter
-   * nsCSSFrameConstructor::PropagateScrollToViewport. By using the same hint as
-   * used when the overflow css property changes, nsChangeHint_ReconstructFrame,
-   * PropagateScrollToViewport will be called.
-   *
-   * The scroll-behavior css property is not expected to change often (the
-   * CSSOM-View DOM methods are likely to be used in those cases); however,
-   * if this does become common perhaps a faster-path might be worth while.
-   *
-   * FIXME(emilio): Can we do what we do for overflow changes?
-   *
-   * FIXME(emilio): These properties no longer propagate from the body to the
-   * viewport.
-   */
-
   if (mFloat != aNewData.mFloat) {
     // Changing which side we're floating on (float:none was handled above).
     hint |= nsChangeHint_ReflowHintsForFloatAreaChange;

layout/style/test/test_dynamic_change_causing_reflow.html

@@ -410,6 +410,78 @@ const gTestcases = [
     expectConstruction: true,
     expectReflow: true,
   },
+  // changing scroll-behavior should not cause reflow or frame construction
+  {
+    elem: document.documentElement,
+    /* beforeStyle: implicitly 'scroll-behavior: auto' */
+    afterStyle:  "scroll-behavior: smooth",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.documentElement,
+    beforeStyle: "scroll-behavior: smooth",
+    afterStyle:  "scroll-behavior: auto",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'scroll-behavior: auto' */
+    afterStyle:  "scroll-behavior: smooth",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "scroll-behavior: smooth",
+    afterStyle:  "scroll-behavior: auto",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  // changing scroll-snap-type should not cause reflow or frame construction
+  {
+    elem: document.documentElement,
+    /* beforeStyle: implicitly 'scroll-snap-type: none' */
+    afterStyle:  "scroll-snap-type: y mandatory",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.documentElement,
+    /* beforeStyle: implicitly 'scroll-snap-type: none' */
+    afterStyle:  "scroll-snap-type: x proximity",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.documentElement,
+    beforeStyle: "scroll-snap-type: y mandatory",
+    afterStyle:  "scroll-snap-type: none",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'scroll-snap-type: none' */
+    afterStyle:  "scroll-snap-type: y mandatory",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'scroll-snap-type: none' */
+    afterStyle:  "scroll-snap-type: x proximity",
+    expectConstruction: false,
+    expectReflow: false,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "scroll-snap-type: y mandatory",
+    afterStyle:  "scroll-snap-type: none",
+    expectConstruction: false,
+    expectReflow: false,
+  },
 ];
 
 // Helper function to let us call either "is" or "isnot" & assemble