Bug 1502010 - Tighten up the ArePointerEventsConsumable checks. r=botond

This patch tries to reduce the false-positive cases where
ArePointerEventsConsumable returns true even though the input events
won't actually result in panning. It does this by ascertaining the
direction of panning (if possible) in the current input block and
checking to see if panning can actually occur in that direction.
Previously it would just check if panning could occur without taking
into account the actual pan direction of the input events.

Differential Revision: https://phabricator.services.mozilla.com/D12824

--HG--
extra : moz-landing-system : lando

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -955,28 +955,47 @@ AsyncPanZoomController::GetSecondTapTolerance() const
 }
 
 bool
-AsyncPanZoomController::ArePointerEventsConsumable(TouchBlockState* aBlock, uint32_t aTouchPoints) {
-  if (aTouchPoints == 0) {
+AsyncPanZoomController::ArePointerEventsConsumable(TouchBlockState* aBlock, const MultiTouchInput& aInput) {
+  uint32_t touchPoints = aInput.mTouches.Length();
+  if (touchPoints == 0) {
     // Cant' do anything with zero touch points
     return false;
   }
 
-  // This logic is simplified, erring on the side of returning true
-  // if we're not sure. It's safer to pretend that we can consume the
-  // event and then not be able to than vice-versa.
+  // This logic is simplified, erring on the side of returning true if we're
+  // not sure. It's safer to pretend that we can consume the event and then
+  // not be able to than vice-versa. But at the same time, we should try hard
+  // to return an accurate result, because returning true can trigger a
+  // pointercancel event to web content, which can break certain features
+  // that are using touch-action and handling the pointermove events.
+  //
   // We could probably enhance this logic to determine things like "we're
   // not pannable, so we can only zoom in, and the zoom is already maxed
   // out, so we're not zoomable either" but no need for that at this point.
 
-  bool pannable = aBlock->GetOverscrollHandoffChain()->CanBePanned(this);
-  bool zoomable = mZoomConstraints.mAllowZoom;
+  bool pannableX = aBlock->TouchActionAllowsPanningX() &&
+      aBlock->GetOverscrollHandoffChain()->CanScrollInDirection(this, ScrollDirection::eHorizontal);
+  bool pannableY = aBlock->TouchActionAllowsPanningY() &&
+      aBlock->GetOverscrollHandoffChain()->CanScrollInDirection(this, ScrollDirection::eVertical);
 
-  pannable &= (aBlock->TouchActionAllowsPanningX() || aBlock->TouchActionAllowsPanningY());
+  bool pannable;
+
+  Maybe<ScrollDirection> panDirection = aBlock->GetBestGuessPanDirection(aInput);
+  if (panDirection == Some(ScrollDirection::eVertical)) {
+    pannable = pannableY;
+  } else if (panDirection == Some(ScrollDirection::eHorizontal)) {
+    pannable = pannableX;
+  } else {
+    // If we don't have a guessed pan direction, err on the side of returning true.
+    pannable = pannableX || pannableY;
+  }
+
+  bool zoomable = mZoomConstraints.mAllowZoom;
   zoomable &= (aBlock->TouchActionAllowsPinchZoom());
 
   // XXX once we fix bug 1031443, consumable should be assigned
-  // pannable || zoomable if aTouchPoints > 1.
-  bool consumable = (aTouchPoints == 1 ? pannable : zoomable);
+  // pannable || zoomable if touchPoints > 1.
+  bool consumable = (touchPoints == 1 ? pannable : zoomable);
   if (!consumable) {
     return false;
   }

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -1215,12 +1215,12 @@ public:
   void FlushRepaintForNewInputBlock();
 
   /**
-   * Given the number of touch points in an input event and touch block they
-   * belong to, check if the event can result in a panning/zooming behavior.
+   * Given an input event and the touch block it belongs to, check if the
+   * event can lead to a panning/zooming behavior.
    * This is primarily used to figure out when to dispatch the pointercancel
    * event for the pointer events spec.
    */
-  bool ArePointerEventsConsumable(TouchBlockState* aBlock, uint32_t aTouchPoints);
+  bool ArePointerEventsConsumable(TouchBlockState* aBlock, const MultiTouchInput& aInput);
 
   /**
    * Clear internal state relating to touch input handling.

gfx/layers/apz/src/InputBlockState.cpp

@@ -6,6 +6,7 @@
 
 #include "InputBlockState.h"
 
+#include "APZUtils.h"
 #include "AsyncPanZoomController.h"         // for AsyncPanZoomController
 #include "ScrollAnimationPhysics.h"         // for kScrollSeriesTimeoutMs
 #include "gfxPrefs.h"                       // for gfxPrefs
@@ -909,6 +910,29 @@ TouchBlockState::UpdateSlopState(const MultiTouchInput& aInput,
   return mInSlop;
 }
 
+Maybe<ScrollDirection>
+TouchBlockState::GetBestGuessPanDirection(const MultiTouchInput& aInput)
+{
+  if (aInput.mType != MultiTouchInput::MULTITOUCH_MOVE ||
+      aInput.mTouches.Length() != 1) {
+    return Nothing();
+  }
+  ScreenPoint vector = aInput.mTouches[0].mScreenPoint - mSlopOrigin;
+  double angle = atan2(vector.y, vector.x); // range [-pi, pi]
+  angle = fabs(angle); // range [0, pi]
+
+  double angleThreshold = TouchActionAllowsPanningXY()
+      ? gfxPrefs::APZAxisLockAngle()
+      : gfxPrefs::APZAllowedDirectPanAngle();
+  if (apz::IsCloseToHorizontal(angle, angleThreshold)) {
+    return Some(ScrollDirection::eHorizontal);
+  }
+  if (apz::IsCloseToVertical(angle, angleThreshold)) {
+    return Some(ScrollDirection::eVertical);
+  }
+  return Nothing();
+}
+
 uint32_t
 TouchBlockState::GetActiveTouchCount() const
 {

gfx/layers/apz/src/InputBlockState.h

@@ -484,6 +484,13 @@ public:
   bool UpdateSlopState(const MultiTouchInput& aInput,
                        bool aApzcCanConsumeEvents);
 
+  /**
+   * Based on the slop origin and the given input event, return a best guess
+   * as to the pan direction of this touch block. Returns Nothing() if no guess
+   * can be made.
+   */
+  Maybe<ScrollDirection> GetBestGuessPanDirection(const MultiTouchInput& aInput);
+
   /**
    * Returns the number of touch points currently active.
    */

gfx/layers/apz/src/InputQueue.cpp

@@ -151,7 +151,7 @@ InputQueue::ReceiveTouchInput(const RefPtr<AsyncPanZoomController>& aTarget,
   if (block->IsDuringFastFling()) {
     INPQ_LOG("dropping event due to block %p being in fast motion\n", block);
     result = nsEventStatus_eConsumeNoDefault;
-  } else if (target && target->ArePointerEventsConsumable(block, aEvent.mTouches.Length())) {
+  } else if (target && target->ArePointerEventsConsumable(block, aEvent)) {
     if (block->UpdateSlopState(aEvent, true)) {
       INPQ_LOG("dropping event due to block %p being in slop\n", block);
       result = nsEventStatus_eConsumeNoDefault;

gfx/layers/apz/test/gtest/TestPanning.cpp

@@ -108,7 +108,7 @@ TEST_F(APZCPanningTester, PanWithTouchActionNone) {
 TEST_F(APZCPanningTester, PanWithTouchActionPanX) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
   SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
-  DoPanTest(false, true, mozilla::layers::AllowedTouchBehavior::HORIZONTAL_PAN);
+  DoPanTest(false, false, mozilla::layers::AllowedTouchBehavior::HORIZONTAL_PAN);
 }
 
 TEST_F(APZCPanningTester, PanWithTouchActionPanY) {