Move downloads Uri query to not run on UI thread

Move downloads Uri query to not run on UI thread

Make the DownloadBroadcastReceiver call goAsync when opening the
download.  This is because the receiver needs to query the
DownloadManager to build the Uri, which should not run on the UI thread.

BUG=709567

[David Trainor- moved to gerrit, 2017-04-07 20:26:28]

dtrainor@chromium.org changed reviewers:
+ nyquist@chromium.org, qinmin@chromium.org

[David Trainor- moved to gerrit, 2017-04-07 20:26:29]

ptal thanks!

[nyquist, 2017-04-07 20:30:04]

lgtm

[qinmin, 2017-04-10 16:23:23]

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
(right):

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java:87:
task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
This could throw exceptions. If goAsync() is called and an exception is thrown,
not sure whether the system will clean up its state about the BroadcastReceiver.

[David Trainor- moved to gerrit, 2017-04-11 01:13:15]

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
(right):

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java:87:
task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
On 2017/04/10 16:23:23, qinmin wrote:
> This could throw exceptions. If goAsync() is called and an exception is
thrown,
> not sure whether the system will clean up its state about the
BroadcastReceiver.

executeOnExecutor() or the internal content?  I'm happy to add a try/finally to
the onPostExecute if necessary.

[qinmin, 2017-04-11 06:18:58]

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
(right):

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java:87:
task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
On 2017/04/11 01:13:15, David Trainor-ping if over 24h wrote:
> On 2017/04/10 16:23:23, qinmin wrote:
> > This could throw exceptions. If goAsync() is called and an exception is
> thrown,
> > not sure whether the system will clean up its state about the
> BroadcastReceiver.
> 
> executeOnExecutor() or the internal content?  I'm happy to add a try/finally
to
> the onPostExecute if necessary.

executeOnExecutor() could throw RejectedExecutionException.

[David Trainor- moved to gerrit, 2017-04-11 16:08:22]

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
(right):

https://codereview.chromium.org/2805533007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java:87:
task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
On 2017/04/11 06:18:58, qinmin wrote:
> On 2017/04/11 01:13:15, David Trainor-ping if over 24h wrote:
> > On 2017/04/10 16:23:23, qinmin wrote:
> > > This could throw exceptions. If goAsync() is called and an exception is
> > thrown,
> > > not sure whether the system will clean up its state about the
> > BroadcastReceiver.
> > 
> > executeOnExecutor() or the internal content?  I'm happy to add a try/finally
> to
> > the onPostExecute if necessary.
> 
> executeOnExecutor() could throw RejectedExecutionException.

Ah nice catch.  Sigh the documentation doesn't mention that... only that it can
throw an IllegalStateException if the async task is running, which wouldn't
apply here.

[David Trainor- moved to gerrit, 2017-04-11 20:47:42]

The CQ bit was checked by dtrainor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 20:48:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 21:33:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-11 21:33:59]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-04-12 07:16:18]

ptal.  Added try catch.

[qinmin, 2017-04-12 13:37:05]

lgtm

[nyquist, 2017-04-18 22:44:20]

Still lgtm :-)

https://codereview.chromium.org/2805533007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
(right):

https://codereview.chromium.org/2805533007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java:61:
final long id = ids[0];
Any reason why we're not doing this conditionally anymore? (i.e. in the
onPostExecute I guess). I was just wondering. I'm fine either way.