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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Flaviu Cos]

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)

Comment 1

[Aaron Train [:aaronmt]]

I can't reproduce this on my Nexus 5.

Comment 2

[Flaviu Cos]

I think this issue is related only to Gingerbread devices due to different text input style in landscape.

Comment 3

[Ioana Chiorean]

Got into this while testing 34.0b6:
https://crash-stats.mozilla.com/report/index/1877a932-fd0f-4ea3-821d-981412141104
https://crash-stats.mozilla.com/report/index/18a1354d-956c-4b10-8de9-9f6d62141104

Comment 4

[:Margaret Leibovic]

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.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: pinsite-crash v0.1

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.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6053ffd738ce

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6053ffd738ce

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8255ad6f81e7

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/35515250c8fb

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/35515250c8fb

Comment 14

[Flaviu Cos]

Attachment: 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.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

I fixed the bad merge. Should be good for the next Beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/29d96e53182f

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/29d96e53182f

Comment 18

[u549602]

Verified as fixed on the last Beta build (44.0b7), verified on a HTC Desire with 2.3.3 Android