New issue
Advanced search Search tips

Issue 895013 link

Starred by 2 users
Status: Started
Owner:
xingliu@chromium.org
Cc:
qin...@chromium.org
shaktisahu@chromium.org
dtrainor@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Offline Item states: Download home v2 should behave the same as old download home.

Project Member Reported by xingliu@chromium.org, Oct 12 
Chrome Version: 71.*
OS: Android

On different combination of states and error encountered of downloads, UI should behave the same as v1 on both notification UI surface and download home v2 UI surface.

Comment 1 by xingliu@chromium.org, Oct 16 

Notification code path still uses the old plumbing objects.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 22 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32528739021da25e59182db41c1b09521a1aa432

commit 32528739021da25e59182db41c1b09521a1aa432
Author: Xing Liu <xingliu@chromium.org>
Date: Mon Oct 22 20:35:00 2018

Download Home V2: Maps errors to OfflineItemStates.

This CL implements a code path to make Java side known how OfflineItem
should be resumed.

OfflineItemState::FAILED: Cannot resume, no further user actions.

OfflineItemState::INTERRUPTED: Can resume, but need to restart from
beginning. Should hook to retry button.

OfflineItemState::PENDING/PAUSE still uses
DownloadUtils.isDownloadPending/isDownloadPause, when download can be
resumed in the middle.


Bug: 895018,895013
Change-Id: I36a82cf3754093bfab85a623818d9e2392a6fc73
Reviewed-on: https://chromium-review.googlesource.com/c/1289677
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601713}
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/chrome/browser/android/download/download_utils.cc
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/chrome/browser/download/offline_item_utils.cc
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/chrome/browser/download/offline_item_utils.h
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/components/download/public/common/BUILD.gn
[modify] https://crrev.com/32528739021da25e59182db41c1b09521a1aa432/components/download/public/common/resume_mode.h

Comment 3 Deleted

Comment 4 by xingliu@chromium.org, Oct 22

Labels: Merge-Request-71
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 23

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 23

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b

commit fe3131be6097f49f3f8e361835d10ca8d1aa8a2b
Author: Xing Liu <xingliu@chromium.org>
Date: Tue Oct 23 23:14:03 2018

Download Home V2: Maps errors to OfflineItemStates.

This CL implements a code path to make Java side known how OfflineItem
should be resumed.

OfflineItemState::FAILED: Cannot resume, no further user actions.

OfflineItemState::INTERRUPTED: Can resume, but need to restart from
beginning. Should hook to retry button.

OfflineItemState::PENDING/PAUSE still uses
DownloadUtils.isDownloadPending/isDownloadPause, when download can be
resumed in the middle.


TBR=xingliu@chromium.org

(cherry picked from commit 32528739021da25e59182db41c1b09521a1aa432)

Bug: 895018,895013
Change-Id: I36a82cf3754093bfab85a623818d9e2392a6fc73
Reviewed-on: https://chromium-review.googlesource.com/c/1289677
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601713}
Reviewed-on: https://chromium-review.googlesource.com/c/1297583
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#279}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/chrome/browser/android/download/download_utils.cc
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/chrome/browser/download/offline_item_utils.cc
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/chrome/browser/download/offline_item_utils.h
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/components/download/public/common/BUILD.gn
[modify] https://crrev.com/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b/components/download/public/common/resume_mode.h
Project Member

Comment 7 by cr-audit...@appspot.gserviceaccount.com, Oct 23

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/fe3131be6097f49f3f8e361835d10ca8d1aa8a2b

Commit: fe3131be6097f49f3f8e361835d10ca8d1aa8a2b
Author: xingliu@chromium.org
Commiter: xingliu@chromium.org
Date: 2018-10-23 23:14:03 +0000 UTC

Download Home V2: Maps errors to OfflineItemStates.

This CL implements a code path to make Java side known how OfflineItem
should be resumed.

OfflineItemState::FAILED: Cannot resume, no further user actions.

OfflineItemState::INTERRUPTED: Can resume, but need to restart from
beginning. Should hook to retry button.

OfflineItemState::PENDING/PAUSE still uses
DownloadUtils.isDownloadPending/isDownloadPause, when download can be
resumed in the middle.


TBR=xingliu@chromium.org

(cherry picked from commit 32528739021da25e59182db41c1b09521a1aa432)

