Bug 1305957 part 9 - Suppress scroll offset adjustment when node moves into flow or out of flow inside of a scrollable frame. r=emilio
authorRyan Hunt <rhunt@eqrion.net>
Tue, 27 Nov 2018 15:39:53 -0600
changeset 510493 3f00aed52fb34af7937d7c4a62265b2ea886b4b5
parent 510492 eef9ed24a0613d3b80ae09074484d4678deb2a2e
child 510494 4b492273f76348a6bb29e034493075282f9a80be
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)
reviewersemilio
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 9 - Suppress scroll offset adjustment when node moves into flow or out of flow inside of a scrollable frame. r=emilio This commit implements the second half of the heuristics to detect style changes that could lead to feedback loops with scroll anchoring. [1] A new change hint is added for when a style is changed from positioned to not positioned. When this hint is applied, scroll anchor suppression is triggered in the scroll anchor container where the frame used to be, and the new scroll anchor container where the frame is added after reconstruction. [1] https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers Differential Revision: https://phabricator.services.mozilla.com/D13273
layout/base/RestyleManager.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -9,16 +9,17 @@
 #include "mozilla/AutoRestyleTimelineMarker.h"
 #include "mozilla/AutoTimelineMarker.h"
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/ComputedStyleInlines.h"
 #include "mozilla/DocumentStyleRootIterator.h"
 #include "mozilla/GeckoBindings.h"
 #include "mozilla/LayerAnimationInfo.h"
 #include "mozilla/layers/AnimationInfo.h"
+#include "mozilla/layout/ScrollAnchorContainer.h"
 #include "mozilla/ServoBindings.h"
 #include "mozilla/ServoStyleSetInlines.h"
 #include "mozilla/Unused.h"
 #include "mozilla/ViewportFrame.h"
 #include "mozilla/dom/ChildIterator.h"
 #include "mozilla/dom/DocumentInlines.h"
 #include "mozilla/dom/ElementInlines.h"
 #include "mozilla/dom/HTMLBodyElement.h"
@@ -1493,27 +1494,72 @@ void RestyleManager::ProcessRestyledFram
         }
         // Don't remove NS_FRAME_MAY_BE_TRANSFORMED since it may still be
         // transformed by other means. It's OK to have the bit even if it's
         // not needed.
       }
     }
 
     if (hint & nsChangeHint_ReconstructFrame) {
+      // Record whether this frame was absolutely positioned before and after
+      // frame construction, to detect changes for scroll anchor adjustment
+      // suppression.
+      bool wasAbsPosStyle = false;
+      ScrollAnchorContainer* previousAnchorContainer = nullptr;
+      AutoWeakFrame previousAnchorContainerFrame;
+      if (frame) {
+        wasAbsPosStyle = frame->StyleDisplay()->IsAbsolutelyPositionedStyle();
+        previousAnchorContainer = ScrollAnchorContainer::FindFor(frame);
+
+        // It's possible for the scroll anchor container to be destroyed by
+        // frame construction, so use a weak frame to detect this.
+        if (previousAnchorContainer) {
+          previousAnchorContainerFrame = previousAnchorContainer->Frame();
+        }
+      }
+
       // If we ever start passing true here, be careful of restyles
       // that involve a reframe and animations.  In particular, if the
       // restyle we're processing here is an animation restyle, but
       // the style resolution we will do for the frame construction
       // happens async when we're not in an animation restyle already,
       // problems could arise.
       // We could also have problems with triggering of CSS transitions
       // on elements whose frames are reconstructed, since we depend on
       // the reconstruction happening synchronously.
       frameConstructor->RecreateFramesForContent(
           content, nsCSSFrameConstructor::InsertionKind::Sync);
+      frame = content->GetPrimaryFrame();
+
+      // See the check above for absolutely positioned style.
+      bool isAbsPosStyle = false;
+      ScrollAnchorContainer* newAnchorContainer = nullptr;
+      if (frame) {
+        isAbsPosStyle = frame->StyleDisplay()->IsAbsolutelyPositionedStyle();
+        newAnchorContainer = ScrollAnchorContainer::FindFor(frame);
+      }
+
+      // If this frame construction was due to a change in absolute
+      // positioning, then suppress scroll anchor adjustments in the scroll
+      // anchor container the frame was in, and the one it moved into.
+      //
+      // This isn't entirely accurate to the specification, which requires us
+      // to do this for all frames that change being absolutely positioned. It's
+      // possible for multiple style changes to cause frame reconstruction and
+      // coalesce, which could cause a suppression trigger to be missed. It's
+      // unclear whether this will be an issue as suppression triggers are just
+      // heuristics.
+      if (wasAbsPosStyle != isAbsPosStyle) {
+        if (previousAnchorContainerFrame) {
+          previousAnchorContainer->SuppressAdjustments();
+        }
+        if (newAnchorContainer) {
+          newAnchorContainer->SuppressAdjustments();
+        }
+      }
     } else {
       NS_ASSERTION(frame, "This shouldn't happen");
 
       if (!frame->FrameMaintainsOverflow()) {
         // frame does not maintain overflow rects, so avoid calling
         // FinishAndStoreOverflow on it:
         hint &=
             ~(nsChangeHint_UpdateOverflow | nsChangeHint_ChildrenOnlyTransform |