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
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)
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+
Target Milestone: --- → Firefox 15
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.
There is one crash in 15.0a1/20120512: bp-3d300374-a0d8-47e9-8e39-b5ca22120513.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago → 7 years ago
Resolution: --- → FIXED
status-firefox14: --- → unaffected
status-firefox15: --- → fixed
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.