Bug: 895018,895013
Change-Id: I36a82cf3754093bfab85a623818d9e2392a6fc73
Reviewed-on: https://chromium-review.googlesource.com/c/1289677
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601713}
Reviewed-on: https://chromium-review.googlesource.com/c/1297583
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#279}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 24 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4a3b4677b7e125e58bb065fc1a705847a4bef55

commit a4a3b4677b7e125e58bb065fc1a705847a4bef55
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed Oct 24 01:18:19 2018

Revert "Download Home V2: Maps errors to OfflineItemStates."

This reverts commit fe3131be6097f49f3f8e361835d10ca8d1aa8a2b.

Reason for revert: Causes build failure, because branch is missing this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1281422

Original change's description:
> Download Home V2: Maps errors to OfflineItemStates.
> 
> This CL implements a code path to make Java side known how OfflineItem
> should be resumed.
> 
> OfflineItemState::FAILED: Cannot resume, no further user actions.
> 
> OfflineItemState::INTERRUPTED: Can resume, but need to restart from
> beginning. Should hook to retry button.
> 
> OfflineItemState::PENDING/PAUSE still uses
> DownloadUtils.isDownloadPending/isDownloadPause, when download can be
> resumed in the middle.
> 
> 
> TBR=xingliu@chromium.org
> 
> (cherry picked from commit 32528739021da25e59182db41c1b09521a1aa432)
> 
> Bug: 895018,895013
> Change-Id: I36a82cf3754093bfab85a623818d9e2392a6fc73
> Reviewed-on: https://chromium-review.googlesource.com/c/1289677
> Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
> Reviewed-by: Min Qin <qinmin@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#601713}
> Reviewed-on: https://chromium-review.googlesource.com/c/1297583
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#279}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=dtrainor@chromium.org,qinmin@chromium.org,xingliu@chromium.org

Change-Id: I0eab61730672a4d2b0d338425616f22f67e544ab
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 895018, 895013
Reviewed-on: https://chromium-review.googlesource.com/c/1297648
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#283}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/chrome/browser/android/download/download_utils.cc
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/chrome/browser/download/offline_item_utils.cc
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/chrome/browser/download/offline_item_utils.h
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/components/download/public/common/BUILD.gn
[modify] https://crrev.com/a4a3b4677b7e125e58bb065fc1a705847a4bef55/components/download/public/common/resume_mode.h
Project Member

Comment 9 by cr-audit...@appspot.gserviceaccount.com, Oct 24 

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/a4a3b4677b7e125e58bb065fc1a705847a4bef55

Commit: a4a3b4677b7e125e58bb065fc1a705847a4bef55
Author: nyquist@chromium.org
Commiter: nyquist@chromium.org
Date: 2018-10-24 01:18:19 +0000 UTC

Revert "Download Home V2: Maps errors to OfflineItemStates."

This reverts commit fe3131be6097f49f3f8e361835d10ca8d1aa8a2b.

Reason for revert: Causes build failure, because branch is missing this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1281422

Original change's description:
> Download Home V2: Maps errors to OfflineItemStates.
> 
> This CL implements a code path to make Java side known how OfflineItem
> should be resumed.
> 
> OfflineItemState::FAILED: Cannot resume, no further user actions.
> 
> OfflineItemState::INTERRUPTED: Can resume, but need to restart from
> beginning. Should hook to retry button.
> 
> OfflineItemState::PENDING/PAUSE still uses
> DownloadUtils.isDownloadPending/isDownloadPause, when download can be
> resumed in the middle.
> 
> 
> TBR=xingliu@chromium.org
> 
> (cherry picked from commit 32528739021da25e59182db41c1b09521a1aa432)
> 
> Bug: 895018,895013
> Change-Id: I36a82cf3754093bfab85a623818d9e2392a6fc73
> Reviewed-on: https://chromium-review.googlesource.com/c/1289677
> Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
> Reviewed-by: Min Qin <qinmin@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#601713}
> Reviewed-on: https://chromium-review.googlesource.com/c/1297583
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#279}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=dtrainor@chromium.org,qinmin@chromium.org,xingliu@chromium.org

Change-Id: I0eab61730672a4d2b0d338425616f22f67e544ab
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 895018, 895013
Reviewed-on: https://chromium-review.googlesource.com/c/1297648
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#283}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Comment 10 by xingliu@chromium.org, Oct 24 

This CL needs another CL landed after M71 branch point. Do we still need this CL to be merged to M71? This is mostly needed by the retry CL targeted M72 I think.
►

Sign in to add a comment