Closed Bug 774209 Opened 13 years ago Closed 13 years ago

java.lang.NullPointerException: at android.widget.PopupWindow$PopupViewContainer.dispatchKeyEvent(PopupWindow.java) on Gingerbread

Categories

(Firefox for Android Graveyard :: General, defect)

15 Branch
ARM
Android
defect
Not set
critical

Tracking

(firefox15+ affected, firefox16+ fixed, firefox17 fixed)

RESOLVED FIXED
Firefox 18
Tracking Status
firefox15 + affected
firefox16 + fixed
firefox17 --- fixed

People

(Reporter: scoobidiver, Assigned: Margaret)

References

Details

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

Crash Data

Attachments

(1 file)

It first appeared in 15.0a2/20120714. The Aurora regression range is: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=7140e39a6427&tochange=50963e16d1dc It might be a regression from bug 764405 or bug 769001. java.lang.NullPointerException at android.widget.PopupWindow$PopupViewContainer.dispatchKeyEvent(PopupWindow.java:1428) at android.view.ViewRoot.deliverKeyEventToViewHierarchy(ViewRoot.java:2560) at android.view.ViewRoot.deliverKeyEvent(ViewRoot.java:2525) at android.view.ViewRoot.handleMessage(ViewRoot.java:1873) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:130) at android.app.ActivityThread.main(ActivityThread.java:3683) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:507) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597) at dalvik.system.NativeStart.main(Native Method) More reports at: https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+android.widget.PopupWindow%24PopupViewContainer.dispatchKeyEvent%28PopupWindow.java%29
It's #35 top crasher in 15.0b1. The trunk is also affected.
Summary: java.lang.NullPointerException: at android.widget.PopupWindow$PopupViewContainer.dispatchKeyEvent(PopupWindow.java) → java.lang.NullPointerException: at android.widget.PopupWindow$PopupViewContainer.dispatchKeyEvent(PopupWindow.java) on Gingerbread
Sriram, does this crash look like fallout from your fix for bug 762727?
It's #2 top crasher in the first hours of 15.0.
Whiteboard: [native-crash] → [native-crash][startupcrash]
Curiously, of the ~480 PopupWindow crashes from the last four weeks, ~475 are on Gingerbread and ~5 on Froyo. I don't see any ICS or JB crashes.
Keywords: topcrash
Will track for 15 and watch for any user comments or increased volume, though at this time it doesn't appear to be a chemspill-worthy issue.
Is there any STR to reproduce this? I tried installing Firefox (release) on two gingerbread phones and neither of them crashed. Hypothesis: Probably APK could get corrupted while downloading -- resulting in CPP crash. Argument: If it's a startup crash on Gingerbread, then it should be a startup crash on all Gingerbread phones. Pointers: mozglue/android/APKOpen.cpp:299 ?? memory/mozalloc/mozalloc_abort.cpp:89 ??
(In reply to Sriram Ramasubramanian [:sriram] from comment #6) > Argument: If it's a startup crash on Gingerbread, then it should be a > startup crash on all Gingerbread phones. > > Pointers: mozglue/android/APKOpen.cpp:299 ?? > memory/mozalloc/mozalloc_abort.cpp:89 ?? This crash is an uncaught Java exception. mozalloc_abort() is in the C stack trace because that is how Java_org_mozilla_gecko_GeckoAppShell_reportJavaCrash() triggers the Breakpad crash reporting.
If the regression range in comment 0 is correct, could this crash be is a regression from Margaret's door hanger changes? Those are the only changesets that touched PopupWindow code. This crash looks like a race condition when a key event is delivered to a PopupWindow that has already been unloaded. Of the 1100+ crash reports from the last 4 weeks, ~1080+ crashed in less than 1000 ms. But I have no idea why a key event would be dispatched so quickly after launch...
Chris - since you're leading the investigation so far, assigning this #6 top FF15 crasher to you. Are there any speculative fixes we could consider making on Aurora/Beta ahead of the next release?
Assignee: nobody → cpeterson
Sriram or Margaret would probably be the best people to investigate. I don't know the UI code very well or what caused this regression.
Assignee: cpeterson → nobody
Assigning to Margaret then for further investigation since we don't want to have a bug tracked for FF16 unassigned.
Assignee: nobody → margaret.leibovic
I searched the internet and found that other app developers have come across this crash, but it doesn't look like anyone has really figured out the root cause, other than recognizing that there's some Android bug here. Some answers mentioned that problems can happen if you give the PopupWindow focus before showing it, or setting a background on the popup before showing it, as problems can occur if the user hits the back button before the popup is actually shown. So maybe our problem has to do with doing things to the PopupWindow before we show it? The fact that there's such a high volume of these crashes, and that some happen very shortly after first launch makes me suspect the problem is with DoorHangerPopup, not SiteIdentityPopup. Looking at the bugs in the regression range, I'm having a tough time figuring out what exactly we could have done wrong to introduce this issue, especially since a lot of the changes should only affect tablets, and we're seeing these crashes on Gingerbread/Froyo. I thought maybe making the change to include an offset (even if it's 0) in showAsDropDown could cause a problem, but looking at the android source code, showAsDropDown(v) just calls showAsDropDow(v, 0, 0). Are we certain that this regression range is correct?
(In reply to Margaret Leibovic [:margaret] from comment #12) > Are we certain that this regression range is correct? The earliest crash report I can find is Aurora 15.0a2 2012-07-14 (like Scoobidriver saw in comment 1): 5e956335-4acf-4797-80f7-a004a2120715 I don't know if comment 1's changeset regression range is correct. You would need to double-check the about:buildconfig revisions for Aurora builds 7-13 and 7-14.
(In reply to Margaret Leibovic [:margaret] from comment #12) > Are we certain that this regression range is correct? My bad. I was wrong. It is shifted by one day. The right one is: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=88382a387a66&tochange=7140e39a6427
Aha, I bet it was caused by http://hg.mozilla.org/releases/mozilla-aurora/rev/caca38771162. Eitan, can we delay calling setFocusable(true) until after we show the popup?
Blocks: 773085
(In reply to Margaret Leibovic [:margaret] from comment #15) > Aha, I bet it was caused by > http://hg.mozilla.org/releases/mozilla-aurora/rev/caca38771162. > > Eitan, can we delay calling setFocusable(true) until after we show the popup? I'm pretty sure, yes.
Hm, thinking about this more, it appears the problem is that popup is getting focus before it's showing, which means that the fact that we're calling setFocusable(true) at all is giving us grief. Maybe we need to do some hack where we call setFocusable(true) after we show the popup then setFocusable(false) when we hide it.
Attached patch workaround patchSplinter Review
Eitan, does this maintain the functionality you added in bug 773085? I'm not certain that this would fix the crash, but it feels like a low-risk approach we could land to see if it works.
Attachment #658625 - Flags: feedback?(eitan)
Comment on attachment 658625 [details] [diff] [review] workaround patch Yeah, looks OK. I can't test it right now. If you had a device with a keyboard or bluetooth keyboard you could. It is a strange fix, but if it does the trick...
Attachment #658625 - Flags: feedback?(eitan) → feedback+
Comment on attachment 658625 [details] [diff] [review] workaround patch (In reply to Eitan Isaacson [:eeejay] from comment #19) > Yeah, looks OK. I can't test it right now. If you had a device with a > keyboard or bluetooth keyboard you could. It is a strange fix, but if it > does the trick... I don't have a device with a keyboard or bluetooth keyboard, and I'm traveling. Maybe Marco can test to confirm this doesn't break accessibility. I know this is a strange fix, but we just need to do something to work around this Android bug on older versions of Android. I'm asking Sriram to review, and I can try landing this to see if it actually fixes the crash. Fingers crossed.
Attachment #658625 - Flags: review?(sriram)
Attachment #658625 - Flags: feedback?(marco.zehe)
Comment on attachment 658625 [details] [diff] [review] workaround patch Review of attachment 658625 [details] [diff] [review]: ----------------------------------------------------------------- The logic looks good to me.
Attachment #658625 - Flags: review?(sriram) → review+
Comment on attachment 658625 [details] [diff] [review] workaround patch This works fine, and alerts such as the one asking whether one wants to install and open the Market Place are still accessible via the keyboard. f=me.
Attachment #658625 - Flags: feedback?(marco.zehe) → feedback+
Target Milestone: --- → Firefox 18
Comment on attachment 658625 [details] [diff] [review] workaround patch This is a speculative crash fix, but it's low-risk enough that I think we can uplift it sooner rather than later to see if it works. I only see one crash on Nightly in the last week, so we may need to uplift to see if it actually works. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 773085 User impact if declined: crashes when doorhangers appear Testing completed (on m-c, etc.): just landed on inbound Risk to taking this patch (and alternatives if risky): low-risk, we verified this doesn't break the accessibility change from bug 773085 String or UUID changes made by this patch: n/a
Attachment #658625 - Flags: approval-mozilla-beta?
Attachment #658625 - Flags: approval-mozilla-aurora?
Comment on attachment 658625 [details] [diff] [review] workaround patch early enough that we can back this out if we have to, but it would be good to get more data via uplifting, approving
Attachment #658625 - Flags: approval-mozilla-beta?
Attachment #658625 - Flags: approval-mozilla-beta+
Attachment #658625 - Flags: approval-mozilla-aurora?
Attachment #658625 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/19e15ec05b04 https://hg.mozilla.org/releases/mozilla-beta/rev/512c15654507 I also just realized that I should add a setFocusable(true) call after we call showAtLocation in the no anchor case (the way we show these doorhangers for web apps). I'll file a follow-up for that. I'm leaving the status flags as affected for now until we learn if this has worked.
It's #19 top crasher in 15.0 and #82 in 16.0b1.
Keywords: topcrash
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
(In reply to Margaret Leibovic [:margaret] from comment #26) > I'm leaving the status flags as affected for now until we learn if this has > worked. The verified status is for that.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: