Closed Bug 828480 Opened 7 years ago Closed 7 years ago
.lang .Null Pointer Exception: at org .mozilla .gecko .All Pages Tab .set Suggestions Enabled(All Pages Tab .java)
This bug tracks crashes not fixed by bug 816902. Here is a crash report :bp-a3b1db88-e2be-452d-8667-0e8cf2130109. java.lang.NullPointerException at org.mozilla.gecko.AllPagesTab.setSuggestionsEnabled(AllPagesTab.java:606) at org.mozilla.gecko.AllPagesTab.access$900(AllPagesTab.java:53) at org.mozilla.gecko.AllPagesTab$5.onClick(AllPagesTab.java:589) at android.view.View.performClick(View.java:4198) at android.view.View$PerformClick.run(View.java:17158) at android.os.Handler.handleCallback(Handler.java:615) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:4918) 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:1004) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:771) 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.AllPagesTab.setSuggestionsEnabled%28AllPagesTab.java%29
Bug 816902 immediately removed these listeners to prevent them from being triggered multiple times here: http://hg.mozilla.org/releases/mozilla-release/file/7e5736897abb/mobile/android/base/awesomebar/AllPagesTab.java#l585 The only place we set mSuggestionsPrompt to null is here: http://hg.mozilla.org/releases/mozilla-release/file/7e5736897abb/mobile/android/base/awesomebar/AllPagesTab.java#l633 That means setSuggestionsEnabled() was already called prior to the NPE, which means that multiple onClick() calls are still possible with the current implementation. I assume the click event isn't synchronously dispatched, so users could still queue up multiple click events before onClick() is first called. I found this same issue described here: http://stackoverflow.com/questions/10848157/android-prevent-multiple-onclick-events-on-a-button-that-has-been-disabled An easy fix would be to add a null check for mSuggestionsPrompt to the beginning of setSuggestionsEnabled(), but I'd be concerned that the Runnable at http://hg.mozilla.org/releases/mozilla-release/file/7e5736897abb/mobile/android/base/awesomebar/AllPagesTab.java#l629 could potentially be executed after the second call to setSuggestionsEnabled() has already begun (which would cause a NPE for mSuggestionsPrompt somewhere later in the method). A fix that should work would be to create a class-level boolean that's toggled on the first call, but I hate creating a boolean for something so trivial...
This just adds a null check to the beginning of setSuggestionsEnabled(). We can look into other fixes if we still have crashes as speculated in comment 1.
Attachment #702055 - Flags: review?(mark.finkle)
Attachment #702055 - Flags: review?(mark.finkle) → review+
Comment on attachment 702055 [details] [diff] [review] Add null check for mSuggestionsOptInPrompt [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 769145 User impact if declined: crashes if suggestions prompt response is clicked repeatedly Testing completed (on m-c, etc.): just landed m-i Risk to taking this patch (and alternatives if risky): very low risk - just adds a null pointer check String or UUID changes made by this patch: none
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Comment on attachment 702055 [details] [diff] [review] Add null check for mSuggestionsOptInPrompt Mobile only null check, good for b3 and Aurora.
(Also, this is in my queue to land on Aurora once it reopens)
You need to log in before you can comment on or make changes to this bug.