New issue
Advanced search Search tips

Issue 895018 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Offline Item states: Clean up UI to download backend plumbing.

Project Member Reported by xingliu@chromium.org, Oct 12

Issue description

Chrome Version: 71.*
OS: Android

All UI states should only(mostly) depends on the OfflineItem plumbed from native code. Currently we plumbs different kind of booleans, do different kind of caches in Java layer.

And these boolean flag can be optimized to map to OfflineItemStates, or at least aggregate to a centralized util class.
 
Project Member

Comment 1 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

Issue 889211 has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 23

Labels: 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

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 5 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

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}

Sign in to add a comment