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

VERIFIED FIXED in Firefox 15

Status

()

--
critical
VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: scoobidiver, Assigned: Margaret)

Tracking

({crash, regression})

15 Branch
Firefox 15
ARM
Android
crash, regression
Points:
---

Firefox Tracking Flags

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

Details

(Whiteboard: [native-crash], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
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
(Reporter)

Updated

7 years ago
Keywords: regression
(Assignee)

Updated

7 years ago
Assignee: nobody → margaret.leibovic
(Assignee)

Comment 2

7 years ago
Created attachment 621758 [details] [diff] [review]
Turn SiteIdentityPopup into a singleton class

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)
(Assignee)

Comment 3

7 years ago
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+
(Assignee)

Comment 4

7 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4940a331b147
Target Milestone: --- → Firefox 15

Comment 5

7 years ago
https://hg.mozilla.org/mozilla-central/rev/4940a331b147
Status: NEW → RESOLVED
Last Resolved: 7 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.
(Reporter)

Comment 7

7 years ago
There is one crash in 15.0a1/20120512: bp-3d300374-a0d8-47e9-8e39-b5ca22120513.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

7 years ago
Created attachment 623689 [details] [diff] [review]
(Part 2) Protect against null selected tab

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
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
(Reporter)

Updated

7 years ago
status-firefox14: --- → unaffected
status-firefox15: --- → fixed

Updated

6 years ago
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
status-firefox16: --- → verified
status-firefox17: --- → verified
You need to log in before you can comment on or make changes to this bug.