Pass original profile to OfflinePageDownloadBridge constructor

Pass original profile to OfflinePageDownloadBridge constructor

In a couple cases, OfflinePageDownloadBridge is constructed with
last used profile which might be incognito profile. OfflinePageModel
is null for this profile, thus we see a crash in
DownloadUIAdapter::FromOfflinePageModel

BUG=692163
Review-Url: https://codereview.chromium.org/2836423002
Cr-Commit-Position: refs/heads/master@{#467475}
Committed: https://chromium.googlesource.com/chromium/src/+/7902e09852f047a487122472800d482fd28e6b7c

[jianli, 2017-04-25 22:50:10]

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

[jianli, 2017-04-25 22:50:43]

jianli@chromium.org changed reviewers:
+ dewittj@chromium.org

[commit-bot: I haz the power, 2017-04-25 22:50:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dewittj, 2017-04-25 23:00:12]

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
(right):

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:76:
return new
OfflinePageDownloadBridge(Profile.getLastUsedProfile().getgetOriginalProfile());
s/getget/get/

also below.

And Profile.getLastUsedProfile() can return null - should we check for this as
well?

[Dmitry Titov, 2017-04-25 23:00:26]

dimich@chromium.org changed reviewers:
+ dimich@chromium.org

[Dmitry Titov, 2017-04-25 23:00:26]

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
(right):

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:76:
return new
OfflinePageDownloadBridge(Profile.getLastUsedProfile().getgetOriginalProfile());
getget

Also, should we rather call getOriginalProfile in the constructor of
OfflinePageDownloadBridge rather than in all the places that create one?

[jianli, 2017-04-25 23:18:19]

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

[jianli, 2017-04-25 23:18:32]

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
(right):

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:76:
return new
OfflinePageDownloadBridge(Profile.getLastUsedProfile().getgetOriginalProfile());
On 2017/04/25 23:00:12, dewittj wrote:
> s/getget/get/
> 
> also below.
> 
> And Profile.getLastUsedProfile() can return null - should we check for this as
> well?

Profile.getLastUsedProfile() will never return null.

https://codereview.chromium.org/2836423002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:76:
return new
OfflinePageDownloadBridge(Profile.getLastUsedProfile().getgetOriginalProfile());
On 2017/04/25 23:00:26, Dmitry Titov wrote:
> getget
> 
> Also, should we rather call getOriginalProfile in the constructor of
> OfflinePageDownloadBridge rather than in all the places that create one?

Good idea. Done.

[commit-bot: I haz the power, 2017-04-25 23:19:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dmitry Titov, 2017-04-25 23:23:13]

LGTM with a nit (and of course pending dewittj@ comments)

https://codereview.chromium.org/2836423002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
(right):

https://codereview.chromium.org/2836423002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:80:
// The incognito profile may be passed.
This is a bit misleading comment. It is not clear if 'passed' means to this
method or further down. Maybe "If passed in profile is Incognito, switch to the
regular since downloads are shared between them, just like Bookmarks" or
something to same effect.

[dewittj, 2017-04-25 23:29:59]

lgtm

[jianli, 2017-04-25 23:33:35]

https://codereview.chromium.org/2836423002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
(right):

https://codereview.chromium.org/2836423002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java:80:
// The incognito profile may be passed.
On 2017/04/25 23:23:12, Dmitry Titov wrote:
> This is a bit misleading comment. It is not clear if 'passed' means to this
> method or further down. Maybe "If passed in profile is Incognito, switch to
the
> regular since downloads are shared between them, just like Bookmarks" or
> something to same effect.

Done.

[jianli, 2017-04-25 23:34:20]

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

[jianli, 2017-04-25 23:34:21]

qinmin for download

[qinmin, 2017-04-26 16:52:57]

lgtm

[qinmin, 2017-04-26 16:52:59]

lgtm

[jianli, 2017-04-26 20:40:45]

The CQ bit was checked by jianli@chromium.org

[jianli, 2017-04-26 20:40:46]

The patchset sent to the CQ was uploaded after l-g-t-m from dimich@chromium.org,
dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/2836423002/#ps40001
(title: "Update comment")

[commit-bot: I haz the power, 2017-04-26 20:41:43]

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-26 20:53:03]

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

[commit-bot: I haz the power, 2017-04-26 20:53:04]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jianli, 2017-04-26 20:59:43]

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

[jianli, 2017-04-26 20:59:44]

dfalcantara for download/ui

I didn't realize that download/ui/OWNERS has "set noparent"

[gone, 2017-04-26 21:03:17]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-04-26 21:03:17]

lgtm

[jianli, 2017-04-26 21:03:50]

The CQ bit was unchecked by jianli@chromium.org

[commit-bot: I haz the power, 2017-04-26 21:03:52]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jianli, 2017-04-26 21:03:55]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2017-04-26 21:04:37]

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-26 22:06:03]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493240635078750,
"parent_rev": "3109df1dc485aa29d6dba54f57a5762fd8957830", "commit_rev":
"36bfb8567013ddbb44f6199d2aabd6e802af8dad"}

[commit-bot: I haz the power, 2017-04-26 22:06:29]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493240635078750,
"parent_rev": "8a5f6b6a321aea54efd0e1d5a7aed02cc2b271e0", "commit_rev":
"7902e09852f047a487122472800d482fd28e6b7c"}

[commit-bot: I haz the power, 2017-04-26 22:06:37]

Description was changed from

==========
Pass original profile to OfflinePageDownloadBridge constructor

In a couple cases, OfflinePageDownloadBridge is constructed with
last used profile which might be incognito profile. OfflinePageModel
is null for this profile, thus we see a crash in
DownloadUIAdapter::FromOfflinePageModel

BUG=692163
==========

to

==========
Pass original profile to OfflinePageDownloadBridge constructor

In a couple cases, OfflinePageDownloadBridge is constructed with
last used profile which might be incognito profile. OfflinePageModel
is null for this profile, thus we see a crash in
DownloadUIAdapter::FromOfflinePageModel

BUG=692163

Review-Url: https://codereview.chromium.org/2836423002
Cr-Commit-Position: refs/heads/master@{#467475}
Committed:
https://chromium.googlesource.com/chromium/src/+/7902e09852f047a487122472800d...
==========

[commit-bot: I haz the power, 2017-04-26 22:06:39]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7902e09852f047a487122472800d...