Closed Bug 1072959 Opened 5 years ago Closed 4 years ago

crash in java.lang.NullPointerException: at org.mozilla.gecko.home.PinSiteDialog$3.onFocusChange(PinSiteDialog.java)

Categories

(Firefox for Android :: Awesomescreen, defect, critical)

All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox33 --- affected
firefox34 --- affected
firefox35 --- affected
firefox36 --- affected
firefox37 --- affected
firefox38 --- affected
firefox39 --- affected
firefox40 --- affected
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected
firefox44 --- verified
firefox45 --- fixed
firefox46 --- fixed
b2g-v2.5 --- fixed
fennec - ---

People

(Reporter: cos_flaviu, Assigned: mfinkle)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-459a5a75-c795-4315-ae60-f96fd2140925.
=============================================================
Environment: 
Device: Samsung Galaxy R (Android 2.3.4);
Build: Nightly 35.0a1 (2014-09-25);

Steps to reproduce:
1. Tap on an empty grid from top sites;
2. Change device orientation to landscape.

Expected result:
The device orientation is successfully changed.

Actual result:
Firefox crashes.

Stack trace:
java.lang.NullPointerException
	at org.mozilla.gecko.home.PinSiteDialog$3.onFocusChange(PinSiteDialog.java:132)
	at android.view.View.onFocusChanged(View.java:2831)
	at android.widget.TextView.onFocusChanged(TextView.java:7089)
	at android.widget.EditText.onFocusChanged(EditText.java:239)
	at android.view.View.handleFocusGainInternal(View.java:2654)
	at android.view.View.requestFocus(View.java:3809)
	at android.view.ViewGroup.onRequestFocusInDescendants(ViewGroup.java:1079)
	at android.view.ViewGroup.requestFocus(ViewGroup.java:1035)
	at android.view.ViewGroup.onRequestFocusInDescendants(ViewGroup.java:1079)
	at android.view.ViewGroup.requestFocus(ViewGroup.java:1035)
	at android.view.ViewGroup.onRequestFocusInDescendants(ViewGroup.java:1079)
	at android.view.ViewGroup.requestFocus(ViewGroup.java:1035)
	at android.view.ViewGroup.onRequestFocusInDescendants(ViewGroup.java:1079)
	at android.view.ViewGroup.requestFocus(ViewGroup.java:1035)
	at android.view.ViewGroup.onRequestFocusInDescendants(ViewGroup.java:1079)
	at android.view.ViewGroup.requestFocus(ViewGroup.java:1038)
	at android.view.View.requestFocus(View.java:3760)
	at android.view.ViewRoot.performTraversals(ViewRoot.java:1259)
	at android.view.ViewRoot.handleMessage(ViewRoot.java:1897)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:130)
	at android.app.ActivityThread.main(ActivityThread.java:3691)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:907)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:665)
	at dalvik.system.NativeStart.main(Native Method)
I can't reproduce this on my Nexus 5.
I think this issue is related only to Gingerbread devices due to different text input style in landscape.
Crash Signature: [@ java.lang.NullPointerException: at org.mozilla.gecko.home.PinSiteDialog$3.onFocusChange(PinSiteDialog.java)] → [@ java.lang.NullPointerException: at org.mozilla.gecko.home.PinSiteDialog$3.onFocusChange(PinSiteDialog.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.PinSiteDialog$3.onFocusChange]
tracking-fennec: --- → ?
I don't see this in any top crash list, but it doesn't look like it would be that hard to write a band-aid patch to add a null check to avoid the NPE.
tracking-fennec: ? → -
Assignee: nobody → margaret.leibovic
I can consistently reproduce this crash on my Gingerbread device.
1. Open a new tab or load about:home
2. Long press on a tile
3. Press "Edit"
4. Pin Site dialog appears
5. Rotate device to landscape
6. Crash
Stealing since I could reproduce the crash.

This simple null-check stops the crash.

Looks like we close the PinSite dialog on rotation, but the sequence is:
1. destroy
2. create
3. set focus handler
4. destroy again

This happens on any rotation. The focus handler must get called after the second destroy (#4) and causes the crash.

Patch adds a comment about the race.
Assignee: margaret.leibovic → mark.finkle
Attachment #8703178 - Flags: review?(margaret.leibovic)
Attachment #8703178 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8703178 [details] [diff] [review]
pinsite-crash v0.1

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: This patch reduces crashes
[Describe test coverage new/current, TreeHerder]: Needs a little bake time
[Risks and why]: Low risk null check
[String/UUID change made/needed]: none
Attachment #8703178 - Flags: approval-mozilla-beta?
Attachment #8703178 - Flags: approval-mozilla-aurora?
Comment on attachment 8703178 [details] [diff] [review]
pinsite-crash v0.1

This hasn't landed on Nightly yet but patch looks safe enough to uplift to Beta44, Aurora45.
Attachment #8703178 - Flags: approval-mozilla-beta?
Attachment #8703178 - Flags: approval-mozilla-beta+
Attachment #8703178 - Flags: approval-mozilla-aurora?
Attachment #8703178 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/6053ffd738ce
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Attached file crash.txt
The crash is still reproducible on Firefox 44 Beta 6.
The only difference is that the crash reporter is no longer displayed.

Attached is a txt file containing the logs.
Flags: needinfo?(cbook)
The uplift to Beta got messed up:
http://mxr.mozilla.org/mozilla-beta/find?text=&string=PinSiteDialog.java

We now have two different files for PinSiteDialog.java

We need to back out the Beta changeset, and I can make a Beta-specific patch.
Flags: needinfo?(cbook)
I fixed the bad merge. Should be good for the next Beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/29d96e53182f
Verified as fixed on the last Beta build (44.0b7), verified on a HTC Desire with 2.3.3 Android
You need to log in before you can comment on or make changes to this bug.