Bug 1490393 - Accumulate the deferred transform down the StackingContextHelper chain if the ASR matches. r=mstange

The implementation of deferred transforms did not handle the case where we ended up deferring multiple transform items before encountering the APZ-relevant display item. In this case we need to somehow accumulate all the deferred transforms. This patch accomplishes that, and includes a mochitest that exercises the relevant behaviour. Depends on D8109 Differential Revision: https://phabricator.services.mozilla.com/D8110 --HG-- extra : moz-landing-system : lando
2018-11-01 21:14:31 +00:00 · 2018-11-01 21:14:31 +00:00 · 1523c9bf61
--- a/gfx/layers/apz/test/mochitest/helper_bug1490393.html
+++ b/gfx/layers/apz/test/mochitest/helper_bug1490393.html
@ -0,0 +1,62 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width; initial-scale=1.0">
+  <title>Dragging the mouse on a scrollbar for a scrollframe inside nested transforms</title>
+  <script type="application/javascript" src="apz_test_native_event_utils.js"></script>
+  <script type="application/javascript" src="apz_test_utils.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
+  <script type="text/javascript">
+
+function* test(testDriver) {
+  var scrollableDiv = document.getElementById('scrollable');
+  scrollableDiv.addEventListener('scroll', () => setTimeout(testDriver, 0), {once: true});
+
+  // Scroll down a small amount (10px). The bug in this case is that the
+  // scrollthumb "jumps" by an additional 40 pixels (height of the "gap" div)
+  // and the scrollframe scrolls by a corresponding amount. So after doing this
+  // drag we check the scroll position to make sure it hasn't scrolled by
+  // too much.
+  // Given the scrollable height of 2000px and scrollframe height of 400px,
+  // the scrollthumb should be approximately 80px tall, and dragging it 10px
+  // should scroll approximately 50 pixels. If the bug manifests, it will get
+  // dragged 50px and scroll approximately 250px.
+  var dragFinisher = yield* dragVerticalScrollbar(scrollableDiv, testDriver, 10, 10);
+  if (!dragFinisher) {
+    ok(true, "No scrollbar, can't do this test");
+    return;
+  }
+
+  // the events above might be stuck in APZ input queue for a bit until the
+  // layer is activated, so we wait here until the scroll event listener is
+  // triggered.
+  yield;
+
+  yield* dragFinisher();
+
+  // Flush everything just to be safe
+  yield flushApzRepaints(testDriver);
+
+  // In this case we just want to make sure the scroll position moved from 0
+  // which indicates the thumb dragging worked properly.
+  ok(scrollableDiv.scrollTop < 100, "Scrollbar drag resulted in a scroll position of " + scrollableDiv.scrollTop);
+}
+
+waitUntilApzStable()
+.then(runContinuation(test))
+.then(subtestDone);
+
+  </script>
+</head>
+<body>
+    <div id="gap" style="min-height: 40px"></div>
+    <div style="height: 400px; transform: translateZ(0)">
+        <div style="height: 100%; opacity: 0.9; will-change: opacity">
+            <div id="scrollable" style="height: 100%; overflow-y: auto; transform: translateZ(0)">
+                <div style="min-height: 2000px">Yay text</div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
--- a/gfx/layers/apz/test/mochitest/test_group_mouseevents.html
+++ b/gfx/layers/apz/test/mochitest/test_group_mouseevents.html
@ -28,7 +28,9 @@ var subtests = [
  {'file': 'helper_bug1462961.html'},
  // Scrollbar dragging where we exercise the snapback behaviour by moving the
  // mouse away from the scrollbar during drag
-  {'file': 'helper_scrollbar_snap_bug1501062.html'}
+  {'file': 'helper_scrollbar_snap_bug1501062.html'},
+  // Test for scrollbar-dragging on scrollframes inside nested transforms
+  {'file': 'helper_bug1490393.html'}
 ];

 if (isApzEnabled()) {
--- a/gfx/layers/wr/StackingContextHelper.cpp
+++ b/gfx/layers/wr/StackingContextHelper.cpp
@ -23,6 +23,7 @@ StackingContextHelper::StackingContextHelper()
 }

 StackingContextHelper::StackingContextHelper(const StackingContextHelper& aParentSC,
+                                             const ActiveScrolledRoot* aAsr,
                                             wr::DisplayListBuilder& aBuilder,
                                             const nsTArray<wr::WrFilterOp>& aFilters,
                                             const LayoutDeviceRect& aBounds,
@ -80,6 +81,24 @@ StackingContextHelper::StackingContextHelper(const StackingContextHelper& aParen

  mAffectsClipPositioning = mReferenceFrameId.isSome() ||
      (aBounds.TopLeft() != LayoutDevicePoint());
+
+  // If the parent stacking context has a deferred transform item, inherit it
+  // into this stacking context, as long as the ASR hasn't changed. Refer to
+  // the comments on StackingContextHelper::mDeferredTransformItem for an
+  // explanation of what goes in these fields.
+  if (aParentSC.mDeferredTransformItem &&
+      aAsr == (*aParentSC.mDeferredTransformItem)->GetActiveScrolledRoot()) {
+    if (mDeferredTransformItem) {
+      // If we are deferring another transform, put the combined transform from
+      // all the ancestor deferred items into mDeferredAncestorTransform
+      mDeferredAncestorTransform = aParentSC.GetDeferredTransformMatrix();
+    } else {
+      // We are not deferring another transform, so we can just inherit the
+      // parent stacking context's deferred data without any modification.
+      mDeferredTransformItem = aParentSC.mDeferredTransformItem;
+      mDeferredAncestorTransform = aParentSC.mDeferredAncestorTransform;
+    }
+  }
 }

 StackingContextHelper::~StackingContextHelper()
@ -99,7 +118,16 @@ Maybe<gfx::Matrix4x4>
 StackingContextHelper::GetDeferredTransformMatrix() const
 {
  if (mDeferredTransformItem) {
-    return Some((*mDeferredTransformItem)->GetTransform().GetMatrix());
+    // See the comments on StackingContextHelper::mDeferredTransformItem for
+    // an explanation of what's stored in mDeferredTransformItem and
+    // mDeferredAncestorTransform. Here we need to return the combined transform
+    // transform from all the deferred ancestors, including
+    // mDeferredTransformItem.
+    gfx::Matrix4x4 result = (*mDeferredTransformItem)->GetTransform().GetMatrix();
+    if (mDeferredAncestorTransform) {
+      result = *mDeferredAncestorTransform * result;
+    }
+    return Some(result);
  } else {
    return Nothing();
  }
--- a/gfx/layers/wr/StackingContextHelper.h
+++ b/gfx/layers/wr/StackingContextHelper.h
@ -16,6 +16,9 @@
 class nsDisplayTransform;

 namespace mozilla {
+
+struct ActiveScrolledRoot;
+
 namespace layers {

 /**
@ -26,6 +29,7 @@ class MOZ_RAII StackingContextHelper
 {
 public:
  StackingContextHelper(const StackingContextHelper& aParentSC,
+                        const ActiveScrolledRoot* aAsr,
                        wr::DisplayListBuilder& aBuilder,
                        const nsTArray<wr::WrFilterOp>& aFilters = nsTArray<wr::WrFilterOp>(),
                        const LayoutDeviceRect& aBounds = LayoutDeviceRect(),
@ -98,7 +102,27 @@ private:
  // new ASR or otherwise was "relevant to APZ", we would then pluck the
  // transform matrix off the deferred item and put it on the
  // WebRenderLayerScrollData instance created for that APZ-relevant descendant.
+  //
+  // One complication with this is if there are multiple nsDisplayTransform
+  // items in the ancestor chain for the APZ-relevant item. As we traverse the
+  // display list, we will defer the outermost nsDisplayTransform item, and when
+  // we encounter the next one we will need to merge it with the already-
+  // deferred one somehow. What we do in this case is have mDeferredTransformItem
+  // always point to the "innermost" deferred transform item (i.e. the closest
+  // ancestor nsDisplayTransform item of the item that created this
+  // StackingContextHelper). And then we use mDeferredAncestorTransform to store
+  // the product of all the other transforms that were deferred. As a result,
+  // there is an invariant here that if mDeferredTransformItem is Nothing(),
+  // mDeferredAncestorTransform will also be Nothing(). Note that we
+  // can only do this if the nsDisplayTransform items share the same ASR. If
+  // we are processing an nsDisplayTransform item with a different ASR than the
+  // previously-deferred item, we assume that the previously-deferred transform
+  // will get sent to APZ as part of a separate WebRenderLayerScrollData item,
+  // and so we don't need to bother with any merging. (The merging probably
+  // wouldn't even make sense because the coordinate spaces might be different
+  // in the face of async scrolling).
  Maybe<nsDisplayTransform*> mDeferredTransformItem;
+  Maybe<gfx::Matrix4x4> mDeferredAncestorTransform;

  bool mIsPreserve3D;
  bool mRasterizeLocally;
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@ -1313,7 +1313,7 @@ WebRenderCommandBuilder::BuildWebRenderCommands(wr::DisplayListBuilder& aBuilder
  mClipManager.BeginBuild(mManager, aBuilder);

  {
-    StackingContextHelper pageRootSc(sc, aBuilder, aFilters);
+    StackingContextHelper pageRootSc(sc, nullptr, aBuilder, aFilters);
    if (ShouldDumpDisplayList(aDisplayListBuilder)) {
      mBuilderDumpIndex = aBuilder.Dump(mDumpIndent + 1, Some(mBuilderDumpIndex), Nothing());
    }
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@ -6750,6 +6750,7 @@ nsDisplayOpacity::CreateWebRenderCommands(

  nsTArray<mozilla::wr::WrFilterOp> filters;
  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           filters,
                           LayoutDeviceRect(),
@ -6803,6 +6804,7 @@ nsDisplayBlendMode::CreateWebRenderCommands(
 {
  nsTArray<mozilla::wr::WrFilterOp> filters;
  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           filters,
                           LayoutDeviceRect(),
@ -6954,7 +6956,7 @@ nsDisplayBlendContainer::CreateWebRenderCommands(
  mozilla::layers::WebRenderLayerManager* aManager,
  nsDisplayListBuilder* aDisplayListBuilder)
 {
-  StackingContextHelper sc(aSc, aBuilder);
+  StackingContextHelper sc(aSc, GetActiveScrolledRoot(), aBuilder);

  return nsDisplayWrapList::CreateWebRenderCommands(
    aBuilder, aResources, sc, aManager, aDisplayListBuilder);
@ -7094,6 +7096,7 @@ nsDisplayOwnLayer::CreateWebRenderCommands(
  prop.effect_type = wr::WrAnimationType::Transform;

  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           nsTArray<wr::WrFilterOp>(),
                           LayoutDeviceRect(),
@ -7862,7 +7865,7 @@ nsDisplayStickyPosition::CreateWebRenderCommands(
  }

  {
-    StackingContextHelper sc(aSc, aBuilder);
+    StackingContextHelper sc(aSc, GetActiveScrolledRoot(), aBuilder);
    nsDisplayWrapList::CreateWebRenderCommands(
      aBuilder, aResources, sc, aManager, aDisplayListBuilder);
  }
@ -8849,6 +8852,7 @@ nsDisplayTransform::CreateWebRenderCommands(
    ActiveLayerTracker::IsStyleMaybeAnimated(Frame(), eCSSProperty_transform);

  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           filters,
                           LayoutDeviceRect(position, LayoutDeviceSize()),
@ -9498,6 +9502,7 @@ nsDisplayPerspective::CreateWebRenderCommands(

  nsTArray<mozilla::wr::WrFilterOp> filters;
  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           filters,
                           LayoutDeviceRect(),
@ -10184,6 +10189,7 @@ nsDisplayMasksAndClipPaths::CreateWebRenderCommands(
      : Nothing();

    layer.emplace(aSc,
+                  GetActiveScrolledRoot(),
                  aBuilder,
                  /*aFilters: */ nsTArray<wr::WrFilterOp>(),
                  /*aBounds: */ bounds,
@ -10498,6 +10504,7 @@ nsDisplayFilters::CreateWebRenderCommands(

  float opacity = mFrame->StyleEffects()->mOpacity;
  StackingContextHelper sc(aSc,
+                           GetActiveScrolledRoot(),
                           aBuilder,
                           wrFilters,
                           LayoutDeviceRect(),