[Downloads] Automatic resumption is not working |
||||||
Issue descriptionAutomatically resuming downloads does not work until Chrome is started. Looks like during a refactor we moved resumption handling into DownloadNotificationService#handleDownloadOperation(). However there is an early-out there that apparently makes that intent never get processed. This is the result of a combination of refactor bugs meant to work around Android Service issues, and was fixed by the larger notification refactor. We had to turn off that code path due to other Android issues, so the bug showed up again. Reasonable fix is to make handleDownloadOperation() not early-out if entry is null and the link is not an offline page (see ~line 1259). Bug probably exists in M62, should exist in M63, and was found in M64. While it's unfortunate, I think we can fix it in M65 and not disrupt M64 release since it was already happening in earlier versions and did not cause large backlash or a huge hit on metrics that we noticed. cc'ing relevant folks about the issue.
,
Jan 18 2018
M63(63.0.3239.111) and M64(64.0.3282.101) tested on Samsung Grand neo plus(GT-I9060C)/KTU84P if flag is disable -> when airplane mode is Turn off, download status is pending if flag is enable ->when airplane mode is Turn off, Automatically resuming
,
Jan 19 2018
This is seen in two cases: (1) Airplane on/swipe/airplane off 1. Start download 2. Turn on airplane mode 3. Swipe Chrome away 4. Turn off airplane mode Expected behavior: download goes into pending when network is gone but automatically resumes after network returns. (2) Chrome crashes 1. Start download 2. Crash Chrome (chrome://inducebrowsercrashforrealz) Expected behavior: download goes into pending briefly but is restarted automatically after a short while and Chrome (native) has restarted. -------------- There is one fix mentioned in the bug that does not return early (adding a check to allow ACTION_DOWNLOAD_RESUME_ALL to fall through after line 1141 https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java?sq=package:chromium&dr=CSs&l=1141). However, this only works on pre-O devices, likely because of the behavior changes when the service becomes foreground. Investigating!
,
Jan 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdb517125d56780650fc110cb05ea9322f671027 commit bdb517125d56780650fc110cb05ea9322f671027 Author: Joy Ming <jming@chromium.org> Date: Tue Jan 23 00:06:36 2018 [Downloads notifications] Fix autoresumption bug. In DownloadsNotificationService (when the #enable-downloads-foreground flag is off), there is a bug in that the automatic resumption that is initiated after the service restarts automatically (ie. when Chrome crashes in the middle of the download or if airplane mode is turned on, Chrome is swiped away, and airplane mode is turned off). This CL fixes that bug by making sure the code does not exit early. Bug: 803512 Change-Id: I63f35a22c5382978d3808548485030c3466132d4 Reviewed-on: https://chromium-review.googlesource.com/875258 Reviewed-by: David Trainor <dtrainor@chromium.org> Commit-Queue: Joy Ming <jming@chromium.org> Cr-Commit-Position: refs/heads/master@{#531071} [modify] https://crrev.com/bdb517125d56780650fc110cb05ea9322f671027/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
,
Jan 23 2018
,
Jan 23 2018
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
,
Jan 25 2018
Does the fix need to be merged in M65? If so, please request merge.
,
Jan 25 2018
,
Jan 26 2018
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04a029be15fead0665593458e05e82e64802cd4d commit 04a029be15fead0665593458e05e82e64802cd4d Author: Joy Ming <jming@chromium.org> Date: Mon Jan 29 18:16:43 2018 [Downloads notifications] Fix autoresumption bug. In DownloadsNotificationService (when the #enable-downloads-foreground flag is off), there is a bug in that the automatic resumption that is initiated after the service restarts automatically (ie. when Chrome crashes in the middle of the download or if airplane mode is turned on, Chrome is swiped away, and airplane mode is turned off). This CL fixes that bug by making sure the code does not exit early. Bug: 803512 Change-Id: I63f35a22c5382978d3808548485030c3466132d4 Reviewed-on: https://chromium-review.googlesource.com/875258 Reviewed-by: David Trainor <dtrainor@chromium.org> Commit-Queue: Joy Ming <jming@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#531071}(cherry picked from commit bdb517125d56780650fc110cb05ea9322f671027) Reviewed-on: https://chromium-review.googlesource.com/891538 Reviewed-by: Joy Ming <jming@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#139} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/04a029be15fead0665593458e05e82e64802cd4d/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by dtrainor@chromium.org
, Jan 18 2018