Closed Bug 752172 Opened 10 years ago Closed 10 years ago

java.lang.NullPointerException: at org.mozilla.gecko.SiteIdentityPopup.show(SiteIdentityPopup.java)

Categories

(Firefox for Android Graveyard :: General, defect)

15 Branch
ARM
Android
defect
Not set
critical

Tracking

(firefox14 unaffected, firefox15 verified, firefox16 verified, firefox17 verified)

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- unaffected
firefox15 --- verified
firefox16 --- verified
firefox17 --- verified

People

(Reporter: scoobidiver, Assigned: Margaret)

References

Details

(Keywords: crash, regression, Whiteboard: [native-crash])

Crash Data

Attachments

(2 files)

There are two crashes from two different users, the first one in 15.0a1/20120504040218 and the second one in 15.0a1/20120504122939: bp-79fa4f9f-b145-4380-88bd-1f1ee2120505.

java.lang.NullPointerException
	at org.mozilla.gecko.SiteIdentityPopup.show(SiteIdentityPopup.java:72)
	at org.mozilla.gecko.BrowserToolbar$4.onClick(BrowserToolbar.java:184)
	at android.view.View.performClick(View.java:3511)
	at android.view.View$PerformClick.run(View.java:14105)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4424)
	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:787)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:554)
	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.SiteIdentityPopup.show%28SiteIdentityPopup.java%29
Introduced with bug 695204.
Blocks: 695204
Keywords: regression
Assignee: nobody → margaret.leibovic
Instead of needing to worry about whether or not mSiteIdentityPopup has been initialized, I decided it would be nicer to just get rid of it, and turn SiteIdentityPopup into a singleton class.

I really wanted to totally decouple it from GeckoApp, but unfortunately we still need to refer to GeckoApp.mAppContext, and we also already refer to GeckoApp.mBrowserToolbar in there.

If you like this, I think we could do similar things with DoorHangerPopup and FormAssistPopup. I also noticed FormAssistPopup was declared in gecko_app.xml, and I think that should just go in its own xml like the other popups. I can do this cleanup/consistency work in another bug, though.
Attachment #621758 - Flags: review?(mark.finkle)
Comment on attachment 621758 [details] [diff] [review]
Turn SiteIdentityPopup into a singleton class

Switching the review request to spread around review load a little more :)
Attachment #621758 - Flags: review?(mark.finkle) → review?(mbrubeck)
Attachment #621758 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4940a331b147
Target Milestone: --- → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/4940a331b147
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Margaret Leibovic [:margaret] from comment #2)

> If you like this, I think we could do similar things with DoorHangerPopup
> and FormAssistPopup. I also noticed FormAssistPopup was declared in
> gecko_app.xml, and I think that should just go in its own xml like the other
> popups. I can do this cleanup/consistency work in another bug, though.

We intend to start using different activities for the Browser and WebApps, so we need to make sure these classes can be used by multiple activities. Creating singletons might not allow for that very easily.
There is one crash in 15.0a1/20120512: bp-3d300374-a0d8-47e9-8e39-b5ca22120513.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Looks like I read the crash stack incorrectly last time. My last patch fixed a potential crash, but not this one.

Looking at the Tabs code, it seems like Tabs.getInstance.getSelectedTab() should really only return null if selectTab() has never been called yet. However, we only make the lock icon visible if there is a valid selected tab, since we use that tab's data to decide to show the lock. Looking at the lock code, we don't actually hide it, but rather set it to be transparent, so I'd guess this crash is just happening if a user accidentally taps on that transparent lock before a tab is selected. If that's the case, bailing if the selected tab is null shouldn't be noticeable to the user. However, I think it would also be worth looking into hiding the lock instead of making it transparent, so that we can never trigger this click listener in that case.
Attachment #623689 - Flags: review?(mark.finkle)
Comment on attachment 623689 [details] [diff] [review]
(Part 2) Protect against null selected tab

Nice explanation
Attachment #623689 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/d0f6caf941d1
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.