Closed Bug 926574 Opened 7 years ago Closed 7 years ago
ANR: SQLite access on main thread when using Cursor
I'm not familiar with the code, but it looks like we are using the deprecated CursorAdapter constructor  in our code which "results in Cursor queries being performed on the application's UI thread and thus can cause poor responsiveness or even Application Not Responding errors". And we do see a number of these ANRs in the wild .  http://developer.android.com/reference/android/widget/CursorAdapter.html#CursorAdapter%28android.content.Context,%20android.database.Cursor%29  http://darchons.github.io/anr-analyzer/?files=c/cb/Anr-20130903.jgz,3/3f/Anr-20130910.jgz,b/bd/Anr-20130917.jgz,d/df/Anr-20130924.jgz#f:anr-20130924,l:28 (requires intranet access)
Look slike all of our HomePage Adapters use this constructor, even the MultiTypeCursorAdapter: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/MultiTypeCursorAdapter.java#28 ReadingListAdapter: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/ReadingListPage.java#200 http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/ReadingListPage.java#139 BookmarksListAdapter: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BookmarksListAdapter.java#43 http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BookmarksPage.java#80 Lucas, Margaret, Sriram: Any reason we are using this constructor over the "recommended" constructor: http://developer.android.com/reference/android/widget/CursorAdapter.html#CursorAdapter(android.content.Context,%20android.database.Cursor,%20int) The docs also talk about using a LoaderManager and CursorLoader, but I see we are using LoadrManagers and CursorLoaders. Do we just need to switch CursorAdapter constructors?
Duplicate of this bug: 867251
I guess we could use the third constructor. The first one was used because we weren't using support v4 library's CursorAdapter earlier. Now we've started using the support v4's CursorAdapter and won't have to worry about API 11+. (Remember the issue with swapCursor() and not available in old phones (11+) and we reverting everything to use support library's? We shall go with the third option now safely). We should register a content observer though, with the flag: FLAG_REGISTER_CONTENT_OBSERVER I'll post a patch today.
(In reply to Sriram Ramasubramanian [:sriram] from comment #3) > We should register a content observer though, with the flag: > FLAG_REGISTER_CONTENT_OBSERVER I saw this in the Javadocs: "This flag is not needed when using a CursorAdapter with a CursorLoader." So maybe we could just pass in 0 when we are using a cursor loader.
I felt the same. We are having a observer in our SimpleCursorLoader.
Comment on attachment 816775 [details] [diff] [review] Patch This is what I would have assumed we'd need to do, but I want more eyes. Handing to Lucas.
Attachment #816775 - Flags: review?(mark.finkle) → review?(lucasr.at.mozilla)
Looks like Sriram has this under control. Clearing needinfo.
(In reply to Mark Finkle (:mfinkle) from comment #4) > (In reply to Sriram Ramasubramanian [:sriram] from comment #3) > > > We should register a content observer though, with the flag: > > FLAG_REGISTER_CONTENT_OBSERVER > > I saw this in the Javadocs: > "This flag is not needed when using a CursorAdapter with a CursorLoader." > > So maybe we could just pass in 0 when we are using a cursor loader. Yep, our custom cursor loader (SimpleCursorLoader) registers a content observer and forces re-load as appropriate.
Comment on attachment 816775 [details] [diff] [review] Patch Review of attachment 816775 [details] [diff] [review]: ----------------------------------------------------------------- Yep.
Attachment #816775 - Flags: review?(lucasr.at.mozilla) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
The patch in this bug appears to have solved the issues in bug 919234, which is tracking 26. Since a patch there would likely have been uplifted to 26, I would +1 this nomination.
Sriram - Please request approval for uplift
tracking-fennec: ? → 26+
Comment on attachment 816775 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Using old version of the constructor. User impact if declined: ANRs randomly. Testing completed (on m-c, etc.): 10/16 Risk to taking this patch (and alternatives if risky): None. String or IDL/UUID changes made by this patch: None.
Attachment #816775 - Flags: approval-mozilla-aurora?
Attachment #816775 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.