Bug 1448513 - Only do the panThreshold check if the threshold is > 0. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 22 Jun 2018 16:29:47 -0400
changeset 809731 ad8b4fbd3b6c245c9d11c47e3c347552398b7d70
parent 809559 6b6f3f6ecf142908b3e437d3bc3fac75540a9bcb
push id113786
push userkgupta@mozilla.com
push dateFri, 22 Jun 2018 20:30:19 +0000
reviewersbotond
bugs1448513
milestone62.0a1
Bug 1448513 - Only do the panThreshold check if the threshold is > 0. r?botond This optimization also sneakily avoids the call to UpdateWithTouchAtDevicePoint which is what updates the Axis::mPos variable to the new touch point. Without that update, mPos remains at the touchstart point, which ensures that the full pixel delta from all the touchmove events gets turned into scrolling. Without this, the first touchmove event always gets consumed in the process of overcoming the pan threshold, which lead to flaky test behaviour. MozReview-Commit-ID: 9cD0l0bfHJh
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/test/mochitest/apz_test_native_event_utils.js
gfx/layers/apz/test/mochitest/helper_basic_pan.html
gfx/layers/apz/test/mochitest/helper_bug1280013.html
gfx/layers/apz/test/mochitest/helper_div_pan.html
gfx/layers/apz/test/mochitest/helper_iframe_pan.html
gfx/layers/apz/test/mochitest/helper_touch_action.html
gfx/layers/apz/test/mochitest/helper_touch_action_complex.html
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1279,20 +1279,28 @@ nsEventStatus AsyncPanZoomController::On
     case NOTHING:
     case ANIMATING_ZOOM:
       // May happen if the user double-taps and drags without lifting after the
       // second tap. Ignore the move if this happens.
       return nsEventStatus_eIgnore;
 
     case TOUCHING: {
       ScreenCoord panThreshold = GetTouchStartTolerance();
-      UpdateWithTouchAtDevicePoint(aEvent);
-
-      if (PanDistance() < panThreshold) {
-        return nsEventStatus_eIgnore;
+
+      // We intentionally skip the UpdateWithTouchAtDevicePoint call when the
+      // panThreshold is zero. This ensures more deterministic behaviour during
+      // testing. If we call that, Axis::mPos gets updated to the point of this
+      // touchmove event, but we "consume" the move to overcome the
+      // panThreshold, so it's hard to pan a specific amount reliably from a
+      // mochitest.
+      if (panThreshold > 0.0f) {
+        UpdateWithTouchAtDevicePoint(aEvent);
+        if (PanDistance() < panThreshold) {
+          return nsEventStatus_eIgnore;
+        }
       }
 
       ParentLayerPoint touchPoint = GetFirstTouchPoint(aEvent);
 
       MOZ_ASSERT(GetCurrentTouchBlock());
       if (gfxPrefs::TouchActionEnabled() && GetCurrentTouchBlock()->TouchActionAllowsPanningXY()) {
         // User tries to trigger a touch behavior. If allowed touch behavior is vertical pan
         // + horizontal pan (touch-action value is equal to AUTO) we can return ConsumeNoDefault
--- a/gfx/layers/apz/test/mochitest/apz_test_native_event_utils.js
+++ b/gfx/layers/apz/test/mochitest/apz_test_native_event_utils.js
@@ -275,26 +275,20 @@ function* synthesizeNativeTouchSequences
         synthesizeNativeTouch(aElement, aPositions[i][j].x, aPositions[i][j].y, SpecialPowers.DOMWindowUtils.TOUCH_CONTACT, null, aTouchIds[j]);
         currentPositions[j] = aPositions[i][j];
       }
     }
   }
   return true;
 }
 
