Bug 1411627 - Stop sending sticky clips to WR for scrollframes with no ASR. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 07 Nov 2017 10:16:05 -0500
changeset 694246 74abebf13326c2ae5104e17f7bd34dae3054b5a6
parent 694245 4c387a97b26ed2c8eda0d013c03dddc6619b8d84
child 694247 7267fe056e173a3730ee270e172aa83beaf58de6
push id88084
push userkgupta@mozilla.com
push dateTue, 07 Nov 2017 15:18:14 +0000
reviewersmstange
bugs1411627
milestone58.0a1
Bug 1411627 - Stop sending sticky clips to WR for scrollframes with no ASR. r?mstange If the scrollframe has no ASR and is not async scrollable, WR will never need to adjust the position of the sticky item. So we don't actually need to send this data to WR. In theory this would be harmless but the later patches in this bug actually expose another coordinate space mismatch between the gecko and WR sticky-position code, so this patch is needed to avoid the latent problem from manifesting to the user. MozReview-Commit-ID: J0LMcU1FudA
layout/painting/nsDisplayList.cpp
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7313,16 +7313,33 @@ bool
 nsDisplayStickyPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                                  mozilla::wr::IpcResourceUpdateQueue& aResources,
                                                  const StackingContextHelper& aSc,
                                                  WebRenderLayerManager* aManager,
                                                  nsDisplayListBuilder* aDisplayListBuilder)
 {
   StickyScrollContainer* stickyScrollContainer = StickyScrollContainer::GetStickyScrollContainerForFrame(mFrame);
   if (stickyScrollContainer) {
+    // If there's no ASR for the scrollframe that this sticky item is attached
+    // to, then don't create a WR sticky item for it either. Trying to do so
+    // will end in sadness because WR will interpret some coordinates as
+    // relative to the nearest enclosing scrollframe, which will correspond
+    // to the nearest ancestor ASR on the gecko side. That ASR will not be the
+    // same as the scrollframe this sticky item is actually supposed to be
+    // attached to, thus the sadness.
+    // Not sending WR the sticky item is ok, because the enclosing scrollframe
+    // will never be asynchronously scrolled. Instead we will always position
+    // the sticky items correctly on the gecko side and WR will never need to
+    // adjust their position itself.
+    if (!stickyScrollContainer->ScrollFrame()->MayBeAsynchronouslyScrolled()) {
+      stickyScrollContainer = nullptr;
+    }
+  }
+
+  if (stickyScrollContainer) {
     float auPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
 
     bool snap;
     nsRect itemBounds = GetBounds(aDisplayListBuilder, &snap);
 
     Maybe<float> topMargin;
     Maybe<float> rightMargin;
     Maybe<float> bottomMargin;