Bug 1305957 part 8 - Suppress scroll offset adjustment when some layout affecting properties are changed on scroll anchor or its ancestors. r=hiro
authorRyan Hunt <rhunt@eqrion.net>
Tue, 27 Nov 2018 15:38:43 -0600
changeset 510492 eef9ed24a0613d3b80ae09074484d4678deb2a2e
parent 510491 a4257d0470d79d33b1e45e974bb4515dc036ee2e
child 510493 3f00aed52fb34af7937d7c4a62265b2ea886b4b5
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1305957
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1305957 part 8 - Suppress scroll offset adjustment when some layout affecting properties are changed on scroll anchor or its ancestors. r=hiro This commit implements the first half of the heuristics to detect style changes that could lead to feedback loops with scroll anchoring. [1] When these style changes are made, a suppression flag is added to the anchor container to ignore any adjustments that would be made at the next layout flush and to invalidate the anchor at that time. [1] https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers Differential Revision: https://phabricator.services.mozilla.com/D13271
layout/generic/ScrollAnchorContainer.cpp
layout/generic/ScrollAnchorContainer.h
layout/generic/nsFrame.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/generic/ScrollAnchorContainer.cpp
+++ b/layout/generic/ScrollAnchorContainer.cpp
@@ -16,17 +16,18 @@
 namespace mozilla {
 namespace layout {
 
 ScrollAnchorContainer::ScrollAnchorContainer(ScrollFrameHelper* aScrollFrame)
     : mScrollFrame(aScrollFrame),
       mAnchorNode(nullptr),
       mLastAnchorPos(0, 0),
       mAnchorNodeIsDirty(true),
-      mApplyingAnchorAdjustment(false) {}
+      mApplyingAnchorAdjustment(false),
+      mSuppressAnchorAdjustment(false) {}
 
 ScrollAnchorContainer::~ScrollAnchorContainer() {}
 
 ScrollAnchorContainer* ScrollAnchorContainer::FindFor(nsIFrame* aFrame) {
   aFrame = aFrame->GetParent();
   if (!aFrame) {
     return nullptr;
   }
--- a/layout/generic/ScrollAnchorContainer.h
+++ b/layout/generic/ScrollAnchorContainer.h
@@ -67,16 +67,22 @@ class ScrollAnchorContainer final {
 
   /**
    * Notify the scroll anchor container that a reflow has happened and it
    * should query its anchor to see if a scroll adjustment needs to occur.
    */
   void ApplyAdjustments();
 
   /**
+   * Notify the scroll anchor container that it should suppress any scroll
+   * adjustment that may happen after the next layout flush.
+   */
+  void SuppressAdjustments();
+
+  /**
    * Notify this scroll anchor container that its anchor node should be
    * invalidated and recomputed at the next available opportunity.
    */
   void InvalidateAnchor();
 
   /**
    * Notify this scroll anchor container that it will be destroyed along with
    * its parent frame.
@@ -130,14 +136,16 @@ class ScrollAnchorContainer final {
   // frame. This is used for calculating the distance to scroll to keep the
   // anchor node in the same relative position
   nsPoint mLastAnchorPos;
 
   // True if we should recalculate our anchor node at the next chance
   bool mAnchorNodeIsDirty : 1;
   // True if we are applying a scroll anchor adjustment
   bool mApplyingAnchorAdjustment : 1;
+  // True if we should suppress anchor adjustments
+  bool mSuppressAnchorAdjustment : 1;
 };
 
 }  // namespace layout
 }  // namespace mozilla
 
 #endif  // mozilla_layout_ScrollAnchorContainer_h_
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1048,47 +1048,81 @@ void nsIFrame::MarkNeedsDisplayItemRebui
   AddAndRemoveImageAssociations(this, oldLayers, newLayers);
 
   oldLayers =
       aOldComputedStyle ? &aOldComputedStyle->StyleSVGReset()->mMask : nullptr;
   newLayers = &StyleSVGReset()->mMask;
   AddAndRemoveImageAssociations(this, oldLayers, newLayers);
 
   if (aOldComputedStyle) {
+    // Detect style changes that should trigger a scroll anchor adjustment
+    // suppression.
+    // https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers
+    bool needAnchorSuppression = false;
+
     // If we detect a change on margin, padding or border, we store the old
     // values on the frame itself between now and reflow, so if someone
     // calls GetUsed(Margin|Border|Padding)() before the next reflow, we
     // can give an accurate answer.
     // We don't want to set the property if one already exists.
     nsMargin oldValue(0, 0, 0, 0);
     nsMargin newValue(0, 0, 0, 0);
     const nsStyleMargin* oldMargin = aOldComputedStyle->PeekStyleMargin();
     if (oldMargin && oldMargin->GetMargin(oldValue)) {
-      if ((!StyleMargin()->GetMargin(newValue) || oldValue != newValue) &&
-          !HasProperty(UsedMarginProperty())) {
-        AddProperty(UsedMarginProperty(), new nsMargin(oldValue));
+      if (!StyleMargin()->GetMargin(newValue) || oldValue != newValue) {
+        if (!HasProperty(UsedMarginProperty())) {
+          AddProperty(UsedMarginProperty(), new nsMargin(oldValue));
+        }
+        needAnchorSuppression = true;
       }
     }
 
     const nsStylePadding* oldPadding = aOldComputedStyle->PeekStylePadding();
     if (oldPadding && oldPadding->GetPadding(oldValue)) {
-      if ((!StylePadding()->GetPadding(newValue) || oldValue != newValue) &&
-          !HasProperty(UsedPaddingProperty())) {
-        AddProperty(UsedPaddingProperty(), new nsMargin(oldValue));
+      if (!StylePadding()->GetPadding(newValue) || oldValue != newValue) {
+        if (!HasProperty(UsedPaddingProperty())) {
+          AddProperty(UsedPaddingProperty(), new nsMargin(oldValue));
+        }
+        needAnchorSuppression = true;
       }
     }
 
     const nsStyleBorder* oldBorder = aOldComputedStyle->PeekStyleBorder();
     if (oldBorder) {
       oldValue = oldBorder->GetComputedBorder();
       newValue = StyleBorder()->GetComputedBorder();
       if (oldValue != newValue && !HasProperty(UsedBorderProperty())) {
         AddProperty(UsedBorderProperty(), new nsMargin(oldValue));
       }
     }
+
+    if (mInScrollAnchorChain) {
+      const nsStylePosition* oldPosition =
+          aOldComputedStyle->PeekStylePosition();
+      if (oldPosition &&
+          (oldPosition->mOffset != StylePosition()->mOffset ||
+           oldPosition->mWidth != StylePosition()->mWidth ||
+           oldPosition->mMinWidth != StylePosition()->mMinWidth ||
+           oldPosition->mMaxWidth != StylePosition()->mMaxWidth ||
+           oldPosition->mHeight != StylePosition()->mHeight ||
+           oldPosition->mMinHeight != StylePosition()->mMinHeight ||
+           oldPosition->mMaxHeight != StylePosition()->mMaxHeight)) {
+        needAnchorSuppression = true;
+      }
+
+      const nsStyleDisplay* oldDisp = aOldComputedStyle->PeekStyleDisplay();
+      if (oldDisp && (oldDisp->mPosition != StyleDisplay()->mPosition ||
+                      oldDisp->TransformChanged(*StyleDisplay()))) {
+        needAnchorSuppression = true;
+      }
+    }
+
+    if (mInScrollAnchorChain && needAnchorSuppression) {
+      ScrollAnchorContainer::FindFor(this)->SuppressAdjustments();
+    }
   }
 
   ImageLoader* imageLoader = PresContext()->Document()->StyleImageLoader();
   imgIRequest* oldBorderImage =
       aOldComputedStyle
           ? aOldComputedStyle->StyleBorder()->GetBorderImageRequest()
           : nullptr;
   imgIRequest* newBorderImage = StyleBorder()->GetBorderImageRequest();
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3142,16 +3142,22 @@ void nsStyleDisplay::FinishStyle(nsPresC
                                  const nsStyleDisplay* aOldStyle) {
   MOZ_ASSERT(NS_IsMainThread());
 
   mShapeOutside.FinishStyle(aPresContext,
                             aOldStyle ? &aOldStyle->mShapeOutside : nullptr);
   GenerateCombinedIndividualTransform();
 }
 
+static inline bool TransformListChanged(
+    const RefPtr<nsCSSValueSharedList>& aList,
+    const RefPtr<nsCSSValueSharedList>& aNewList) {
+  return !aList != !aNewList || (aList && *aList != *aNewList);
+}
+
 static inline nsChangeHint CompareTransformValues(
     const RefPtr<nsCSSValueSharedList>& aList,
     const RefPtr<nsCSSValueSharedList>& aNewList) {
   nsChangeHint result = nsChangeHint(0);
 
   // Note: If we add a new change hint for transform changes here, we have to
   // modify KeyframeEffect::CalculateCumulativeChangeHint too!
   if (!aList != !aNewList || (aList && *aList != *aNewList)) {
@@ -3420,16 +3426,21 @@ nsChangeHint nsStyleDisplay::CalcDiffere
                 mScrollSnapCoordinate != aNewData.mScrollSnapCoordinate ||
                 mWillChange != aNewData.mWillChange)) {
     hint |= nsChangeHint_NeutralChange;
   }
 
   return hint;
 }
 
+bool nsStyleDisplay::TransformChanged(const nsStyleDisplay& aNewData) const {
+  return TransformListChanged(mSpecifiedTransform,
+                              aNewData.mSpecifiedTransform);
+}
+
 void nsStyleDisplay::GenerateCombinedIndividualTransform() {
   // FIXME(emilio): This should probably be called from somewhere like what we
   // do for image layers, instead of FinishStyle.
   //
   // This does and undoes the work a ton of times in Stylo.
   mIndividualTransform = nullptr;
 
   // Follow the order defined in the spec to append transform functions.
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1876,16 +1876,18 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   nsStyleDisplay(const nsStyleDisplay& aOther);
   ~nsStyleDisplay();
 
   void FinishStyle(nsPresContext*, const nsStyleDisplay*);
   const static bool kHasFinishStyle = true;
 
   nsChangeHint CalcDifference(const nsStyleDisplay& aNewData) const;
 
+  bool TransformChanged(const nsStyleDisplay& aNewData) const;
+
   // We guarantee that if mBinding is non-null, so are mBinding->GetURI() and
   // mBinding->mOriginPrincipal.
   RefPtr<mozilla::css::URLValue> mBinding;
   mozilla::StyleDisplay mDisplay;
   mozilla::StyleDisplay mOriginalDisplay;  // saved mDisplay for
                                            //         position:absolute/fixed
                                            //         and float:left/right;
                                            //         otherwise equal to