Closed Bug 781259 Opened 8 years ago Closed 8 years ago

java.lang.NullPointerException: at org.mozilla.gecko.DoorHangerPopup$3.run(DoorHangerPopup.java)

Categories

(Firefox for Android :: General, defect)

17 Branch
ARM
Android
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 19
Tracking Status
firefox17 - verified
firefox18 --- verified
firefox19 + verified
fennec 19+ ---

People

(Reporter: scoobidiver, Assigned: bnicholson)

Details

(4 keywords, Whiteboard: [native-crash][STR in comment 4, 11, and 13])

Crash Data

Attachments

(1 file)

There's one crash in 17.0a1/20120808: bp-ab6d1629-0e23-4459-8b71-79abe2120808.

java.lang.NullPointerException
	at org.mozilla.gecko.DoorHangerPopup$3.run(DoorHangerPopup.java:181)
	at android.app.Activity.runOnUiThread(Activity.java:4591)
	at org.mozilla.gecko.DoorHangerPopup.removeDoorHanger(DoorHangerPopup.java:179)
	at org.mozilla.gecko.DoorHangerPopup.removeTransientDoorHangers(DoorHangerPopup.java:196)
	at org.mozilla.gecko.DoorHangerPopup.onTabChanged(DoorHangerPopup.java:104)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:361)
	at org.mozilla.gecko.GeckoApp$1.run(GeckoApp.java:697)
	at android.os.Handler.handleCallback(Handler.java:615)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4745)
	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:786)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
	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.DoorHangerPopup%243.run%28DoorHangerPopup.java%29
It might be a regression from bug 732336.
Keywords: regression
Version: Trunk → Firefox 17
It's #9 top crasher (xul crash signatures count for one) in 17.0a2.
Keywords: topcrash
tracking-fennec: --- → ?
Sending over to Margaret for investigation.
Assignee: nobody → margaret.leibovic
I reproduced this crash on the latest Aurora and Nightly builds by following this STR:
* before running the following steps, copy this webpage to the clipboard: http://popuptest.com/popuptest1.html

1. Go to Settings and clear all data
2. Quit Firefox
3. Reopen it and long tap immediately on URL bar (3 options should be displayed in context menu)
4. Tap on Paste&Go option


Actual result:

Aurora: https://crash-stats.mozilla.com/report/index/bp-ca7bf3db-cc4a-43ab-aa15-cfb8e2121001
Nightly: https://crash-stats.mozilla.com/report/index/bp-316756d8-c5ca-4da2-a1f5-403062121001

--
Firefox 17.0a2 (2012-09-30)
Firefox 18.0a1 (2012-09-30)
Device: Galaxy Note
OS: Android 4.0.4
We could uplift this is low risk
Assignee: margaret.leibovic → sriram
tracking-fennec: ? → 19+
It's #29 top crasher in 17.0b1.
Keywords: topcrash
At #29 Fennec topcrasher, that's just not much to track on.  We'll consider an uplift if someone has a verifiable fix.
It starts spiking in 19.0a1/20121018 with startup crashes. The regression range for the spike is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5
Keywords: topcrash
There are about 10 crashes per build from the spike.
I was able to reproduce this crash in Nightly: with a clean profile, open Fennec and rotate the phone before the telemetry prompt appears.
Whiteboard: [native-crash] → [native-crash][STR in comment 4 and 11]
Adding some debugging messages, it looks like the following is happening:
1) addDoorHanger() is called.
2) The Runnable in addDoorHanger() is posted to the UI thread, but not yet run. Inside of this Runnable is where DoorHangerPopup is initialized.
3) removeDoorHanger() is called. This is called because we receive a LOCATION_CHANGE event for about:home, which calls removeTransientDoorHangers(), which then calls removeDoorHanger() for the doorhanger added in #1
4) The Runnable in removeDoorHanger() is posted to the UI thread, and it is run immediately. It is run immediately because our LOCATION_CHANGE callback is executed on the UI thread, and as described in the runOnUiThread() API [1], runOnUiThread() will immediately run the given Runnable if we're already on the UI thread.
5) We then call mContent.removeView() inside of this Runnable. Since it ran before the Runnable in #2, we never called init(), so mContent is null, causing the NPE.

This means we have two bugs here. One, we need to make sure the Runnable from addDoorHanger() always executes before the Runnable in removeDoorHanger(). Two, we shouldn't be triggering a removeDoorHanger() call at startup when we receive the LOCATION_CHANGE event for about:home.

[1] http://developer.android.com/reference/android/app/Activity.html#runOnUiThread%28java.lang.Runnable%29
It's quite easy to reproduce this crash if you go to popuptest.com/popuptest1.com, quit and restart Fennec and then tap on its thumbnail from about:home just after Fennec is opened.
Whiteboard: [native-crash][STR in comment 4 and 11] → [native-crash][STR in comment 4, 11, and 13]
Brian, do you have any new info about this crash? It's the #2 topcrash for Nightly 19 (and #28 on Beta 17b3).
DoorHangerPopup isn't designed to be thread-safe, but we're using and modifying mDoorHangers on both the Gecko thread and the UI thread. This is creating race conditions (see comment 12).

The easiest way to fix this is to make addDoorHanger(), removeDoorHanger(), etc. all run on the UI thread. There's no heavy operations like DB accesses or JSON parsing being done in these methods, so there's no need for these methods to be split between running on the Gecko/UI threads.

Also, bug 800199 makes the telemetry prompt doorhanger appear after about:home's location change event, so there's no need to use a persistence hack anymore.
Assignee: sriram → bnicholson
Attachment #676502 - Flags: review?(mark.finkle)
Attachment #676502 - Flags: review?(mark.finkle) → review+
Comment on attachment 676502 [details] [diff] [review]
Move DoorHangerPopup code to UI thread

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: doorhanger crashes
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Attachment #676502 - Flags: approval-mozilla-aurora?
Comment on attachment 676502 [details] [diff] [review]
Move DoorHangerPopup code to UI thread

Might be too late, but nom'ing for beta in case.
Attachment #676502 - Flags: approval-mozilla-beta?
http://hg.mozilla.org/mozilla-central/rev/c6cccc9e83fa
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Comment on attachment 676502 [details] [diff] [review]
Move DoorHangerPopup code to UI thread

build on trunk looks ok - approving for branches.
Attachment #676502 - Flags: approval-mozilla-beta?
Attachment #676502 - Flags: approval-mozilla-beta+
Attachment #676502 - Flags: approval-mozilla-aurora?
Attachment #676502 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.