Bug 749384 - Notify PZC of some events before the touchevent listeners deal with it so that we remain responsive. r=wesj a=mfinkle
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 15 May 2012 13:22:26 -0400
changeset 95789 bc28e94bedd622181abc467a31d7f9b59f7d79f0
parent 95788 c3339c6f1ab85ded5faa0420fc758a6712fed047
child 95790 d9e6e9bb3979cb919d17ea851cbdf6ad6126f7e0
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj, mfinkle
bugs749384
milestone14.0a2
Bug 749384 - Notify PZC of some events before the touchevent listeners deal with it so that we remain responsive. r=wesj a=mfinkle
mobile/android/base/gfx/TouchEventHandler.java
mobile/android/base/ui/PanZoomController.java
--- a/mobile/android/base/gfx/TouchEventHandler.java
+++ b/mobile/android/base/gfx/TouchEventHandler.java
@@ -38,16 +38,21 @@ import org.mozilla.gecko.Tabs;
  * default-prevented, which means that the DOWN/POINTER_DOWN event that started
  * the block, or the first MOVE event following that, were prevent-defaulted.
  *
  * A "default-prevented notification" is when we here in Java-land receive a notification
  * from gecko as to whether or not a block of events was default-prevented. This happens
  * at some point after the first or second event in the block is processed in Gecko.
  * This code assumes we get EXACTLY ONE default-prevented notification for each block
  * of events.
+ *
+ * Note that even if all events are default-prevented, we still send specific types
+ * of notifications to the pan/zoom controller. The notifications are needed
+ * to respond to user actions a timely manner regardless of default-prevention,
+ * and fix issues like bug 749384.
  */
 public final class TouchEventHandler implements Tabs.OnTabsChangedListener {
     private static final String LOGTAG = "GeckoTouchEventHandler";
 
     // The time limit for listeners to respond with preventDefault on touchevents
     // before we begin panning the page
     private final int EVENT_LISTENER_TIMEOUT = 200;
 
@@ -146,16 +151,22 @@ public final class TouchEventHandler imp
             // this is the start of a new block of events! whee!
             mHoldInQueue = mWaitForTouchListeners;
 
             // Set mDispatchEvents to true so that we are guaranteed to either queue these
             // events or dispatch them. The only time we should not do either is once we've
             // heard back from content to preventDefault this block.
             mDispatchEvents = true;
             if (mHoldInQueue) {
+                // if the new block we are starting is the current block (i.e. there are no
+                // other blocks waiting in the queue, then we should let the pan/zoom controller
+                // know we are waiting for the touch listeners to run
+                if (mEventQueue.isEmpty()) {
+                    mPanZoomController.waitingForTouchListeners(event);
+                }
                 // if we're holding the events in the queue, set the timeout so that
                 // we dispatch these events if we don't get a default-prevented notification
                 mView.postDelayed(mListenerTimeoutProcessor, EVENT_LISTENER_TIMEOUT);
             } else {
                 // if we're not holding these events, then we still need to pretend like
                 // we did and had a ListenerTimeoutProcessor fire so that when we get
                 // the default-prevented notification for this block, it doesn't accidentally
                 // act upon some other block
@@ -167,16 +178,18 @@ public final class TouchEventHandler imp
         // it directly, do that. it is possible that both mHoldInQueue and mDispatchEvents
         // are false, in which case we are processing a block of events that we know
         // has been default-prevented. in that case we don't keep the events as we don't
         // need them (but we still pass them to the gecko listener).
         if (mHoldInQueue) {
             mEventQueue.add(MotionEvent.obtain(event));
         } else if (mDispatchEvents) {
             dispatchEvent(event);
+        } else if (touchFinished(event)) {
+            mPanZoomController.preventedTouchFinished();
         }
 
         // notify gecko of the event
         mOnTouchListener.onTouch(mView, event);
 
         return true;
     }
 
@@ -210,16 +223,21 @@ public final class TouchEventHandler imp
         mOnTouchListener = onTouchListener;
     }
 
     private boolean isDownEvent(MotionEvent event) {
         int action = (event.getAction() & MotionEvent.ACTION_MASK);
         return (action == MotionEvent.ACTION_DOWN || action == MotionEvent.ACTION_POINTER_DOWN);
     }
 
