NPE at BrowserSearch$SearchAdapter.getCount

RESOLVED FIXED in Firefox 26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bnicholson, Assigned: bnicholson)

Tracking

Trunk
Firefox 26
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-fig)

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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

6 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 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+

Updated

6 years ago
Duplicate of this bug: 898424
(Assignee)

Comment 4

6 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

6 years ago
http://hg.mozilla.org/projects/fig/rev/d42a6a047a7a
Status: NEW → ASSIGNED
Whiteboard: fixed-fig
https://hg.mozilla.org/mozilla-central/rev/d42a6a047a7a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.