Bug 1490393 - Flush a deferred transform before picking up another if the ASR changes. r=mstange

The implementation of deferred transforms did not handle the case where we ended up deferring multiple transform items in a row with different ASRs. In this case, when we encounter the nested transform item that we want to defer, we need to flush the previously-deferred transform item into a WebRenderLayerScrollData item. This patch accomplishes that, and includes a mochitest that exercises the relevant behaviour. Depends on D8110 Differential Revision: https://phabricator.services.mozilla.com/D8111 --HG-- extra : moz-landing-system : lando
2018-11-01 21:14:50 +00:00 · 2018-11-01 21:14:50 +00:00 · b6030f993c
--- a/gfx/layers/apz/test/mochitest/helper_bug1490393-2.html
+++ b/gfx/layers/apz/test/mochitest/helper_bug1490393-2.html
@ -0,0 +1,63 @@
+<!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%; overflow-x: auto; overflow-y: hidden; transform: translateZ(0)">
+            <div id="scrollable" style="display: inline-block; height: 100%; overflow-y: auto; transform: translateZ(0)">
+                <div style="min-height: 2000px">Yay text</div>
+            </div>
+            <div style="display: inline-block; width: 2000px; height: 100%;"></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
@ -29,8 +29,9 @@ var subtests = [
  // Scrollbar dragging where we exercise the snapback behaviour by moving the
  // mouse away from the scrollbar during drag
  {'file': 'helper_scrollbar_snap_bug1501062.html'},
-  // Test for scrollbar-dragging on scrollframes inside nested transforms
-  {'file': 'helper_bug1490393.html'}
+  // Tests for scrollbar-dragging on scrollframes inside nested transforms
+  {'file': 'helper_bug1490393.html'},
+  {'file': 'helper_bug1490393-2.html'}
 ];

 if (isApzEnabled()) {
--- a/gfx/layers/wr/StackingContextHelper.h
+++ b/gfx/layers/wr/StackingContextHelper.h
@ -120,7 +120,9 @@ private:
  // 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).
+  // in the face of async scrolling). This behaviour of forcing a
+  // WebRenderLayerScrollData item to be generated when the ASR changes is
+  // implemented in WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList.
  Maybe<nsDisplayTransform*> mDeferredTransformItem;
  Maybe<gfx::Matrix4x4> mDeferredAncestorTransform;

--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@ -1423,6 +1423,20 @@ WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList* a
        forceNewLayerData = true;
      }

+      // Refer to the comment on StackingContextHelper::mDeferredTransformItem
+      // for an overview of what this is about. This bit of code applies to the
+      // case where we are deferring a transform item, and we then need to defer
+      // another transform with a different ASR. In such a case we cannot just
+      // merge the deferred transforms, but need to force a new
+      // WebRenderLayerScrollData item to flush the old deferred transform, so
+      // that we can then start deferring the new one.
+      if (!forceNewLayerData &&
+          item->GetType() == DisplayItemType::TYPE_TRANSFORM &&
+          aSc.GetDeferredTransformItem() &&
+          (*aSc.GetDeferredTransformItem())->GetActiveScrolledRoot() != asr) {
+        forceNewLayerData = true;
+      }
+
      // If we're going to create a new layer data for this item, stash the
      // ASR so that if we recurse into a sublist they will know where to stop
      // walking up their ASR chain when building scroll metadata.