[downloads] notification persists for paused download |
|||||||||
Issue descriptionChrome Version: (copy from chrome://version) Chrome Beta OS: (e.g. Win7, OSX 10.9.5, etc...) What steps will reproduce the problem? (1) Download an offline page. (2) Pause the download. (3) Swipe the download notification away. What is the expected result? There is no notification remaining. What happens instead? A small "Chrome Downloads" notification remains (which indicates the foreground is still on) NOTE: This is reproducible on Chrome Beta (63.0.3239.41) and Chrome Dev (64.0.3261.0). Please use labels and text to provide additional information. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Nov 14 2017
This issue is happening because the foreground DownloadNotificationService is getting unexpectedly restarted and not stopped again. More specifically that "notifyDownloadPuased" is called multiple times, once propagated through DownloadBroadcastReceiver when the user interacts with the notification and another time through SystemDownloadNotifier, presumably through propagation by the native (debugging logs: https://paste.googleplex.com/6577383289126912). This is similar to the other issue that was seen related to offline pages restarting the foreground service after it was stopped already and then not shutting down the service again, in that case after notifyDownloadFailed shuts down the foreground service, notifyDownloadCanceled is called which accidentally starts but does not stop the service (crbug.com/776998). I have written a CL so that the user experience is as intended, but wonder if there's a larger problem where offline pages is altering the foreground service logic unknowingly by making multiple calls.
,
Nov 14 2017
jming@, could you please add the OS for this bug ?
,
Nov 14 2017
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16b84d67c1349f534233cecefc332cf94387542a commit 16b84d67c1349f534233cecefc332cf94387542a Author: Joy Ming <jming@chromium.org> Date: Wed Nov 15 22:15:00 2017 Fix bug where notification for paused download persists. There was a bug where the notification for a paused offline page would persist even after swiped away. This is because in some cases, the foreground download notification service is restarted after the download was paused but it does not get shut down again so downloads remains in the foreground. This CL fixes this by putting in a check that will check to stop the service if it is accidentally restarted. Bug: 784602 Change-Id: Ic521fe39db62b3a0028ac3bed788f521cf0dcb44 Reviewed-on: https://chromium-review.googlesource.com/769869 Commit-Queue: Joy Ming <jming@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Cr-Commit-Position: refs/heads/master@{#516853} [modify] https://crrev.com/16b84d67c1349f534233cecefc332cf94387542a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java [modify] https://crrev.com/16b84d67c1349f534233cecefc332cf94387542a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java [modify] https://crrev.com/16b84d67c1349f534233cecefc332cf94387542a/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java
,
Nov 15 2017
,
Nov 15 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 16 2017
Please verify the fix in the next canary
,
Nov 16 2017
Link to video: https://drive.google.com/open?id=1IMm7pOOeU--ggsPCfjibjg_Dt32OWxhe. This change is not in the latest Canary yet, will verify tomorrow.
,
Nov 17 2017
Jming@ Thanks for providing the video to verify the issue. We don't have access to view the video, but I verified the issue as per your steps provided in the issue. Build: M64-64.0.3270.2 Device: Pixel Xl O device
,
Nov 17 2017
NOTE: This problem only happens when chrome://flags#enable-downloads-foreground is disabled. The flag should be disabled by default on Beta but enabled by default on Canary.
,
Nov 17 2017
,
Nov 17 2017
Merge approved! please verify the fix after merging on branch 3239.
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b67a9c84106a77022afe4bb77c44f5e4890fd153 commit b67a9c84106a77022afe4bb77c44f5e4890fd153 Author: Joy Ming <jming@chromium.org> Date: Fri Nov 17 23:26:26 2017 Fix bug where notification for paused download persists. There was a bug where the notification for a paused offline page would persist even after swiped away. This is because in some cases, the foreground download notification service is restarted after the download was paused but it does not get shut down again so downloads remains in the foreground. This CL fixes this by putting in a check that will check to stop the service if it is accidentally restarted. Bug: 784602 Change-Id: Ic521fe39db62b3a0028ac3bed788f521cf0dcb44 Reviewed-on: https://chromium-review.googlesource.com/769869 Commit-Queue: Joy Ming <jming@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#516853}(cherry picked from commit 16b84d67c1349f534233cecefc332cf94387542a) Reviewed-on: https://chromium-review.googlesource.com/777405 Reviewed-by: Min Qin <qinmin@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#534} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/b67a9c84106a77022afe4bb77c44f5e4890fd153/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java [modify] https://crrev.com/b67a9c84106a77022afe4bb77c44f5e4890fd153/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java [modify] https://crrev.com/b67a9c84106a77022afe4bb77c44f5e4890fd153/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java
,
Nov 20 2017
Please verify the fix and close the bug if everything looks good!
,
Nov 21 2017
Works as per expected behavior, Issue Verified in Pixel/O on 63.0.3239.60 Also verified M64-64.0.3270.2 as per C#10, hence closing the status. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by jming@chromium.org
, Nov 14 2017Labels: ReleaseBlock-Stable M-63
Owner: jming@chromium.org
Status: Assigned (was: Untriaged)