Catch exception from DownloadManager.getUriForDownloadedFile() for Xiaomi phones

Catch exception from DownloadManager.getUriForDownloadedFile() for Xiaomi phones

This is crashing on Xiaomi phones
Android API doc never deprecates the call, so it is their ROM issue
Catch the exception so Chrome will not crash.
But click on download completion notification will not have any effect.

BUG=678938
Review-Url: https://codereview.chromium.org/2616893005
Cr-Commit-Position: refs/heads/master@{#442750}
Committed: https://chromium.googlesource.com/chromium/src/+/f8bee1d41368c1a6f66901fdf65dbdb49e002127

[qinmin, 2017-01-06 23:18:20]

qinmin@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[qinmin, 2017-01-06 23:18:20]

PTAL

[gone, 2017-01-06 23:37:36]

IIRC there's a second location where this function is called from.  Does that
need to be updated too?  Maybe clicking on the notification in this case should
send you to download home?

[qinmin, 2017-01-10 00:04:15]

On 2017/01/06 23:37:36, dfalcantara (check queue) wrote:
> IIRC there's a second location where this function is called from.  Does that
> need to be updated too?  Maybe clicking on the notification in this case
should
> send you to download home?

We also called the call in DownloadBroadcastReceiver, catched the exception
there too.
In both places, if getUriForDownloadedFile() returns null, chrome always call
DownloadManagerService.openDownloadPage(), which should bring us to download
home

[gone, 2017-01-10 19:14:34]

On 2017/01/10 00:04:15, qinmin wrote:
> On 2017/01/06 23:37:36, dfalcantara (check queue) wrote:
> > IIRC there's a second location where this function is called from.  Does
that
> > need to be updated too?  Maybe clicking on the notification in this case
> should
> > send you to download home?
> 
> We also called the call in DownloadBroadcastReceiver, catched the exception
> there too.
> In both places, if getUriForDownloadedFile() returns null, chrome always call
> DownloadManagerService.openDownloadPage(), which should bring us to download
> home

So does that mean you need to update that callsite for this CL, or is this patch
reviewable as is?

[gone, 2017-01-10 19:15:03]

On 2017/01/10 19:14:34, dfalcantara (check queue) wrote:
> On 2017/01/10 00:04:15, qinmin wrote:
> > On 2017/01/06 23:37:36, dfalcantara (check queue) wrote:
> > > IIRC there's a second location where this function is called from.  Does
> that
> > > need to be updated too?  Maybe clicking on the notification in this case
> > should
> > > send you to download home?
> > 
> > We also called the call in DownloadBroadcastReceiver, catched the exception
> > there too.
> > In both places, if getUriForDownloadedFile() returns null, chrome always
call
> > DownloadManagerService.openDownloadPage(), which should bring us to download
> > home
> 
> So does that mean you need to update that callsite for this CL, or is this
patch
> reviewable as is?

(lgtm if nothing else to add)

[qinmin, 2017-01-10 22:12:04]

On 2017/01/10 19:14:34, dfalcantara (check queue) wrote:
> On 2017/01/10 00:04:15, qinmin wrote:
> > On 2017/01/06 23:37:36, dfalcantara (check queue) wrote:
> > > IIRC there's a second location where this function is called from.  Does
> that
> > > need to be updated too?  Maybe clicking on the notification in this case
> > should
> > > send you to download home?
> > 
> > We also called the call in DownloadBroadcastReceiver, catched the exception
> > there too.
> > In both places, if getUriForDownloadedFile() returns null, chrome always
call
> > DownloadManagerService.openDownloadPage(), which should bring us to download
> > home
> 
> So does that mean you need to update that callsite for this CL, or is this
patch
> reviewable as is?

Updated the call site in DownloadBroadcastReceiver. Sorry, git cl upload was
stuck yesterday waiting for my patch description :(

[gone, 2017-01-10 22:13:22]

cool cool

lgtm

[qinmin, 2017-01-10 22:53:54]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2017-01-10 22:54:22]

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-01-11 00:25:00]

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

[commit-bot: I haz the power, 2017-01-11 00:25:01]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[qinmin, 2017-01-11 00:30:18]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2017-01-11 00:31:00]

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-01-11 01:03:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484094618981640,
"parent_rev": "267fd834f5c7c9d1e65b0a391a169bc689ba4a85", "commit_rev":
"f8bee1d41368c1a6f66901fdf65dbdb49e002127"}

[commit-bot: I haz the power, 2017-01-11 01:04:37]

Description was changed from

==========
Catch exception from DownloadManager.getUriForDownloadedFile() for Xiaomi phones

This is crashing on Xiaomi phones
Android API doc never deprecates the call, so it is their ROM issue
Catch the exception so Chrome will not crash.
But click on download completion notification will not have any effect.

BUG=678938
==========

to

==========
Catch exception from DownloadManager.getUriForDownloadedFile() for Xiaomi phones

This is crashing on Xiaomi phones
Android API doc never deprecates the call, so it is their ROM issue
Catch the exception so Chrome will not crash.
But click on download completion notification will not have any effect.

BUG=678938

Review-Url: https://codereview.chromium.org/2616893005
Cr-Commit-Position: refs/heads/master@{#442750}
Committed:
https://chromium.googlesource.com/chromium/src/+/f8bee1d41368c1a6f66901fdf65d...
==========

[commit-bot: I haz the power, 2017-01-11 01:04:38]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f8bee1d41368c1a6f66901fdf65d...