Bug 1256341 - Guard against scenarios where GenerateSingleTap is called without an active touch block. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 16 Mar 2016 17:58:37 -0400
changeset 289153 772bdc0bc0ea1131a065a0f7564e93b5d24e19f1
parent 289152 7a7c00aa23976ff1b02bdaf622cfe55c32c1ddef
child 289154 c7969fc23e51c269bf9cbcce200dc05e26214dc8
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1256341
milestone48.0a1
Bug 1256341 - Guard against scenarios where GenerateSingleTap is called without an active touch block. r=botond MozReview-Commit-ID: 8nnj9IlwIuL
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
gfx/layers/apz/src/GestureEventListener.cpp
gfx/layers/apz/src/GestureEventListener.h
gfx/layers/apz/src/InputBlockState.cpp
gfx/layers/apz/src/InputBlockState.h
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2036,16 +2036,17 @@ nsEventStatus AsyncPanZoomController::On
 
 nsEventStatus AsyncPanZoomController::OnLongPress(const TapGestureInput& aEvent) {
   APZC_LOG("%p got a long-press in state %d\n", this, mState);
   RefPtr<GeckoContentController> controller = GetGeckoContentController();
   if (controller) {
     CSSPoint geckoScreenPoint;
     if (ConvertToGecko(aEvent.mPoint, &geckoScreenPoint)) {
       CancelableBlockState* block = CurrentInputBlock();
+      MOZ_ASSERT(block);
       if (!block->AsTouchBlock()) {
         APZC_LOG("%p dropping long-press because some non-touch block interrupted it\n", this);
         return nsEventStatus_eIgnore;
       }
       if (block->AsTouchBlock()->IsDuringFastFling()) {
         APZC_LOG("%p dropping long-press because of fast fling\n", this);
         return nsEventStatus_eIgnore;
       }
@@ -2062,18 +2063,31 @@ nsEventStatus AsyncPanZoomController::On
   return GenerateSingleTap(aEvent.mPoint, aEvent.modifiers);
 }
 
 nsEventStatus AsyncPanZoomController::GenerateSingleTap(const ScreenIntPoint& aPoint, mozilla::Modifiers aModifiers) {
   RefPtr<GeckoContentController> controller = GetGeckoContentController();
   if (controller) {
     CSSPoint geckoScreenPoint;
     if (ConvertToGecko(aPoint, &geckoScreenPoint)) {
-      if (!CurrentTouchBlock()->SetSingleTapOccurred()) {
-        return nsEventStatus_eIgnore;
+      CancelableBlockState* block = CurrentInputBlock();
+      MOZ_ASSERT(block);
+      TouchBlockState* touch = block->AsTouchBlock();
+      // |block| may be a non-touch block in the case where this function is
+      // invoked by GestureEventListener on a timeout. In that case we already
+      // verified that the single tap is allowed so we let it through.
+      // XXX there is a bug here that in such a case the touch block that
+      // generated this tap will not get its mSingleTapOccurred flag set.
+      // See https://bugzilla.mozilla.org/show_bug.cgi?id=1256344#c6
+      if (touch) {
+        if (touch->IsDuringFastFling()) {
+          APZC_LOG("%p dropping single-tap because it was during a fast-fling\n", this);
+          return nsEventStatus_eIgnore;
+        }
+        touch->SetSingleTapOccurred();
       }
       // Because this may be being running as part of APZCTreeManager::ReceiveInputEvent,
       // calling controller->HandleSingleTap directly might mean that content receives
       // the single tap message before the corresponding touch-up. To avoid that we
       // schedule the singletap message to run on the next spin of the event loop.
       // See bug 965381 for the issue this was causing.
       controller->PostDelayedTask(
         NewRunnableMethod(controller.get(), &GeckoContentController::HandleSingleTap,
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -617,21 +617,16 @@ protected:
   /**
    * Gets the pointer to the apzc tree manager. All the access to tree manager
    * should be made via this method and not via private variable since this method
    * ensures that no lock is set.
    */
   APZCTreeManager* GetApzcTreeManager() const;
 
   /**
-   * Gets a ref to the input queue that is shared across the entire tree manager.
-   */
-  const RefPtr<InputQueue>& GetInputQueue() const;
-
-  /**
    * Convert ScreenPoint relative to the screen to CSSPoint relative
    * to the parent document. This excludes the transient compositor transform.
    * NOTE: This must be converted to CSSPoint relative to the child
    * document before sending over IPC to a child process.
    */
   bool ConvertToGecko(const ScreenIntPoint& aPoint, CSSPoint* aOut);
 
   enum AxisLockMode {
@@ -815,16 +810,21 @@ public:
    */
   bool ArePointerEventsConsumable(TouchBlockState* aBlock, uint32_t aTouchPoints);
 
   /**
    * Clear internal state relating to touch input handling.
    */
   void ResetTouchInputState();
 
+  /**
+   * Gets a ref to the input queue that is shared across the entire tree manager.
+   */
+  const RefPtr<InputQueue>& GetInputQueue() const;
+
 private:
   void CancelAnimationAndGestureState();
 
   RefPtr<InputQueue> mInputQueue;
   CancelableBlockState* CurrentInputBlock() const;
   TouchBlockState* CurrentTouchBlock() const;
   bool HasReadyTouchBlock() const;
 
--- a/gfx/layers/apz/src/GestureEventListener.cpp
+++ b/gfx/layers/apz/src/GestureEventListener.cpp
@@ -439,28 +439,30 @@ void GestureEventListener::HandleInputTi
   }
   default:
     NS_WARNING("Unhandled state upon long tap timeout");
     SetState(GESTURE_NONE);
     break;
   }
 }
 
-void GestureEventListener::HandleInputTimeoutMaxTap()
+void GestureEventListener::HandleInputTimeoutMaxTap(bool aDuringFastFling)
 {
   GEL_LOG("Running max-tap timeout task in state %d\n", mState);
 
   mMaxTapTimeoutTask = nullptr;
 
   if (mState == GESTURE_FIRST_SINGLE_TOUCH_DOWN) {
     SetState(GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN);
   } else if (mState == GESTURE_FIRST_SINGLE_TOUCH_UP ||
              mState == GESTURE_SECOND_SINGLE_TOUCH_DOWN) {
     SetState(GESTURE_NONE);
-    TriggerSingleTapConfirmedEvent();
+    if (!aDuringFastFling) {
+      TriggerSingleTapConfirmedEvent();
+    }
   } else {
     NS_WARNING("Unhandled state upon MAX_TAP timeout");
     SetState(GESTURE_NONE);
   }
 }
 
 void GestureEventListener::TriggerSingleTapConfirmedEvent()
 {
@@ -515,18 +517,20 @@ void GestureEventListener::CancelMaxTapT
     mMaxTapTimeoutTask = nullptr;
   }
 }
 
 void GestureEventListener::CreateMaxTapTimeoutTask()
 {
   mLastTapInput = mLastTouchInput;
 
+  TouchBlockState* block = mAsyncPanZoomController->GetInputQueue()->CurrentTouchBlock();
   mMaxTapTimeoutTask =
-    NewRunnableMethod(this, &GestureEventListener::HandleInputTimeoutMaxTap);
+    NewRunnableMethod(this, &GestureEventListener::HandleInputTimeoutMaxTap,
+                      block->IsDuringFastFling());
 
   mAsyncPanZoomController->PostDelayedTask(
     mMaxTapTimeoutTask,
     MAX_TAP_TIME);
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/src/GestureEventListener.h
+++ b/gfx/layers/apz/src/GestureEventListener.h
@@ -132,17 +132,17 @@ private:
    * finite-state machine triggering state transitions.
    */
   nsEventStatus HandleInputTouchSingleStart();
   nsEventStatus HandleInputTouchMultiStart();
   nsEventStatus HandleInputTouchEnd();
   nsEventStatus HandleInputTouchMove();
   nsEventStatus HandleInputTouchCancel();
   void HandleInputTimeoutLongTap();
-  void HandleInputTimeoutMaxTap();
+  void HandleInputTimeoutMaxTap(bool aDuringFastFling);
 
   void TriggerSingleTapConfirmedEvent();
 
   bool MoveDistanceIsLarge();
 
   /**
    * Do actual state transition and reset substates.
    */
--- a/gfx/layers/apz/src/InputBlockState.cpp
+++ b/gfx/layers/apz/src/InputBlockState.cpp
@@ -736,26 +736,21 @@ TouchBlockState::SetDuringFastFling()
 }
 
 bool
 TouchBlockState::IsDuringFastFling() const
 {
   return mDuringFastFling;
 }
 
-bool
+void
 TouchBlockState::SetSingleTapOccurred()
 {
-  TBS_LOG("%p attempting to set single-tap occurred; disallowed=%d\n",
-    this, mDuringFastFling);
-  if (!mDuringFastFling) {
-    mSingleTapOccurred = true;
-    return true;
-  }
-  return false;
+  TBS_LOG("%p setting single-tap-occurred flag\n", this);
+  mSingleTapOccurred = true;
 }
 
 bool
 TouchBlockState::SingleTapOccurred() const
 {
   return mSingleTapOccurred;
 }
 
--- a/gfx/layers/apz/src/InputBlockState.h
+++ b/gfx/layers/apz/src/InputBlockState.h
@@ -411,20 +411,18 @@ public:
   void SetDuringFastFling();
   /**
    * @return true iff SetDuringFastFling was called on this block.
    */
   bool IsDuringFastFling() const;
   /**
    * Set the single-tap-occurred flag that indicates that this touch block
    * triggered a single tap event.
-   * @return true if the flag was set. This may not happen if, for example,
-   *         SetDuringFastFling was previously called.
    */
-  bool SetSingleTapOccurred();
+  void SetSingleTapOccurred();
   /**
    * @return true iff the single-tap-occurred flag is set on this block.
    */
   bool SingleTapOccurred() const;
 
   /**
    * Add a new touch event to the queue of events in this input block.
    */