700354 - Use of AsyncTask needs some cleanup.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Doug Turner (:dougt)]

What we are seeing is that the completion callback is happening on the wrong thread.  Many users are creating AsyncTasks from the Gecko thread, and then in the completion callback accessing UI elements that must only be accessed from the java thread.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 2

[Doug Turner (:dougt)]

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

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

::: embedding/android/GeckoApp.java
@@ +340,4 @@
>          if (!checkAndSetLaunchState(LaunchState.Launching, LaunchState.Launched))
>              return false;
>  
> +        class GeckoThread extends Thread {

put in its own file.

::: embedding/android/Tab.java
@@ +57,5 @@
>  
> +// AsyncTask runs onPostExecute on the thread it is constructed on
> +// We construct these off of the main thread, and we want that to run 
> +// on the main UI thread, so this is a convenience class to do that
> +abstract class GeckoAsyncTask<Params, Progress, Result> {

move into its own file.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Not sure why you wanted to move GeckoThread out of GeckoApp. I kinda liked it better as an inner class. Easier access to GeckoApp innards and less stuff exposed from GeckoApp.

Otherwise looks fine.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> Comment on attachment 572650 [details] [diff] [review] [diff] [details] [review]
> patch
> 
> Not sure why you wanted to move GeckoThread out of GeckoApp. I kinda liked
> it better as an inner class. Easier access to GeckoApp innards and less
> stuff exposed from GeckoApp.
me too, just doing what doug asked for.

Although, with the landing of pcwalton's patch queue, nothing needs to be made any more public for this change

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Brad, did you land this?

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

yup, https://hg.mozilla.org/projects/birch/rev/13f5d3d70ecb