765407 - crash java.lang.NullPointerException: at android.view.GestureDetector.onTouchEvent(GestureDetector.java) part 2

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

This is a testcase that is still crashing in current trunk build on the Galaxy Nexus after bug 745250 is fixed.

Steps to reproduce:
- Open testcas, wait till it opens a window
- Then, double tap continuously, after a while Fennec should crash


This bug was filed from the Socorro interface and is 
report bp-c568a154-cfcd-4b31-851c-eec562120615 .
============================================================= 
java.lang.NullPointerException
	at android.view.GestureDetector.onTouchEvent(GestureDetector.java:563)
	at org.mozilla.gecko.gfx.TouchEventHandler.dispatchEvent(TouchEventHandler.java:249)
	at org.mozilla.gecko.gfx.TouchEventHandler.handleEvent(TouchEventHandler.java:189)
	at org.mozilla.gecko.gfx.LayerView.onTouchEvent(LayerView.java:90)
	at android.view.View.dispatchTouchEvent(View.java:5541)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:1957)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1726)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.superDispatchTouchEvent(PhoneWindow.java:1912)
	at com.android.internal.policy.impl.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1371)
	at android.app.Activity.dispatchTouchEvent(Activity.java:2364)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchTouchEvent(PhoneWindow.java:1860)
	at android.view.View.dispatchPointerEvent(View.java:5721)
	at android.view.ViewRootImpl.deliverPointerEvent(ViewRootImpl.java:2890)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2466)
	at android.view.ViewRootImpl.processInputEvents(ViewRootImpl.java:845)
	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:859)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2442)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4424)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:784)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
	at dalvik.system.NativeStart.main(Native Method)

Comment 1

[Martijn Wargers (dead)]

From bug 745250, comment 28:
This crash seems to be different; best to file a new bug. This one is happening
on this line in GestureDetector.java:

               handled |= mDoubleTapListener.onDoubleTapEvent(ev);

probably because we set the double-tap listener to null in some cases. This
crash seems to be a regression from bug 707571.

Comment 2

[Matt Brubeck (:mbrubeck)]

From reading the Android code, this can happen if the user double-taps, and then the page changes while their finger is still on the screen and we call setOnDoubleTapListener(null) before they release it.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch

We don't actually need to set the double-tap listener to null.  If we just leave it enabled, then PanZoomController will receive some extra events, but it already has code to ignore those events based on LayerController.getAllowZoom().

Comment 4

[Matt Brubeck (:mbrubeck)]

Alternate steps to reproduce the crash:
1. Go to a page that is user-scalable like http://limpet.net/mbrubeck/
2. Click a link to a page that is not user-scalable (like the "Twitter" link on that page).
3. Very quickly, before the link loads, double-tap and leave your finger down after the second tap.
4. After the link loads, lift your finger.

Comment 5

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

(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> We don't actually need to set the double-tap listener to null.  If we just
> leave it enabled, then PanZoomController will receive some extra events, but
> it already has code to ignore those events based on
> LayerController.getAllowZoom().

Does the behaviour you described at [1] still happen?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=707571#c36

Comment 6

[Matt Brubeck (:mbrubeck)]

Yeah, the fix from bug 707571 comment 36 is still needed, and works as intended.   PanZoomController.onSingleTapUp still consumes ACTION_UP events on pages with zooming disabled, so we need the extra code in TouchEventHandler to pass them to the SimpleScaleGestureDetector.

I'll revisit that code in bug 756474.

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 634032 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/a605c9f4218b

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 729198

User impact if declined: Reproducible crashes when double-tapping on certain web pages during page load.

Testing completed (on m-c, etc.): Landed on inbound 6/18

Risk to taking this patch (and alternatives if risky): This is a low-risk mobile-only patch that just backs out part of bug 729198 that turned out to be unnecessary (and harmful).

String or UUID changes made by this patch: None.

Comment 8

[Johnathan Nightingale [:johnath]]

Comment on attachment 634032 [details] [diff] [review]
patch

.N+ - clear to land on aurora and beta tip, but not the FN14 relbranch

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a605c9f4218b

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ded367e9699f
https://hg.mozilla.org/releases/mozilla-beta/rev/7945471fc07f

Comment 11

[Adrian Tamas (:AdrianT)]

Unable to get Firefox to crash following the steps in comment 0.

Verified on:
Nightly 16.0a1 2012-07-11/Aurora 15.0a2 2012-07-11/Firefox Mobile 14 beta 12 build 1
HTC Desire
Android 2.2.2