Fixed download notification crash caused by uninitialized browser process

Fixed download notification crash caused by uninitialized browser process

If browser process is killed, tapping on download notification leads to
a crash. Currently this is happening due to an early call to
getLastUsedProfile() used by FeatureEngagementTracker. This can be
prevented by adding an extra check for browser statup complete.

Also reordered the openDownloadedContent function to open the
downloaded item instead of download home in that scenario. That alone
should also prevent the crash.

BUG=715442
Review-Url: https://codereview.chromium.org/2876973002
Cr-Commit-Position: refs/heads/master@{#471195}
Committed: https://chromium.googlesource.com/chromium/src/+/96d623e0e8954edf9c9908ff5ccfa9580b4946a7

[shaktisahu, 2017-05-11 22:32:13]

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

[shaktisahu, 2017-05-11 22:32:14]

PTAL

https://codereview.chromium.org/2876973002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
(left):

https://codereview.chromium.org/2876973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java:1130:
|| !hasDownloadManagerService()) {
This was added to fix the original crash in
https://codereview.chromium.org/2842163005
However, this is probably less preferable since it would open download home
instead of the item.

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

https://codereview.chromium.org/2876973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:165:
if (BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
This might not be needed if the other fix is added in
DownloadManagerService.openDownloadedContent. However being a public function, I
feel we should have this check here or a similar check to skip the
FeatureEngagementTracker stuff  when tab == null

[qinmin, 2017-05-11 23:39:13]

lgtm

[shaktisahu, 2017-05-11 23:40:05]

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

[commit-bot: I haz the power, 2017-05-11 23:41:21]

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-05-11 23:47:34]

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

[commit-bot: I haz the power, 2017-05-11 23:47:35]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[shaktisahu, 2017-05-12 00:10:36]

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

[commit-bot: I haz the power, 2017-05-12 00:11:21]

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-05-12 01:52:52]

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

[commit-bot: I haz the power, 2017-05-12 01:52:53]

Dry run: This issue passed the CQ dry run.

[shaktisahu, 2017-05-12 02:41:04]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2017-05-12 02:41:04]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/2876973002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-12 02:41:48]

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-05-12 02:46:32]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494556864040500,
"parent_rev": "212c28942bc0778facead183e54271561cd16d38", "commit_rev":
"96d623e0e8954edf9c9908ff5ccfa9580b4946a7"}

[commit-bot: I haz the power, 2017-05-12 02:46:48]

Description was changed from

==========
Fixed download notification crash caused by uninitialized browser process

If browser process is killed, tapping on download notification leads to
a crash. Currently this is happening due to an early call to
getLastUsedProfile() used by FeatureEngagementTracker. This can be
prevented by adding an extra check for browser statup complete.

Also reordered the openDownloadedContent function to open the
downloaded item instead of download home in that scenario. That alone
should also prevent the crash.

BUG=715442
==========

to

==========
Fixed download notification crash caused by uninitialized browser process

If browser process is killed, tapping on download notification leads to
a crash. Currently this is happening due to an early call to
getLastUsedProfile() used by FeatureEngagementTracker. This can be
prevented by adding an extra check for browser statup complete.

Also reordered the openDownloadedContent function to open the
downloaded item instead of download home in that scenario. That alone
should also prevent the crash.

BUG=715442

Review-Url: https://codereview.chromium.org/2876973002
Cr-Commit-Position: refs/heads/master@{#471195}
Committed:
https://chromium.googlesource.com/chromium/src/+/96d623e0e8954edf9c9908ff5ccf...
==========

[commit-bot: I haz the power, 2017-05-12 02:46:49]

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