[Downloads] Add Incognito support

[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
Committed: https://crrev.com/7223c9decebd30e78f52166795f942f421ee4aa2
Cr-Commit-Position: refs/heads/master@{#412303}

[gone, 2016-08-15 20:13:51]

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

[gone, 2016-08-15 20:13:52]

Min: PTAL at the backend stuff.

Ian: PTAL at the UI bits.

[gone, 2016-08-15 20:14:45]

Description was changed from

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
==========

to

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
==========

[gone, 2016-08-15 20:25:41]

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

[commit-bot: I haz the power, 2016-08-15 20:26:00]

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, 2016-08-15 20:59:46]

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

[commit-bot: I haz the power, 2016-08-15 20:59:48]

Dry run: This issue passed the CQ dry run.

[Ian Wen, 2016-08-15 23:17:27]

Didn't find any important issues. All nits.

lgtm for UI stuff.

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File chrome/android/java/src/org/chromium/chrome/browser/BasicNativePage.java
(left):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/BasicNativePage.java:23:
private final Tab mTab;
Would you mind making a protected method getTab() instead of a protected
attribute? I feel it's much easier to read the code in child classes, if the
method is protected instead of the attribute.

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
(right):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:31:
intent.putExtra(IntentHandler.EXTRA_OPEN_NEW_INCOGNITO_TAB, tab.isIncognito());
Haha you are sneaky.

It's a bit weird to use this constant, since this is not strictly about "open
new tab". Yet I don't see this to be a super big issue. I might leave it as is.

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:81:
getDownloadManagerService().getAllDownloads(false);
Maybe add some comments here? Calling getAllDownloads() twice in sequence might
be confusing for future readers.

[gone, 2016-08-15 23:29:49]

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File chrome/android/java/src/org/chromium/chrome/browser/BasicNativePage.java
(left):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/BasicNativePage.java:23:
private final Tab mTab;
On 2016/08/15 23:17:27, Ian Wen wrote:
> Would you mind making a protected method getTab() instead of a protected
> attribute? I feel it's much easier to read the code in child classes, if the
> method is protected instead of the attribute.

Switched it so that the Tab is directly passed through.

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
(right):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:31:
intent.putExtra(IntentHandler.EXTRA_OPEN_NEW_INCOGNITO_TAB, tab.isIncognito());
On 2016/08/15 23:17:27, Ian Wen wrote:
> Haha you are sneaky.
> 
> It's a bit weird to use this constant, since this is not strictly about "open
> new tab". Yet I don't see this to be a super big issue. I might leave it as
is.

Yeah... I wasn't sure if it was worth adding yet another extra for "use
incognito".  Theresa didn't feel too strongly about it either.

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://chromiumcodereview.appspot.com/2243423002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:81:
getDownloadManagerService().getAllDownloads(false);
On 2016/08/15 23:17:27, Ian Wen wrote:
> Maybe add some comments here? Calling getAllDownloads() twice in sequence
might
> be confusing for future readers.

Done.

[gone, 2016-08-15 23:48:22]

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

[commit-bot: I haz the power, 2016-08-15 23:48:51]

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, 2016-08-16 00:35:29]

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

[commit-bot: I haz the power, 2016-08-16 00:35:30]

Dry run: This issue passed the CQ dry run.

[qinmin, 2016-08-16 16:53:14]

lgtm % comments

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
File chrome/browser/android/download/download_manager_service.cc (right):

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
chrome/browser/android/download/download_manager_service.cc:231:
content::DownloadManager* off_the_record_manager = GetDownloadManager(true);
Can we do this by checking the DownloadItem's BrowserContext rather than
comparing the DownloadManager pointers?

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
chrome/browser/android/download/download_manager_service.cc:254:
content::DownloadManager* off_the_record_manager = GetDownloadManager(true);
ditto

[gone, 2016-08-16 18:24:25]

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
File chrome/browser/android/download/download_manager_service.cc (right):

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
chrome/browser/android/download/download_manager_service.cc:231:
content::DownloadManager* off_the_record_manager = GetDownloadManager(true);
On 2016/08/16 16:53:14, qinmin wrote:
> Can we do this by checking the DownloadItem's BrowserContext rather than
> comparing the DownloadManager pointers?

Didn't work before, but seems to now.  Weird, but I'll take it.

https://chromiumcodereview.appspot.com/2243423002/diff/40001/chrome/browser/a...
chrome/browser/android/download/download_manager_service.cc:254:
content::DownloadManager* off_the_record_manager = GetDownloadManager(true);
On 2016/08/16 16:53:14, qinmin wrote:
> ditto

Done.

[gone, 2016-08-16 18:24:38]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2016-08-16 18:24:39]

The patchset sent to the CQ was uploaded after l-g-t-m from ianwen@chromium.org,
qinmin@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2243423002/#ps60001
(title: "Replace profile checking")

[commit-bot: I haz the power, 2016-08-16 18:25:10]

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, 2016-08-16 18:26:56]

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

[commit-bot: I haz the power, 2016-08-16 18:26:57]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[gone, 2016-08-16 18:49:37]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2016-08-16 18:49:37]

The patchset sent to the CQ was uploaded after l-g-t-m from ianwen@chromium.org,
qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/2243423002/#ps100001
(title: "Formatting")

[commit-bot: I haz the power, 2016-08-16 18:49:53]

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, 2016-08-16 19:20:44]

Description was changed from

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
==========

to

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
==========

[commit-bot: I haz the power, 2016-08-16 19:20:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-16 19:23:07]

Description was changed from

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324
==========

to

==========
[Downloads] Add Incognito support

* Change Download Home to show Incognito downloads when Downloads
  is opened from an incognito tab.

* DownloadManagerService now monitors regular for Incognito items, as well.

* DownloadManagerService now monitors for AllDownloadItemNotifier::Update()
  to know when to alert the UI about updates to download items
  instead of monitoring it on the Java side.

Minor things:

* Move where the DownloadActivity is launched to DownloadUtils.

* Change the AsyncTask to be explicitly multi-threaded in SpaceDisplay.

BUG=616324

Committed: https://crrev.com/7223c9decebd30e78f52166795f942f421ee4aa2
Cr-Commit-Position: refs/heads/master@{#412303}
==========

[commit-bot: I haz the power, 2016-08-16 19:23:09]

Patchset 6 (id:??) landed as
https://crrev.com/7223c9decebd30e78f52166795f942f421ee4aa2
Cr-Commit-Position: refs/heads/master@{#412303}