Closed
Bug 898219
Opened 11 years ago
Closed 11 years ago
NPE at BrowserSearch$SearchAdapter.getCount
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 26
People
(Reporter: bnicholson, Assigned: bnicholson)
References
Details
(Whiteboard: fixed-fig)
Attachments
(1 file)
3.47 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
From https://tbpl.mozilla.org/php/getParsedLog.php?id=25742166&tree=Fig&full=1
07-25 16:33:50.943 E/GeckoAppShell( 1789): java.lang.NullPointerException
07-25 16:33:50.943 E/GeckoAppShell( 1789): at org.mozilla.gecko.home.BrowserSearch$SearchAdapter.getCount(BrowserSearch.java:644)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.widget.AdapterView.checkFocus(AdapterView.java:689)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.widget.AdapterView$AdapterDataSetObserver.onInvalidated(AdapterView.java:813)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.database.DataSetObservable.notifyInvalidated(DataSetObservable.java:43)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.widget.BaseAdapter.notifyDataSetInvalidated(BaseAdapter.java:54)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.support.v4.widget.CursorAdapter.swapCursor(CursorAdapter.java:352)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.support.v4.widget.SimpleCursorAdapter.swapCursor(SimpleCursorAdapter.java:326)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at org.mozilla.gecko.home.BrowserSearch$CursorLoaderCallbacks.onLoaderReset(BrowserSearch.java:762)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.support.v4.app.LoaderManagerImpl$LoaderInfo.destroy(LoaderManager.java:339)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.support.v4.app.LoaderManagerImpl.doDestroy(LoaderManager.java:776)
07-25 16:33:50.943 E/GeckoAppShell( 1789): at android.support.v4.app.Fragment.onDestroy(Fragment.java:1140)
Assignee | ||
Comment 1•11 years ago
|
||
onDestroy() triggers onLoaderReset(), which calls mAdapter.swapCursor(null), which apparently ends up using getCount() in the adapter. Conceptually, I think mSearchEngines should be set/cleared in onCreate()/onDestroy(), but we do it now in onViewCreated() only because we need to ensure the view exists before the prompt is created. We can still create the initial array in onCreate(), though, so I think it makes sense to do so. The adapter relies on its existence, so it should fix this NPE if we wait until onDestroy() to clear it. This also means we shouldn't need the null check in setSuggestions(), which I didn't like having.
Attachment #781380 -
Flags: review?(lucasr.at.mozilla)
Comment 2•11 years ago
|
||
Comment on attachment 781380 [details] [diff] [review]
Don't clear reference to mSearchEngines until onDestroy()
Review of attachment 781380 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/home/BrowserSearch.java
@@ +343,5 @@
> getLoaderManager().restartLoader(SUGGESTION_LOADER_ID, null, mSuggestionLoaderCallbacks);
> }
>
> private void setSuggestions(ArrayList<String> suggestions) {
> + mSearchEngines.get(0).suggestions = suggestions;
Not a problem in your patch but I wonder: is mSearchEngines.get(0) guaranteed to be non-null?
Attachment #781380 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #2)
> Not a problem in your patch but I wonder: is mSearchEngines.get(0)
> guaranteed to be non-null?
Yeah, I think so. SearchLoader (which is the only place this method gets called) won't be triggered without a SuggestClient, and we don't create a SuggestClient unless we have a search engine with suggestions.
Assignee | ||
Comment 5•11 years ago
|
||
Status: NEW → ASSIGNED
Whiteboard: fixed-fig
Comment 6•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Updated•4 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
•