add pause button for resumable in-progress downloads.

add pause button for resumable in-progress downloads.

This CL adds a pause button for in-progress downloads that are
resumable.

UI review: https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/jixCWMMg_vA
Mocks: https://folio.googleplex.com/chrome-ux/mocks/065-downloads/07-resumable/121615_NotificationChanges

BUG=579110
Committed: https://crrev.com/62c38d412f4501d5bf8411bf87ee1b81a3975cb8
Cr-Commit-Position: refs/heads/master@{#374702}

[qinmin, 2016-02-09 00:10:35]

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

[qinmin, 2016-02-09 00:10:36]

PTAL

[Ted C, 2016-02-09 20:21:21]

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

https://codereview.chromium.org/1673183003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:267:
&& !DownloadManagerService.hasDownloadManagerService()) {
in this case, should we update the icon to resume?

If we don't, then the user wouldn't have the ability to cause the process to
restart right?

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

https://codereview.chromium.org/1673183003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java:224:
info.getDownloadId(), info.getFileName(), info.isResumable());
what happens if it is paused but not resumable?

[qinmin, 2016-02-09 20:57:44]

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

https://codereview.chromium.org/1673183003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:267:
&& !DownloadManagerService.hasDownloadManagerService()) {
On 2016/02/09 20:21:20, Ted C wrote:
> in this case, should we update the icon to resume?
> 
> If we don't, then the user wouldn't have the ability to cause the process to
> restart right?

When browser process goes away, pauseAllDownloads() should get called.
(onCreate() was called before onStartCommand()) so we should have already showed
the resume button.

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

https://codereview.chromium.org/1673183003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java:224:
info.getDownloadId(), info.getFileName(), info.isResumable());
On 2016/02/09 20:21:20, Ted C wrote:
> what happens if it is paused but not resumable?


Good question. We don't show the pause button for download whose isResumable()
is false. As a result, there is no way to pause the download if isResumable() is
false since pause will only be caused by user interactions. Time out, connection
error, browser crash won't trigger pause. I added an assert here to check
isResumable() should be true.

However, isResumable() contains only a small set of downloads that are not
resumable. In most cases, we cannot really know whether a download is resumable
before we try to resume it (eg, session cookies etc). So in those cases, we
always show a resume button. When user hits the resume button, it will fail if
the download is not really resumable.

[Ted C, 2016-02-09 21:02:51]

lgtm

[qinmin, 2016-02-09 21:18:45]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-02-09 21:20:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1673183003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1673183003/20001

[commit-bot: I haz the power, 2016-02-10 00:47:19]

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

[commit-bot: I haz the power, 2016-02-10 00:47:20]

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

[qinmin, 2016-02-10 00:48:03]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-02-10 00:50:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1673183003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1673183003/20001

[commit-bot: I haz the power, 2016-02-10 06:53:35]

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

[commit-bot: I haz the power, 2016-02-10 06:53:36]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_TIMED_OUT,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[qinmin, 2016-02-10 19:24:55]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-02-10 19:27:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1673183003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1673183003/20001

[commit-bot: I haz the power, 2016-02-10 19:42:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 22:30:30]

Description was changed from

==========
add pause button for resumable in-progress downloads.

This CL adds a pause button for in-progress downloads that are
resumable.

UI review:
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/jixCWMM...
Mocks:
https://folio.googleplex.com/chrome-ux/mocks/065-downloads/07-resumable/12161...

BUG=579110
==========

to

==========
add pause button for resumable in-progress downloads.

This CL adds a pause button for in-progress downloads that are
resumable.

UI review:
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/jixCWMM...
Mocks:
https://folio.googleplex.com/chrome-ux/mocks/065-downloads/07-resumable/12161...

BUG=579110

Committed: https://crrev.com/62c38d412f4501d5bf8411bf87ee1b81a3975cb8
Cr-Commit-Position: refs/heads/master@{#374702}
==========

[commit-bot: I haz the power, 2016-02-16 22:30:31]

Patchset 2 (id:??) landed as
https://crrev.com/62c38d412f4501d5bf8411bf87ee1b81a3975cb8
Cr-Commit-Position: refs/heads/master@{#374702}