crash in java.lang.NullPointerException: at org.mozilla.gecko.home.TopSitesPage$EditPinnedSiteListener.onSiteSelected(TopSitesPage.java)

RESOLVED FIXED in Firefox 28, Firefox OS v1.3

Status

()

Firefox for Android
Awesomescreen
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: aaronmt, Assigned: lucasr)

Tracking

(Blocks: 1 bug, {crash})

28 Branch
Firefox 30
All
Android
crash
Points:
---

Firefox Tracking Flags

(firefox28 verified, firefox29 verified, firefox30 verified, b2g-v1.3 fixed, fennec28+)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
This bug was filed from the Socorro interface and is 
report bp-7d9bd778-d336-43a6-9e0a-ff93e2131105.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.home.TopSitesPage$EditPinnedSiteListener.onSiteSelected(TopSitesPage.java:403)
	at org.mozilla.gecko.home.PinSiteDialog$2.onKey(PinSiteDialog.java:115)
	at android.view.View.dispatchKeyEvent(View.java:7085)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1361)
(Reporter)

Updated

5 years ago
Version: Firefox 26 → Firefox 28

Comment 1

5 years ago
Regression from bug 925082?
(Assignee)

Comment 2

5 years ago
(In reply to :Margaret Leibovic from comment #1)
> Regression from bug 925082?

Doesn't seem so. From the looks of it, it seems the NPE is caused by a null mOnSiteSelectedListener or maybe a null activity once onSiteSelected is called. The lines to exactly match the code anymore.
(Reporter)

Comment 3

4 years ago
This is dominating crash-stats on 28.0b1 as far as I can tell
(Assignee)

Updated

4 years ago
Assignee: nobody → lucasr.at.mozilla
tracking-fennec: --- → ?
(Assignee)

Comment 4

4 years ago
Created attachment 8374833 [details] [diff] [review]
PinSiteDialog fragment should be owned by TopSitesPanel (r=mfinkle)
(Assignee)

Comment 5

4 years ago
Created attachment 8374834 [details] [diff] [review]
Unset site selection listener when PinSiteDialog gets destroyed (r=mfinkle)
(Assignee)

Comment 6

4 years ago
Comment on attachment 8374833 [details] [diff] [review]
PinSiteDialog fragment should be owned by TopSitesPanel (r=mfinkle)

One the issues here is that the dialog fragment is owned by the activity which means the TopSitesPanel might get detached from the activity and the dialog could still stick around (enough time to catch a key event and trigger the OnSiteSelectedListener).

This patch corrects this by managing PinSiteDialog in the TopSitePanel's fragment manager.
Attachment #8374833 - Flags: review?(mark.finkle)
(Assignee)

Comment 7

4 years ago
Comment on attachment 8374834 [details] [diff] [review]
Unset site selection listener when PinSiteDialog gets destroyed (r=mfinkle)

Extra safety measure. Ensure that the OnSiteSelectedListener will not be triggered after the PinSiteDialog is destroyed.

FYI: this bug is among the top crashers across all our channels.
Attachment #8374834 - Flags: review?(mark.finkle)
(Assignee)

Updated

4 years ago
Blocks: 960202
Comment on attachment 8374833 [details] [diff] [review]
PinSiteDialog fragment should be owned by TopSitesPanel (r=mfinkle)

I had to read-up on getChildFragmentManager. Looks like the right approach.
Attachment #8374833 - Flags: review?(mark.finkle) → review+
Attachment #8374834 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 10

4 years ago
Comment on attachment 8374833 [details] [diff] [review]
PinSiteDialog fragment should be owned by TopSitesPanel (r=mfinkle)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Probably the new about:home stuff released in Fx26. 
User impact if declined: One of top crashers in Fx28 right now. 
Testing completed (on m-c, etc.): No easy way to reproduce it so hard to tell if this is actually going to fix the bug. This is a very educated guess though. Let's land this in nightly, and gradually land in aurora and then beta to see how crashstats looks. 
Risk to taking this patch (and alternatives if risky): Fairly low, small patches with no functional change. 
String or IDL/UUID changes made by this patch: n/a
Attachment #8374833 - Flags: approval-mozilla-beta?
Attachment #8374833 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 11

4 years ago
Comment on attachment 8374834 [details] [diff] [review]
Unset site selection listener when PinSiteDialog gets destroyed (r=mfinkle)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Probably the new about:home stuff released in Fx26. 
User impact if declined: One of top crashers in Fx28 right now. 
Testing completed (on m-c, etc.): No easy way to reproduce it so hard to tell if this is actually going to fix the bug. This is a very educated guess though. Let's land this in nightly, and gradually land in aurora and then beta to see how crashstats looks. 
Risk to taking this patch (and alternatives if risky): Fairly low, small patches with no functional change. 
String or IDL/UUID changes made by this patch: n/a
Attachment #8374834 - Flags: approval-mozilla-beta?
Attachment #8374834 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/fbd6ad52ace7
https://hg.mozilla.org/mozilla-central/rev/6e3808d2f605
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
tracking-fennec: ? → 28+
Comment on attachment 8374833 [details] [diff] [review]
PinSiteDialog fragment should be owned by TopSitesPanel (r=mfinkle)

To get the crash data we should uplift this to branches now in order to have it in 28.0b4 mobile next week and gathering feedback with time to backout or make any tweaks if needed before ship.
Attachment #8374833 - Flags: approval-mozilla-beta?
Attachment #8374833 - Flags: approval-mozilla-beta+
Attachment #8374833 - Flags: approval-mozilla-aurora?
Attachment #8374833 - Flags: approval-mozilla-aurora+
Attachment #8374834 - Flags: approval-mozilla-beta?
Attachment #8374834 - Flags: approval-mozilla-beta+
Attachment #8374834 - Flags: approval-mozilla-aurora?
Attachment #8374834 - Flags: approval-mozilla-aurora+
status-b2g-v1.3: --- → fixed
This is reproducible starting from V27.

Here are the reproduce steps:

1. Launch Firefox
2. Clear all user data, so there is some unpinned icon on Topsites page
3. Tap on support.mozilla.org and at the same time, quickly tap on "+" to add a site to pin
   You will notice that page 'support.mozilla.org' opens at background while website picker appears on the top layer.
4. Tap on any websites to Pin.
   --> Firefox crashes.

I took a video for the STR: https://mozilla.box.com/s/qjww078t5qrn5epbl5sm
Keywords: steps-wanted
BTW, after this fix was landed, I could not reproduce this crash with the same steps on V28 beta6 and latest nightly.
Verified as fixed in builds:
28.0
29.0
30.0
Latest Nightly 33.0a1 (2014-06-10)

Devices: 
Asus Transformer Pad TF300T (Android 4.2.1)
Motorola Razr (Android 4.0.4)
status-firefox28: fixed → verified
status-firefox29: fixed → verified
status-firefox30: fixed → verified
You need to log in before you can comment on or make changes to this bug.