Closed Bug 898219 Opened 8 years ago Closed 8 years ago

NPE at BrowserSearch$SearchAdapter.getCount

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: bnicholson, Assigned: bnicholson)

References

Details

(Whiteboard: fixed-fig)

Attachments

(1 file)

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)
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+
Duplicate of this bug: 898424
(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.
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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.