+    private boolean touchFinished(MotionEvent event) {
+        int action = (event.getAction() & MotionEvent.ACTION_MASK);
+        return (action == MotionEvent.ACTION_UP || action == MotionEvent.ACTION_CANCEL);
+    }
+
     /**
      * Dispatch the event to the gesture detectors and the pan/zoom controller.
      */
     private void dispatchEvent(MotionEvent event) {
         if (mGestureDetector.onTouchEvent(event)) {
             return;
         }
         mScaleGestureDetector.onTouchEvent(event);
@@ -247,31 +265,34 @@ public final class TouchEventHandler imp
         // the next DOWN or POINTER_DOWN event.
 
         MotionEvent event = mEventQueue.poll();
         while (true) {
             // for each event we process, only dispatch it if the block hasn't been
             // default-prevented.
             if (allowDefaultAction) {
                 dispatchEvent(event);
+            } else if (touchFinished(event)) {
+                mPanZoomController.preventedTouchFinished();
             }
             event = mEventQueue.peek();
             if (event == null) {
                 // we have processed the backlog of events, and are all caught up.
                 // now we can set clear the hold flag and set the dispatch flag so
                 // that the handleEvent() function can do the right thing for all
                 // remaining events in this block (which is still ongoing) without
                 // having to put them in the queue.
                 mHoldInQueue = false;
                 mDispatchEvents = allowDefaultAction;
                 break;
             }
             if (isDownEvent(event)) {
                 // we have finished processing the block we were interested in.
                 // now we wait for the next call to processEventBlock
+                mPanZoomController.waitingForTouchListeners(event);
                 break;
             }
             // pop the event we peeked above, as it is still part of the block and
             // we want to keep processing
             mEventQueue.remove();
         }
     }
 
--- a/mobile/android/base/ui/PanZoomController.java
+++ b/mobile/android/base/ui/PanZoomController.java
@@ -124,17 +124,21 @@ public class PanZoomController
         TOUCHING,       /* one touch-start event received */
         PANNING_LOCKED, /* touch-start followed by move (i.e. panning with axis lock) */
         PANNING,        /* panning without axis lock */
         PANNING_HOLD,   /* in panning, but not moving.
                          * similar to TOUCHING but after starting a pan */
         PANNING_HOLD_LOCKED, /* like PANNING_HOLD, but axis lock still in effect */
         PINCHING,       /* nth touch-start, where n > 1. this mode allows pan and zoom */
         ANIMATED_ZOOM,  /* animated zoom to a new rect */
-        BOUNCE          /* in a bounce animation */
+        BOUNCE,         /* in a bounce animation */
+
+        WAITING_LISTENERS, /* a state halfway between NOTHING and TOUCHING - the user has
+                        put a finger down, but we don't yet know if a touch listener has
+                        prevented the default actions yet. we still need to abort animations. */
     }
 
     private final LayerController mController;
     private final SubdocumentScrollHelper mSubscroller;
     private final Axis mX;
     private final Axis mY;
 
     private Thread mMainThread;
@@ -293,16 +297,40 @@ public class PanZoomController
             synchronized (mController) {
                 mController.setViewportMetrics(getValidViewportMetrics());
                 mController.notifyLayerClientOfGeometryChange();
             }
             break;
         }
     }
 
