Closed Bug 756474 Opened 12 years ago Closed 12 years ago

Harden the SimpleScaleGestureDectector against missed events

Categories

(Firefox for Android Graveyard :: General, defect)

15 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 16

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file, 1 obsolete file)

In bug 707571 I came across a case where SimpleScaleGestureDectector's list of active pointers was incorrect, because one of the pointers it was tracking sent an event which another gesture dectector consumed.

In bug 707571 comment 41, kats suggested:
> We should probably harden the SimpleScaleGestureDetector against this case in general,
> by clearing the list of stored touch points before processing an ACTION_DOWN event.

If this works, we can remove the work-around added here:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce227a3c263a#l2.74

Assigning to myself so I don't forget to do this, but feel free to steal this if you want to fix it while I'm on vacation.
By the way, the original bug was fixed upstream in newer versions of Android:
https://github.com/android/platform_frameworks_base/commit/17921eef

If I back out my fix then I can reproduce it on Android 2.3, but not on Android 4.0.
Attached patch patch (obsolete) — Splinter Review
Attachment #634068 - Flags: review?(bugmail.mozilla)
Status: NEW → ASSIGNED
Comment on attachment 634068 [details] [diff] [review]
patch

Review of attachment 634068 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/ui/SimpleScaleGestureDetector.java
@@ +50,5 @@
>      /** Forward touch events to this function. */
>      public void onTouchEvent(MotionEvent event) {
>          switch (event.getAction() & MotionEvent.ACTION_MASK) {
>          case MotionEvent.ACTION_DOWN:
> +            mPointerInfo.clear();

Instead of just clearing the list here, I think we want to run through the code in onTouchEnd pretending that a cancel event came in. In particular we shoudl recycle the pointerInfo objects and send a EventType.END scale gesture to the listener if there are stored pointers.
Attachment #634068 - Flags: review?(bugmail.mozilla) → review-
Attached patch patch v2Splinter Review
(In reply to Kartikaya Gupta (:kats) from comment #3)
> Instead of just clearing the list here, I think we want to run through the
> code in onTouchEnd pretending that a cancel event came in.

Done.
Attachment #634068 - Attachment is obsolete: true
Attachment #634495 - Flags: review?(bugmail.mozilla)
Comment on attachment 634495 [details] [diff] [review]
patch v2

Review of attachment 634495 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks!
Attachment #634495 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/dfa91708c0e1
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.