Closed
Bug 1265908
Opened 8 years ago
Closed 8 years ago
Potential NullPointerException while querying BrowserProvider for BOOKMARKS
Categories
(Firefox for Android Graveyard :: Data Providers, defect)
Firefox for Android Graveyard
Data Providers
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: Grisha, Unassigned)
Details
Attachments
(1 file)
If BrowserProvider was queried for BOOKMARKS with the following being true: - selection=null - shouldShowDeleted(uri) === true Then we will crash here: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java#1007 This change was introduced here: https://hg.mozilla.org/mozilla-central/rev/d5064e39467c An actual crash log from today: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.lang.String.contains(java.lang.CharSequence)' on a null object reference at org.mozilla.gecko.db.BrowserProvider.query(BrowserProvider.java:1054) at android.content.ContentProvider.query(ContentProvider.java:1017) at android.content.ContentProvider$Transport.query(ContentProvider.java:238) at android.content.ContentResolver.query(ContentResolver.java:491) at android.content.ContentResolver.query(ContentResolver.java:434) at org.mozilla.gecko.sync.repositories.android.RepoUtils$QueryHelper.safeQuery(RepoUtils.java:51) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositoryDataAccessor.fetchAll(AndroidBrowserRepositoryDataAccessor.java:124) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositorySession.createRecordToGuidMap(AndroidBrowserRepositorySession.java:681) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositorySession.putRecordToGuidMap(AndroidBrowserRepositorySession.java:747) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositorySession.updateBookkeeping(AndroidBrowserRepositorySession.java:757) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserBookmarksRepositorySession.updateBookkeeping(AndroidBrowserBookmarksRepositorySession.java:825) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositorySession.replace(AndroidBrowserRepositorySession.java:575) at org.mozilla.gecko.sync.repositories.android.AndroidBrowserRepositorySession$1.run(AndroidBrowserRepositorySession.java:514) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588) at java.lang.Thread.run(Thread.java:818)https://pastebin.mozilla.org/8868293
Reporter | ||
Comment 1•8 years ago
|
||
We should be covering CP code with junit4 tests, to avoid issues like this. Example of what BrowserProvider tests might look like (in that case, for history and visits): https://reviewboard.mozilla.org/r/40955/diff/13#3
Comment 2•8 years ago
|
||
It is possible and valid to have a null selection. All other manipulations are null-safe, and we need to be able to handle the null-case when testing for annotations being part of the selection. Review commit: https://reviewboard.mozilla.org/r/47611/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47611/
Attachment #8743157 -
Flags: review?(gkruglov)
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8743157 [details] MozReview Request: Bug 1265908 - Avoid NPE with empty selection r?grisha https://reviewboard.mozilla.org/r/47611/#review44521
Attachment #8743157 -
Flags: review?(gkruglov) → review+
Comment 4•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/59ce733181109d13ec748b94d2069ebe699be33b Bug 1265908 - Avoid NPE with empty selection r=grisha
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/59ce73318110
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•