730874 - crash [@ java.lang.IllegalStateException: Cannot add header view to list -- setAdapter has already been called. at android.widget.ListView.addHeaderView(ListView.java) ]

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This bug was filed from the Socorro interface and is 
report bp-928a705e-4964-404c-95fb-7d2242120225 .
============================================================= 
java.lang.IllegalStateException: Cannot add header view to list -- setAdapter has already been called.
	at android.widget.ListView.addHeaderView(ListView.java:279)
	at org.mozilla.gecko.AwesomeBarTabs$BookmarksQueryTask.onPostExecute(AwesomeBarTabs.java:365)
	at org.mozilla.gecko.AwesomeBarTabs$BookmarksQueryTask.onPostExecute(AwesomeBarTabs.java:329)
	at android.os.AsyncTask.finish(AsyncTask.java:417)
	at android.os.AsyncTask.access$300(AsyncTask.java:127)
	at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:429)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:130)
	at org.mozilla.gecko.GeckoApp$35.run(GeckoApp.java:1824)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at android.app.ActivityThread.main(ActivityThread.java:3683)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597)
	at dalvik.system.NativeStart.main(Native Method)

More Reports:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&query_search=signature&query_type=contains&query=java.lang.IllegalStateException%3A%20Cannot%20add%20header%20view%20to%20list%20--%20setAdapter%20has%20already%20been%20called.%20at%20android.widget.ListView.addHeaderView%28ListView.java%29&reason_type=contains&date=02%2F27%2F2012%2018%3A16%3A55&range_value=30&range_unit=days&hang_type=any&process_type=any&do_query=1&signature=java.lang.IllegalStateException%3A%20Cannot%20add%20header%20view%20to%20list%20--%20setAdapter%20has%20already%20been%20called.%20at%20android.widget.ListView.addHeaderView%28ListView.java%29

Earliest crash seen in 13a1: 20120224163427

Comment 1

[Aaron Train [:aaronmt]]

Regression from bug 722020?

Comment 2

[:Margaret Leibovic]

Hrm, how is that happening? I purposefully set the header view before calling setAdapter:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBarTabs.java#365

I guess maybe we can create a BookmarkQueryTask more than once? I'll look into this.

Comment 3

[:Margaret Leibovic]

Has anyone been able to reproduce this crash?

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Nexus One - 20120227031120
Nexus S - 20120224163427
samsung GT-I9100 - 20120224163427

Have not been able to repro it as of yet.

Comment 5

[:Margaret Leibovic]

Attachment: patch

This is just a speculative fix, since I can't reproduce this issue.

Since the onTabChanged listener is checking to see if the adapters are initialized before making the AsyncTasks, it's possible that we can kick off multiple tasks to initialize the same adapter, which would lead to this crash. Instead, it seems like what we really want is to make sure we haven't already kicked off an async task, so this patch keeps track of whether or not that's happened.

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 601461 [details] [diff] [review]
patch

Review of attachment 601461 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/AwesomeBarTabs.java
@@ +710,5 @@
>                  boolean hideSoftInput = true;
>  
>                  // Lazy load bookmarks and history lists. Only query the database
>                  // if those lists requested by user.
> +                if (tabId.equals(BOOKMARKS_TAB) && mBookmarksQueryTask== null) {

I think the right fix is to have something like:

if (tabId.equals(BOOKMARKS_TAB) && mBookmarksAdapter == null && mBookmarksQueryTask == null) {
   ...
}

Which means you'd cover the case where adapter is already set or about to be set by the asynctask. You have to set mBookmarksQueryTask to null once the task is done.

@@ +713,5 @@
>                  // if those lists requested by user.
> +                if (tabId.equals(BOOKMARKS_TAB) && mBookmarksQueryTask== null) {
> +                    mBookmarksQueryTask = new BookmarksQueryTask();
> +                    mBookmarksQueryTask.execute();
> +                } else if (tabId.equals(HISTORY_TAB) && mHistoryQueryTask == null) {

Analogous solution here.

Comment 7

[:Margaret Leibovic]

Attachment: patch v2

Comment 8

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fad541ac48da

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/fad541ac48da