797705 - java.lang.NullPointerException: at org.mozilla.gecko.GeckoEvent.createNativeGestureEvent(GeckoEvent.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There's one crash in 18.0a1/20121003: bp-783dd5e1-0287-438d-a33a-c39212121004.

java.lang.NullPointerException
	at org.mozilla.gecko.GeckoEvent.createNativeGestureEvent(GeckoEvent.java:276)
	at org.mozilla.gecko.ui.PanZoomController.onScaleEnd(PanZoomController.java:961)
	at org.mozilla.gecko.ui.SimpleScaleGestureDetector.sendScaleGesture(SimpleScaleGestureDetector.java:211)
	at org.mozilla.gecko.ui.SimpleScaleGestureDetector.onTouchEnd(SimpleScaleGestureDetector.java:129)
	at org.mozilla.gecko.ui.SimpleScaleGestureDetector.onTouchEvent(SimpleScaleGestureDetector.java:72)
	at org.mozilla.gecko.gfx.TouchEventHandler.dispatchEvent(TouchEventHandler.java:262)
	at org.mozilla.gecko.gfx.TouchEventHandler.handleEvent(TouchEventHandler.java:148)
	at org.mozilla.gecko.gfx.LayerView.onTouchEvent(LayerView.java:136)
	at android.view.View.dispatchTouchEvent(View.java:5568)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2064)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1732)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2070)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:1746)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.superDispatchTouchEvent(PhoneWindow.java:1961)
	at com.android.internal.policy.impl.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1398)
	at android.app.Activity.dispatchTouchEvent(Activity.java:2428)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchTouchEvent(PhoneWindow.java:1909)
	at android.view.View.dispatchPointerEvent(View.java:5748)
	at android.view.ViewRootImpl.deliverPointerEvent(ViewRootImpl.java:3165)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2719)
	at android.view.ViewRootImpl.processInputEvents(ViewRootImpl.java:951)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2733)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:156)
	at android.app.ActivityThread.main(ActivityThread.java:4987)
	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)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.GeckoEvent.createNativeGestureEvent%28GeckoEvent.java%29

Comment 1

[Scoobidiver (away)]

There's another crash from another user. The regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=85dd8e346102&tochange=635fcc11d2b1
It's likely a regression from bug 784887.

Comment 2

[Scoobidiver (away)]

It's #6 top crasher in 18.0a2.

Comment 3

[bhavana bajaj [:bajaj]]

Hey Scott passing this on to you as the suspected regressing bug is bug 784887.Can you please look at it as its a top crasher  ? Thanks !

Comment 4

[Scott Johnson (:jwir3)]

Sure, I'll take a look at it.

Comment 5

[Scott Johnson (:jwir3)]

The crash stack seems to indicate that the line:
>         event.mPoints[0] = new Point(Math.round(geckoPoint.x), Math.round(geckoPoint.y));
crashes with an NPE if geckoPoint is null. This can happen if the line:
> GeckoApp.mAppContext.getLayerView().convertViewPointToLayerPoint(geckoPoint);
returns null, which only appears to happen if GeckoLayerClient.mGeckoIsReady is false.

This appears to be the case if notifyGeckoReady() has not yet been called. 

So, the fix for this seems to be to wait until we have received a "Gecko:Ready" notification before starting to pass these messages. 

wesj, does this seem correct to you?

Comment 6

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

That's correct, but instead of checking to see if gecko is ready (which will be expensive) I would suggest just handling the null return and bailing out, similar to how PanZoomController.sendPointToGecko does it. In this case you can propagate a null return out of the createNativeGestureEvent method and then just not send anything to gecko.

Comment 7

[Scott Johnson (:jwir3)]

(In reply to Kartikaya Gupta (:kats) from comment #6)
> That's correct, but instead of checking to see if gecko is ready (which will
> be expensive) I would suggest just handling the null return and bailing out,
> similar to how PanZoomController.sendPointToGecko does it. In this case you
> can propagate a null return out of the createNativeGestureEvent method and
> then just not send anything to gecko.

Ah, ok. I didn't realize checking to see if Gecko was ready was expensive. Ok, I can do that. (I thought that was the cop-out way to do it, so I didn't want to suggest that originally, hehe.) 

Thanks for making life easier, kats! :)

Comment 8

[Scott Johnson (:jwir3)]

Attachment: b797705

Comment 9

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

Comment on attachment 676781 [details] [diff] [review]
b797705

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

r=me with style issues fixed.

::: mobile/android/base/GeckoEvent.java
@@ +276,5 @@
> +
> +        if (geckoPoint == null) {
> +        	// This could happen if Gecko isn't ready yet.
> +        	return null;
> +        }

4-space indent, please.

::: mobile/android/base/ui/PanZoomController.java
@@ +923,5 @@
>          PointF point = new PointF(detector.getFocusX(), detector.getFocusY());
>          GeckoEvent event = GeckoEvent.createNativeGestureEvent(GeckoEvent.ACTION_MAGNIFY_END, point, getMetrics().zoomFactor);
> +
> +        if (event == null)
> +        	return;

4-space indent, and braces.

Comment 10

[Scott Johnson (:jwir3)]

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51c7ee235438

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/51c7ee235438

Comment 12

[Scott Johnson (:jwir3)]

Landed on aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/b3a817168571