937274 - Crash in GestureEventListener::HandlePinchGestureEvent(), line 260

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Botond Ballo [:botond]]

This shows up in recent m-c.

STR:
  1. Run B2G browser, load a page.
  2. Zoom in and out a few times. The crash reproduces intermittently, but fairly frequently.

Some debugging reveals that the GestureEventListener sees a MULTITOUCH_END, clears mTouches in HandleInputEvent(), and then calls HandlePinchGestureEvent() in the last line of HandleInputEvent(), which expects mTouches to be nonempty.

Comment 1

[Botond Ballo [:botond]]

Investigating this bug led me to discover a related issue with PINCHGESTURE_END events, for which I filed bug 937343. I think I can fix this bug independently of that one.

Comment 2

[Botond Ballo [:botond]]

Attachment: bug937274.patch

In addition to fixing the crash, this patch actually fixes the issue in bug 937343, though not in the nicest way (we still store a value that's a touch point in a field called mFocusPoint).

Try push: https://tbpl.mozilla.org/?tree=Try&rev=cc2def03af9d

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 830475 [details] [diff] [review]
bug937274.patch

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

Presumably you'll put in a patch in bug 937343 that removes the bad use of focus point and kicks off a MULTITOUCH_START instead?

Also the ApzcPinch function in TestAsyncPanZoomController.cpp should probably be updated to have -1 on the spans although leaving them as-is won't cause the test to fail or anything.

Comment 4

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Comment on attachment 830475 [details] [diff] [review]
> bug937274.patch
> 
> Review of attachment 830475 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Presumably you'll put in a patch in bug 937343 that removes the bad use of
> focus point and kicks off a MULTITOUCH_START instead?

Yeah, unless someone has a better suggestion.

> Also the ApzcPinch function in TestAsyncPanZoomController.cpp should
> probably be updated to have -1 on the spans although leaving them as-is
> won't cause the test to fail or anything.

Good catch, I will do that.

Comment 5

[Botond Ballo [:botond]]

Attachment: bug937274.patch

Updated TestAsyncPanZoomController as per Kats' suggestion. Carrying r+.

Comment 6

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)
> Try push: https://tbpl.mozilla.org/?tree=Try&rev=cc2def03af9d

Looks clean.

https://hg.mozilla.org/integration/mozilla-inbound/rev/548b80dc1998

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/548b80dc1998