+    /** This function must be called on the UI thread. */
+    public void waitingForTouchListeners(MotionEvent event) {
+        checkMainThread();
+        if ((event.getAction() & MotionEvent.ACTION_MASK) == MotionEvent.ACTION_DOWN) {
+            // this is the first touch point going down, so we enter the pending state
+            mSubscroller.cancel();
+            // seting the state will kill any animations in progress, possibly leaving
+            // the page in overscroll
+            mState = PanZoomState.WAITING_LISTENERS;
+        }
+    }
+
+    /** This function must be called on the UI thread. */
+    public void preventedTouchFinished() {
+        checkMainThread();
+        if (mState == PanZoomState.WAITING_LISTENERS) {
+            // if we enter here, we just finished a block of events whose default actions
+            // were prevented by touch listeners. Now there are no touch points left, so
+            // we need to reset our state and re-bounce because we might be in overscroll
+            mState = PanZoomState.NOTHING;
+            bounce();
+        }
+    }
+
     /** This must be called on the UI thread. */
     public void pageSizeUpdated() {
         if (mState == PanZoomState.NOTHING) {
             synchronized (mController) {
                 ViewportMetrics validated = getValidViewportMetrics();
                 if (! (new ViewportMetrics(mController.getViewportMetrics())).fuzzyEquals(validated)) {
                     // page size changed such that we are now in overscroll. snap to the
                     // the nearest valid viewport
@@ -329,16 +357,17 @@ public class PanZoomController
             // case this touchstart is just a tap that doesn't end up triggering
             // a redraw
             mController.setForceRedraw();
             mController.notifyLayerClientOfGeometryChange();
             // fall through
         case FLING:
         case BOUNCE:
         case NOTHING:
+        case WAITING_LISTENERS:
             startTouch(event.getX(0), event.getY(0), event.getEventTime());
             return false;
         case TOUCHING:
         case PANNING:
         case PANNING_LOCKED:
         case PANNING_HOLD:
         case PANNING_HOLD_LOCKED:
         case PINCHING:
@@ -349,16 +378,17 @@ public class PanZoomController
         return false;
     }
 
     private boolean onTouchMove(MotionEvent event) {
 
         switch (mState) {
         case FLING:
         case BOUNCE:
+        case WAITING_LISTENERS:
             // should never happen
             Log.e(LOGTAG, "Received impossible touch move while in " + mState);
             // fall through
         case ANIMATED_ZOOM:
         case NOTHING:
             // may happen if user double-taps and drags without lifting after the
             // second tap. ignore the move if this happens.
             return false;
@@ -397,16 +427,17 @@ public class PanZoomController
         return false;
     }
 
     private boolean onTouchEnd(MotionEvent event) {
 
         switch (mState) {
         case FLING:
         case BOUNCE:
+        case WAITING_LISTENERS:
             // should never happen
             Log.e(LOGTAG, "Received impossible touch end while in " + mState);
             // fall through
         case ANIMATED_ZOOM:
         case NOTHING:
             // may happen if user double-taps and drags without lifting after the
             // second tap. ignore if this happens.
             return false;
@@ -431,18 +462,28 @@ public class PanZoomController
             mState = PanZoomState.NOTHING;
             return true;
         }
         Log.e(LOGTAG, "Unhandled case " + mState + " in onTouchEnd");
         return false;
     }
 
     private boolean onTouchCancel(MotionEvent event) {
+        if (mState == PanZoomState.WAITING_LISTENERS) {
+            // we might get a cancel event from the TouchEventHandler while in the
+            // WAITING_LISTENERS state if the touch listeners prevent-default the
+            // block of events. at this point being in WAITING_LISTENERS is equivalent
+            // to being in NOTHING with the exception of possibly being in overscroll.
+            // so here we don't want to do anything right now; the overscroll will be
+            // corrected in preventedTouchFinished().
+            return false;
+        }
+
+        cancelTouch();
         mState = PanZoomState.NOTHING;
-        cancelTouch();
         // ensure we snap back if we're overscrolled
         bounce();
         return false;
     }
 
     private void startTouch(float x, float y, long time) {
         mX.startTouch(x);
         mY.startTouch(y);