1.88 KB, patch
|Details | Diff | Splinter Review|
3.88 KB, patch
|Details | Diff | Splinter Review|
There's one crash in 24.0a1/20130604: bp-9137613a-f142-4bc8-9ef9-e93712130605. java.lang.NullPointerException at org.mozilla.gecko.background.healthreport.EnvironmentBuilder.getStorage(EnvironmentBuilder.java:34) at org.mozilla.gecko.health.BrowserHealthRecorder.<init>(BrowserHealthRecorder.java:106) at org.mozilla.gecko.GeckoApp$14.run(GeckoApp.java:1218) at android.os.Handler.handleCallback(Handler.java:605) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:154) at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:32) More reports at: https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.background.healthreport.EnvironmentBuilder.getStorage%28EnvironmentBuilder.java%29
This sounds like the equivalent of that npe fetching the passwords provider. The one that doesn't make sense.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Priority: -- → P1
This exception means that acquireContentProviderClient returned null for our authority. That should never happen; if it does, it should happen for *every* install of that APK. This is exactly the same problem as Bug 745837. And hey, check out the crap in the crash report: https://crash-stats.mozilla.com/report/index/9137613a-f142-4bc8-9ef9-e93712130605 Running Android 0.0.0 on an HTC HTC Desire S. Right. Custom ROM that doesn't work right? Disabled permissions? We can add a guard around this, but that's the point of defensive coding where you simply don't believe that any API call will work at all. Android has driven us here.
Also, I'm hesitant to 'fix' this, because the fix will cause us to have no errors reported, instead silently failing to init FHR. Let's leave this for a little while, see if it was an isolated incident.
The crash report seems valid. There are a lot of crashes reported with "HTC HTC Desire S" as the device and 0.0.0 as the Linux OS version. The Android version appears to actually be 4.0.4. The crash happens at http://hg.mozilla.org/mozilla-central/file/7e3a4ebcf067/mobile/android/base/background/healthreport/EnvironmentBuilder.java#l34 which means "pr" (returned from getLocalContentProvider() just above) is null. This seems to be allowed by the javadoc for getLocalContentProvider(), although I guess it might violate the expectations in that area of the code, I'm not sure.
You're right; I misread the line number. Same kind of invariant failing, though: it means there's no CP running in the same thread for that authority, which should not be the case. Poking around in the Android source now.
I've been turning this over in my head. The only way I can see this happening is if the CP is first instantiated in a different process to the crashing GeckoApp's thread. Android ensures that there's only one of the CP, and it lives in our main process (because we don't specify otherwise), regardless of who spawned it. So this implies a new process has instantiated a BrowserHealthRecorder or GeckoApp. Is that possible? Can an app launch a GeckoApp activity in its own process in current Nightly? Either that, or this is a weird Android bug. (Also to investigate: perhaps the passwords process is alive, and somehow claimed our CP?) I figure Brad might know the answer to that last question...
Band-aid patch for this yields the following behavior: I/GeckoApp( 5958): Creating BrowserHealthRecorder. D/GeckoHealthRec( 5958): Initializing. Dispatcher is org.mozilla.gecko.util.EventDispatcher@42862170 E/GeckoLogger( 5958): fennec_rnewman :: GeckoEnvBuilder :: Unable to retrieve local content provider. Running in a different process? D/GeckoHealthRec( 5958): Initializing profile cache. I/GeckoLogger( 5958): fennec_rnewman :: GeckoProfileInfo :: Restoring ProfileInformationCache from file. D/GeckoHealthRec( 5958): Successfully restored state. Initializing storage. D/GeckoHealthRec( 5958): Done initializing profile cache. Beginning storage init. W/GeckoHealthRec( 5958): Storage is null during init; shutting down? D/GeckoSessInfo( 5958): Recording start of session: 1370567957195 ... D/GeckoHealthRec( 5958): Recording session end: P E/GeckoHealthRec( 5958): Attempted to record session end without initialized recorder. I/GeckoHealthRec( 7440): Closing incompletely initialized BrowserHealthRecorder. W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Addons:All' has no listeners W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Addons:Change' has no listeners W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Pref:Change' has no listeners W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Search:Keyword' has no listeners W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Search:Event' has no listeners That is: we'll quietly log a few failures, and otherwise won't interfere. I'll prep this for landing, but I want to see if we get any more crashes, so I won't land it just yet.
(In reply to Richard Newman [:rnewman] from comment #6) > Android ensures that there's only one of the CP, and it lives in our main > process (because we don't specify otherwise), regardless of who spawned it. > So this implies a new process has instantiated a BrowserHealthRecorder or > GeckoApp. > > Is that possible? Can an app launch a GeckoApp activity in its own process > in current Nightly? The only thing that comes to my mind is WebApps. They do run in a different process than the Firefox browser. But I don't think this is what you are referring to. I can't think of any other processes instantiating GeckoApp.
Attachment #759535 - Flags: review?(nalexander) → review+
Attachment #759533 - Flags: review?(nalexander) → review+
Whiteboard: [native-crash][startupcrash] → [native-crash][startupcrash][fixed in services][qa-]
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [native-crash][startupcrash][fixed in services][qa-] → [native-crash][startupcrash][qa-]
Target Milestone: --- → Firefox 24
Comment on attachment 759533 [details] [diff] [review] Part 1: return null if unable to retrieve storage in EnvironmentBuilder. (git) [Approval Request Comment] Bug caused by (feature/regressing bug #): Affects code in Bug 868449, but caused by... Android. User impact if declined: One startup crash on Nightly since all this landed; it's possible there could be more. Testing completed (on m-c, etc.): Bake time. Risk to taking this patch (and alternatives if risky): None. String or IDL/UUID changes made by this patch: None.
Attachment #759533 - Flags: approval-mozilla-aurora?
Attachment #759535 - Flags: approval-mozilla-aurora?
Comment on attachment 759533 [details] [diff] [review] Part 1: return null if unable to retrieve storage in EnvironmentBuilder. (git) Approving as part of the body of work needed for FHR on FF23 Android.
Attachment #759533 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #759535 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You beat me to it! m-cMerge doesn't set status flags unless a bug is tracking.
You need to log in before you can comment on or make changes to this bug.