New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 784602 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[downloads] notification persists for paused download

Project Member Reported by jming@chromium.org, Nov 13 2017

Issue description

Chrome 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.

 

Comment 1 by jming@chromium.org, Nov 14 2017

Cc: dtrainor@chromium.org dim...@chromium.org
Labels: ReleaseBlock-Stable M-63
Owner: jming@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by jming@chromium.org, 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.
Labels: Needs-Feedback
jming@, could you please add the OS for this bug ?

Comment 4 by jming@chromium.org, Nov 14 2017

Labels: OS-Android
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Comment 6 by jming@chromium.org, Nov 15 2017

Labels: Merge-Request-63
Project Member

Comment 7 by sheriffbot@chromium.org, Nov 15 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
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

Comment 8 by cma...@chromium.org, Nov 16 2017

Please verify the fix in the next canary

Comment 9 by jming@chromium.org, 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.

Comment 10 by kravula@google.com, 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

Comment 11 by jming@chromium.org, 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.
Labels: config-specific

Comment 13 by cmasso@google.com, Nov 17 2017

Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Merge approved! please verify the fix after merging on branch 3239.
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 17 2017

Labels: -merge-approved-63 merge-merged-3239
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

Please verify the fix and close the bug if everything looks good!
Status: Verified (was: Assigned)
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