-// A handy constant when synthesizing native touch drag events with the pref
-// "apz.touch_start_tolerance" set to 0. In this case, the first touchmove with
-// a nonzero pixel movement is consumed by the APZ to transition from the
-// "touching" state to the "panning" state, so calls to synthesizeNativeTouchDrag
-// should add an extra pixel pixel for this purpose. The TOUCH_SLOP provides
-// a constant that can be used for this purpose. Note that if the touch start
-// tolerance is set to something higher, the touch slop amount used must be
-// correspondingly increased so as to be higher than the tolerance.
-const TOUCH_SLOP = 1;
-function synthesizeNativeTouchDrag(aElement, aX, aY, aDeltaX, aDeltaY, aObserver = null, aTouchId = 0) {
+// Note that when calling this function you'll want to make sure that the pref
+// "apz.touch_start_tolerance" is set to 0, or some of the touchmove will get
+// consumed to overcome the panning threshold.
+function synthesizeNativeTouchDrag(aElement, aX, aY, aDeltaX, aDeltaY, aSlop, aObserver = null, aTouchId = 0) {
   var steps = Math.max(Math.abs(aDeltaX), Math.abs(aDeltaY));
   var positions = new Array();
   positions.push([{ x: aX, y: aY }]);
   for (var i = 1; i < steps; i++) {
     var dx = i * (aDeltaX / steps);
     var dy = i * (aDeltaY / steps);
     var pos = { x: aX + dx, y: aY + dy };
     positions.push([pos]);
--- a/gfx/layers/apz/test/mochitest/helper_basic_pan.html
+++ b/gfx/layers/apz/test/mochitest/helper_basic_pan.html
@@ -12,17 +12,17 @@
 function scrollPage() {
   var transformEnd = function() {
     SpecialPowers.Services.obs.removeObserver(transformEnd, "APZ:TransformEnd", false);
     dump("Transform complete; flushing repaints...\n");
     flushApzRepaints(checkScroll);
   };
   SpecialPowers.Services.obs.addObserver(transformEnd, "APZ:TransformEnd");
 
-  synthesizeNativeTouchDrag(document.body, 10, 100, 0, -(50 + TOUCH_SLOP));
+  synthesizeNativeTouchDrag(document.body, 10, 100, 0, -50);
   dump("Finished native drag, waiting for transform-end observer...\n");
 }
 
 function checkScroll() {
   is(window.scrollY, 50, "check that the window scrolled");
   subtestDone();
 }
 
--- a/gfx/layers/apz/test/mochitest/helper_bug1280013.html
+++ b/gfx/layers/apz/test/mochitest/helper_bug1280013.html
@@ -14,34 +14,34 @@ function* test(testDriver) {
   ok(screen.height > 500, "Screen height must be at least 500 pixels for this test to work");
 
   // This listener will trigger the test to continue once APZ is done with
   // processing the scroll.
   SpecialPowers.Services.obs.addObserver(testDriver, "APZ:TransformEnd");
 
   // Scroll down to the iframe. Do it in two drags instead of one in case the
   // device screen is short
-  yield synthesizeNativeTouchDrag(document.body, 10, 400, 0, -(350 + TOUCH_SLOP));
-  yield synthesizeNativeTouchDrag(document.body, 10, 400, 0, -(350 + TOUCH_SLOP));
+  yield synthesizeNativeTouchDrag(document.body, 10, 400, 0, -350);
+  yield synthesizeNativeTouchDrag(document.body, 10, 400, 0, -350);
   // Now the top of the visible area should be at y=700 of the top-level page,
   // so if the screen is >= 500px tall, the entire iframe should be visible, at
   // least vertically.
 
   // However, because of the overflow:hidden on the root elements, all this
   // scrolling is happening in APZ and is not reflected in the main-thread
   // scroll position (it is stored in the callback transform instead). We check
   // this by checking the scroll offset.
   yield flushApzRepaints(testDriver);
   is(window.scrollY, 0, "Main-thread scroll position is still at 0");
 
   // Scroll the iframe by 300px. Note that since the main-thread scroll position
   // is still 0, the subframe's getBoundingClientRect is going to be off by
   // 700 pixels, so we compensate for that here.
   var subframe = document.getElementById('subframe');
-  yield synthesizeNativeTouchDrag(subframe, 10, 200 - 700, 0, -(300 + TOUCH_SLOP));
+  yield synthesizeNativeTouchDrag(subframe, 10, 200 - 700, 0, -300);
 
   // Remove the observer, we don't need it any more.
   SpecialPowers.Services.obs.removeObserver(testDriver, "APZ:TransformEnd", false);
 
   // Flush any pending paints
   yield flushApzRepaints(testDriver);
 
   // get the displayport for the subframe
--- a/gfx/layers/apz/test/mochitest/helper_div_pan.html
+++ b/gfx/layers/apz/test/mochitest/helper_div_pan.html
@@ -12,17 +12,17 @@
 function scrollOuter() {
   var transformEnd = function() {
     SpecialPowers.Services.obs.removeObserver(transformEnd, "APZ:TransformEnd", false);
     dump("Transform complete; flushing repaints...\n");
     flushApzRepaints(checkScroll);
   };
   SpecialPowers.Services.obs.addObserver(transformEnd, "APZ:TransformEnd");
 
-  synthesizeNativeTouchDrag(document.getElementById('outer'), 10, 100, 0, -(50 + TOUCH_SLOP));
+  synthesizeNativeTouchDrag(document.getElementById('outer'), 10, 100, 0, -50);
   dump("Finished native drag, waiting for transform-end observer...\n");
 }
 
 function checkScroll() {
   var outerScroll = document.getElementById('outer').scrollTop;
   is(outerScroll, 50, "check that the div scrolled");
   subtestDone();
 }
--- a/gfx/layers/apz/test/mochitest/helper_iframe_pan.html
+++ b/gfx/layers/apz/test/mochitest/helper_iframe_pan.html
@@ -13,17 +13,17 @@ function scrollOuter() {
   var outer = document.getElementById('outer');
   var transformEnd = function() {
     SpecialPowers.Services.obs.removeObserver(transformEnd, "APZ:TransformEnd", false);
     dump("Transform complete; flushing repaints...\n");
     flushApzRepaints(checkScroll, outer.contentWindow);
   };
   SpecialPowers.Services.obs.addObserver(transformEnd, "APZ:TransformEnd");
 
-  synthesizeNativeTouchDrag(outer.contentDocument.body, 10, 100, 0, -(50 + TOUCH_SLOP));
+  synthesizeNativeTouchDrag(outer.contentDocument.body, 10, 100, 0, -50);
   dump("Finished native drag, waiting for transform-end observer...\n");
 }
 
 function checkScroll() {
   var outerScroll = document.getElementById('outer').contentWindow.scrollY;
   if (getPlatform() == "windows") {
     // On windows, because we run this test with native event synthesization,
     // Windows can end up eating the first touchmove which can result in the
--- a/gfx/layers/apz/test/mochitest/helper_touch_action.html
+++ b/gfx/layers/apz/test/mochitest/helper_touch_action.html
@@ -15,84 +15,84 @@ function checkScroll(x, y, desc) {
 }
 
 function* test(testDriver) {
   var target = document.getElementById('target');
 
   document.body.addEventListener('touchend', testDriver, { passive: true });
 
   // drag the page up to scroll down by 50px
-  yield ok(synthesizeNativeTouchDrag(target, 10, 100, 0, -(50 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 10, 100, 0, -50),
       "Synthesized native vertical drag (1), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(0, 50, "After first vertical drag, with pan-y" );
 
   // switch style to pan-x
   document.body.style.touchAction = 'pan-x';
   ok(true, "Waiting for pan-x to propagate...");
   yield waitForAllPaintsFlushed(function() {
     flushApzRepaints(testDriver);
   });
 
   // drag the page up to scroll down by 50px, but it won't happen because pan-x
-  yield ok(synthesizeNativeTouchDrag(target, 10, 100, 0, -(50 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 10, 100, 0, -50),
      "Synthesized native vertical drag (2), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(0, 50, "After second vertical drag, with pan-x");
 
   // drag the page left to scroll right by 50px
-  yield ok(synthesizeNativeTouchDrag(target, 100, 10, -(50 + TOUCH_SLOP), 0),
+  yield ok(synthesizeNativeTouchDrag(target, 100, 10, -50, 0),
      "Synthesized horizontal drag (1), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(50, 50, "After first horizontal drag, with pan-x");
 
   // drag the page diagonally right/down to scroll up/left by 40px each axis;
   // only the x-axis will actually scroll because pan-x
-  yield ok(synthesizeNativeTouchDrag(target, 10, 10, (40 + TOUCH_SLOP), (40 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 10, 10, 40, 40),
      "Synthesized diagonal drag (1), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(10, 50, "After first diagonal drag, with pan-x");
 
   // switch style back to pan-y
   document.body.style.touchAction = 'pan-y';
   ok(true, "Waiting for pan-y to propagate...");
   yield waitForAllPaintsFlushed(function() {
     flushApzRepaints(testDriver);
   });
 
   // drag the page diagonally right/down to scroll up/left by 40px each axis;
   // only the y-axis will actually scroll because pan-y
-  yield ok(synthesizeNativeTouchDrag(target, 10, 10, (40 + TOUCH_SLOP), (40 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 10, 10, 40, 40),
      "Synthesized diagonal drag (2), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(10, 10, "After second diagonal drag, with pan-y");
 
   // switch style to none
   document.body.style.touchAction = 'none';
   ok(true, "Waiting for none to propagate...");
   yield waitForAllPaintsFlushed(function() {
     flushApzRepaints(testDriver);
   });
 
   // drag the page diagonally up/left to scroll down/right by 40px each axis;
   // neither will scroll because of touch-action
-  yield ok(synthesizeNativeTouchDrag(target, 100, 100, -(40 + TOUCH_SLOP), -(40 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 100, 100, -40, -40),
       "Synthesized diagonal drag (3), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(10, 10, "After third diagonal drag, with none");
 
   document.body.style.touchAction = 'manipulation';
   ok(true, "Waiting for manipulation to propagate...");
   yield waitForAllPaintsFlushed(function() {
     flushApzRepaints(testDriver);
   });
 
   // drag the page diagonally up/left to scroll down/right by 40px each axis;
   // both will scroll because of touch-action
-  yield ok(synthesizeNativeTouchDrag(target, 100, 100, -(40 + TOUCH_SLOP), -(40 + TOUCH_SLOP)),
+  yield ok(synthesizeNativeTouchDrag(target, 100, 100, -40, -40),
       "Synthesized diagonal drag (4), waiting for touch-end event...");
   yield flushApzRepaints(testDriver);
   checkScroll(50, 50, "After fourth diagonal drag, with manipulation");
 }
 
 waitUntilApzStable()
 .then(runContinuation(test))
 .then(subtestDone);
--- a/gfx/layers/apz/test/mochitest/helper_touch_action_complex.html
+++ b/gfx/layers/apz/test/mochitest/helper_touch_action_complex.html
@@ -52,23 +52,19 @@ function* test(testDriver) {
 
   // Helper function for the tests below.
   // Touch-pan configuration |configuration| towards scroll offset (dx, dy) with
   // the pan touching down at (x, y). Check that the final scroll offset is
   // (ex, ey). |desc| is some description string.
   function* scrollAndCheck(configuration, x, y, dx, dy, ex, ey, desc) {
     // Start with a clean slate
     yield resetConfiguration(configuration, testDriver);
-    // Figure out the panning deltas
-    if (dx != 0) {
-      dx = -(dx + TOUCH_SLOP);
-    }
-    if (dy != 0) {
-      dy = -(dy + TOUCH_SLOP);
-    }
+    // Reverse the touch delta in order to scroll in the desired direction
+    dx = -dx;
+    dy = -dy;
     // Do the pan
     yield ok(synthesizeNativeTouchDrag(scrollframe, x, y, dx, dy),
         "Synthesized drag of (" + dx + ", " + dy + ") on configuration " + configuration);
     yield waitForAllPaints(function() {
       flushApzRepaints(testDriver);
     });
     // Check for expected scroll position
     checkScroll(scrollframe, ex, ey, 'configuration ' + configuration + ' ' + desc);