Bug 1719855 - Add two flags to differentiate a state that it's ready …

…to be handled in APZ but not yet ready to inform the handled result to GeckoView. r=geckoview-reviewers,botond,calu

Depends on D176481

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

gfx/layers/apz/src/InputBlockState.cpp

@@ -640,6 +640,8 @@ TouchBlockState::TouchBlockState(
       mInSlop(false),
       mForLongTap(false),
       mLongTapWasProcessed(false),
+      mIsWaitingLongTapResult(false),
+      mNeedsWaitTouchMove(false),
       mTouchCounter(aCounter),
       mStartTime(GetTargetApzc()->GetFrameTime().Time()) {
   mOriginalTargetConfirmedState = mTargetConfirmed;
@@ -684,6 +686,10 @@ bool TouchBlockState::IsReadyForHandling() const {
     return false;
   }
 
+  if (mIsWaitingLongTapResult) {
+    return false;
+  }
+
   return mAllowedTouchBehaviorSet || IsContentResponseTimerExpired();
 }
 

gfx/layers/apz/src/InputBlockState.h

@@ -500,8 +500,20 @@ class TouchBlockState : public CancelableBlockState {
   void SetLongTapProcessed() {
     MOZ_ASSERT(!mForLongTap);
     mLongTapWasProcessed = true;
+    mIsWaitingLongTapResult = false;
   }
 
+  void SetWaitingLongTapResult() {
+    MOZ_ASSERT(!mForLongTap);
+    mIsWaitingLongTapResult = true;
+  }
+  bool IsWaitingLongTapResult() const { return mIsWaitingLongTapResult; }
+
+  void SetNeedsToWaitTouchMove(bool aNeedsWaitTouchMove) {
+    mNeedsWaitTouchMove = aNeedsWaitTouchMove;
+  }
+  bool IsReadyForCallback() const { return !mNeedsWaitTouchMove; };
+
   /**
    * 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
@@ -534,6 +546,17 @@ class TouchBlockState : public CancelableBlockState {
   // on the first touch block after the long tap was processed.
   bool mForLongTap;
   bool mLongTapWasProcessed;
+
+  // A flag representing a state while we are waiting for a content response for
+  // the long tap.
+  // The reason why we have this flag separately from `mLongTapWasProcessed` is
+  // the block is not ready to be processed during the wait, and is ready once
+  // after `mLongTapWasProcessed` became true.
+  bool mIsWaitingLongTapResult;
+  // A flag representing a state that this block still needs to wait for a
+  // content response for a touch move event. It will be set just before
+  // triggering a long-press event.
+  bool mNeedsWaitTouchMove;
   ScreenIntPoint mSlopOrigin;
   // A reference to the InputQueue's touch counter
   TouchCounter& mTouchCounter;

gfx/layers/apz/src/InputQueue.cpp

@@ -205,7 +205,7 @@ APZEventResult InputQueue::ReceiveTouchInput(
       // If all following conditions are met, we need to wait for a content
       // response (again);
       //  1) this is the first event bailing out from in-slop state after a
-      //     long-tap event was processed
+      //     long-tap event has been fired
       //  2) there's any APZ-aware event listeners
       //  3) the event block hasn't yet been prevented
       //
@@ -216,7 +216,8 @@ APZEventResult InputQueue::ReceiveTouchInput(
       //  until a long-tap event happens, then if the user started moving their
       // finger, we have to wait for a content response twice, one is for
       // `touchstart` and one is for `touchmove`.
-      if (wasInSlop && block->WasLongTapProcessed() &&
+      if (wasInSlop &&
+          (block->WasLongTapProcessed() || block->IsWaitingLongTapResult()) &&
           !block->IsTargetOriginallyConfirmed() && !block->ShouldDropEvents()) {
         INPQ_LOG(
             "bailing out from in-stop state in block %p after a long-tap "
@@ -225,6 +226,7 @@ APZEventResult InputQueue::ReceiveTouchInput(
         block->ResetContentResponseTimerExpired();
         ScheduleMainThreadTimeout(aTarget, block);
       }
+      block->SetNeedsToWaitTouchMove(false);
       result.SetStatusForTouchEvent(*block, aFlags, consumableFlags, target);
     }
   }
@@ -656,6 +658,14 @@ TouchBlockState* InputQueue::StartNewTouchBlockForLongTap(
   newBlock->CopyPropertiesFrom(*currentBlock);
   newBlock->SetForLongTap();
 
+  // Tell the original touch block that we are going to fire a long tap event.
+  // NOTE: If we get a new touch-move event while we are waiting for a response
+  // of the long-tap event, we need to wait it before processing the original
+  // touch block because if the long-tap event response prevents us from
+  // scrolling we must stop processing any subsequent touch-move events in the
+  // same block.
+  currentBlock->SetWaitingLongTapResult();
+
   // We need to keep the current block alive, it will be used once after this
   // new touch block for long-tap was processed.
   mPrevActiveTouchBlock = currentBlock;
@@ -842,6 +852,13 @@ void InputQueue::MainThreadTimeout(uint64_t aInputBlockId) {
     NS_WARNING("input block is not a cancelable block");
   }
   if (success) {
+    if (inputBlock->AsTouchBlock() && inputBlock->AsTouchBlock()->IsInSlop()) {
+      // If the touch block is still in slop, it's still possible this block
+      // needs to send a touchmove to content after the long-press gesture
+      // since preventDefault() in a touchmove event handler should stop
+      // handling the block at all.
+      inputBlock->AsTouchBlock()->SetNeedsToWaitTouchMove(true);
+    }
     ProcessQueue();
   }
 }
@@ -884,7 +901,14 @@ void InputQueue::ContentReceivedInputBlock(uint64_t aInputBlockId,
     INPQ_LOG("couldn't find block=%" PRIu64 "\n", aInputBlockId);
   }
   if (success) {
-    ProcessQueue();
+    if (ProcessQueue()) {
+      // If we've switched the active touch block back to the original touch
+      // block from the block for long-tap, run ProcessQueue again.
+      // If we haven't yet received new touch-move events which need to be
+      // processed (e.g. we are waiting for a content response for a touch-move
+      // event), below ProcessQueue call is mostly no-op.
+      ProcessQueue();
+    }
   }
 }
 
@@ -1021,7 +1045,7 @@ static APZHandledResult GetHandledResultFor(
   return APZHandledResult{APZHandledPlace::HandledByRoot, rootApzc};
 }
 
-void InputQueue::ProcessQueue() {
+bool InputQueue::ProcessQueue() {
   APZThreadUtils::AssertOnControllerThread();
 
   while (!mQueuedInputs.IsEmpty()) {
@@ -1040,13 +1064,23 @@ void InputQueue::ProcessQueue() {
 
     // If there is an input block callback registered for this
     // input block, invoke it.
-    auto it = mInputBlockCallbacks.find(curBlock->GetBlockId());
-    if (it != mInputBlockCallbacks.end()) {
-      APZHandledResult handledResult =
-          GetHandledResultFor(target, *curBlock, it->second.mEagerStatus);
-      it->second.mCallback(curBlock->GetBlockId(), handledResult);
-      // The callback is one-shot; discard it after calling it.
-      mInputBlockCallbacks.erase(it);
+    //
+    // NOTE: In the case where the block is a touch block and the block is not
+    // ready to invoke the callback because of waiting a touch move response
+    // from content, we skip the block.
+    if (!curBlock->AsTouchBlock() ||
+        curBlock->AsTouchBlock()->IsReadyForCallback()) {
+      auto it = mInputBlockCallbacks.find(curBlock->GetBlockId());
+      if (it != mInputBlockCallbacks.end()) {
+        INPQ_LOG("invoking the callback for input from block %p id %" PRIu64
+                 "\n",
+                 curBlock, curBlock->GetBlockId());
+        APZHandledResult handledResult =
+            GetHandledResultFor(target, *curBlock, it->second.mEagerStatus);
+        it->second.mCallback(curBlock->GetBlockId(), handledResult);
+        // The callback is one-shot; discard it after calling it.
+        mInputBlockCallbacks.erase(it);
+      }
     }
 
     // target may be null here if the initial target was unconfirmed and then
@@ -1073,6 +1107,7 @@ void InputQueue::ProcessQueue() {
     mQueuedInputs.RemoveElementAt(0);
   }
 
+  bool processQueueAgain = false;
   if (CanDiscardBlock(mActiveTouchBlock)) {
     const bool forLongTap = mActiveTouchBlock->ForLongTap();
     INPQ_LOG("discarding a touch block %p id %" PRIu64 "\n",
@@ -1087,6 +1122,7 @@ void InputQueue::ProcessQueue() {
       mPrevActiveTouchBlock->SetLongTapProcessed();
       mActiveTouchBlock = mPrevActiveTouchBlock;
       mPrevActiveTouchBlock = nullptr;
+      processQueueAgain = true;
     }
   }
   if (CanDiscardBlock(mActiveWheelBlock)) {
@@ -1104,6 +1140,8 @@ void InputQueue::ProcessQueue() {
   if (CanDiscardBlock(mActiveKeyboardBlock)) {
     mActiveKeyboardBlock = nullptr;
   }
+
+  return processQueueAgain;
 }
 
 bool InputQueue::CanDiscardBlock(InputBlockState* aBlock) {

gfx/layers/apz/src/InputQueue.h

@@ -233,7 +233,11 @@ class InputQueue {
                                  CancelableBlockState* aBlock);
   void MainThreadTimeout(uint64_t aInputBlockId);
   void MaybeLongTapTimeout(uint64_t aInputBlockId);
-  void ProcessQueue();
+
+  // Returns true if there's one more queued event we need to process as a
+  // result of switching the active block back to the original touch block from
+  // the touch block for long-tap.
+  bool ProcessQueue();
   bool CanDiscardBlock(InputBlockState* aBlock);
   void UpdateActiveApzc(const RefPtr<AsyncPanZoomController>& aNewActive);
 

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/InputResultDetailTest.kt

@@ -414,4 +414,88 @@ class InputResultDetailTest : BaseSessionTest() {
             (PanZoomController.OVERSCROLL_FLAG_HORIZONTAL or PanZoomController.OVERSCROLL_FLAG_VERTICAL),
         )
     }
+
+    @WithDisplay(width = 100, height = 100)
+    @Test
+    fun testPreventTouchMoveAfterLongTap() {
+        sessionRule.display?.run { setDynamicToolbarMaxHeight(20) }
+
+        setupDocument(ROOT_100VH_HTML_PATH)
+
+        // Setup a touchmove event listener preventing scrolling.
+        val touchmovePromise = mainSession.evaluatePromiseJS(
+            """
+            new Promise(resolve => {
+                window.addEventListener('touchmove', (e) => {
+                    e.preventDefault();
+                    resolve(true);
+                }, { passive: false });
+            });
+            """.trimIndent(),
+        )
+
+        // Setup a contextmenu event.
+        val contextmenuPromise = mainSession.evaluatePromiseJS(
+            """
+            new Promise(resolve => {
+                window.addEventListener('contextmenu', (e) => {
+                    e.preventDefault();
+                    resolve(true);
+                }, { once: true });
+            });
+            """.trimIndent(),
+        )
+
+        mainSession.flushApzRepaints()
+
+        val downTime = SystemClock.uptimeMillis()
+        val down = MotionEvent.obtain(
+            downTime,
+            SystemClock.uptimeMillis(),
+            MotionEvent.ACTION_DOWN,
+            50f,
+            50f,
+            0,
+        )
+        val result = mainSession.panZoomController.onTouchEventForDetailResult(down)
+
+        // Wait until a contextmenu event happens.
+        assertThat("contextmenu", contextmenuPromise.value as Boolean, equalTo(true))
+
+        // Start moving.
+        val move = MotionEvent.obtain(
+            downTime,
+            SystemClock.uptimeMillis(),
+            MotionEvent.ACTION_MOVE,
+            50f,
+            70f,
+            0,
+        )
+        mainSession.panZoomController.onTouchEvent(move)
+
+        assertThat("touchmove", touchmovePromise.value as Boolean, equalTo(true))
+
+        val value = sessionRule.waitForResult(result)
+
+        // The input result for the initial touch-start event should have been handled by
+        // the content.
+        assertResultDetail(
+            ROOT_100VH_HTML_PATH,
+            value,
+            PanZoomController.INPUT_RESULT_HANDLED_CONTENT,
+            PanZoomController.SCROLLABLE_FLAG_BOTTOM,
+            (PanZoomController.OVERSCROLL_FLAG_HORIZONTAL or PanZoomController.OVERSCROLL_FLAG_VERTICAL),
+        )
+
+        val up = MotionEvent.obtain(
+            downTime,
+            SystemClock.uptimeMillis(),
+            MotionEvent.ACTION_UP,
+            50f,
+            70f,
+            0,
+        )
+
+        mainSession.panZoomController.onTouchEvent(up)
+    }
 }