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

Issue 747563 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug


Sign in to add a comment

[Downloads Foreground Service] Refactor downloads to be a foreground service for all Android versions.

Project Member Reported by jming@chromium.org, Jul 21 2017

Issue description

Inclusive bug for process involved in refactoring downloads to be a foreground service for all Android versions.

Design doc: go/downloads-foreground
 

Comment 1 by jming@chromium.org, Jul 21 2017

Blockedon: 747567

Comment 2 by jming@chromium.org, Jul 21 2017

Blockedon: 747564

Comment 3 by jming@chromium.org, Jul 21 2017

Blockedon: 747569

Comment 4 by jming@chromium.org, Jul 21 2017

Blockedon: 747571

Comment 5 by jming@chromium.org, Jul 21 2017

Blockedon: 746692
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 25 2017

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

commit 05a91e2d034b4495852b5ed6be69ab933a921079
Author: Joy Ming <jming@chromium.org>
Date: Tue Jul 25 16:02:57 2017

Move the logic for notification creation to DownloadNotificationFactory.

This is the first step in a refactor to make downloads a foreground
service. This step moves the logic of notification creation from
DownloadNotificationService into the newly constructed
DownloadNotificationFactory through the passing of a DownloadUpdate
object. This intermediate step ensures that the notifications are still
being rendered correctly. Eventually, all of the notification logic
(creation, updating, and removal) will be managed by this new class.

Bug:  747564 , 747563 
Change-Id: Ie44a76de4d61f2a6ad1bac0e0fd05ce9d63c1816
Reviewed-on: https://chromium-review.googlesource.com/572038
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489321}
[modify] https://crrev.com/05a91e2d034b4495852b5ed6be69ab933a921079/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[add] https://crrev.com/05a91e2d034b4495852b5ed6be69ab933a921079/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java
[modify] https://crrev.com/05a91e2d034b4495852b5ed6be69ab933a921079/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[add] https://crrev.com/05a91e2d034b4495852b5ed6be69ab933a921079/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUpdate.java
[modify] https://crrev.com/05a91e2d034b4495852b5ed6be69ab933a921079/chrome/android/java_sources.gni

Comment 7 by jming@chromium.org, Jul 26 2017

Blockedon: 749323

Comment 8 by jming@chromium.org, Jul 27 2017

Blockedon: 749878

Comment 9 by jming@chromium.org, Jul 27 2017

Blockedon: 749880

Comment 10 by jming@chromium.org, Jul 27 2017

Blockedon: 749882

Comment 11 by jming@chromium.org, Jul 27 2017

Blockedon: 749883
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 28 2017

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

commit 399f968b1df1818493aa5b8b2031764a8ebccd4f
Author: Joy Ming <jming@chromium.org>
Date: Fri Jul 28 17:34:22 2017

Refactor non-progress download updates to not be batched.

Before this CL, all updates from native to Java (DownloadManagerService
to SystemDownloadNotifier) were batched and sent every second. However,
for the refactor that makes downloads a foreground service, immediate
updates are required for the cases that are not related to progress, or
where a download is paused, cancelled, completed, or interrupted. This
CL refactors the DownloadManagerService so that the updates are
immediate except for those related to download progress, which continue
to be batched.

Bug:  747567 , 747563 
Change-Id: I687fd42d14be8cda3b4ea4cd287220037e15b7a0
Reviewed-on: https://chromium-review.googlesource.com/585776
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490445}
[modify] https://crrev.com/399f968b1df1818493aa5b8b2031764a8ebccd4f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/399f968b1df1818493aa5b8b2031764a8ebccd4f/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java

Comment 13 by jming@chromium.org, Jul 28 2017

Blockedon: 750270
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 9 2017

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

commit fe416305b68801e7728dd8482fa309b62698199a
Author: Joy Ming <jming@chromium.org>
Date: Wed Aug 09 16:51:48 2017

Create DownloadBroadcastManager to handle communication to native.

This is part of a larger refactor to make downloads a foreground
service. This handles the propagation of notification interaction
to native. Before, the path was DownloadBroadcastReceiver->
DownloadNotificationService->native. This CL takes the logic
relating to spinning up and communicating to the native out of the
DownloadNotificationService and into DownloadBroadcastManager, making
the path DBR->DNS->DBM->native. The reason this still routes through
the DownloadNotificationService is because currently DNS still handles
some non-native logic (ie. updating the notifications). After the
refactor is completed, the notification interaction will no longer
have to be routed through the DownloadNotificationService and the
path will be DBR->DBM->native, where the DownloadBroadcastManager
will be a service that just spins up and communicates to native.

Bug:  747569 , 747563 
Change-Id: I27ff959b31cebba8f44fc2cc1cebfbeaf466cb64
Reviewed-on: https://chromium-review.googlesource.com/581870
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493025}
[add] https://crrev.com/fe416305b68801e7728dd8482fa309b62698199a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/fe416305b68801e7728dd8482fa309b62698199a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/fe416305b68801e7728dd8482fa309b62698199a/chrome/android/java_sources.gni
[modify] https://crrev.com/fe416305b68801e7728dd8482fa309b62698199a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java
[modify] https://crrev.com/fe416305b68801e7728dd8482fa309b62698199a/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 11 2017

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

commit cc46b05b55683ecf63128f6062811093fcf2ad79
Author: Joy Ming <jming@chromium.org>
Date: Fri Aug 11 00:41:59 2017

Create a class to start and stop the foreground service for downloads.

This is one of the steps in the refactor to make downloads a foreground
service. This step creates a separate class that manages starting and
stoping the foreground service. Currently, this is not being used by
any of the classes, but will eventually be integrated to be called
every time there is a change in a download (ie. a download has been
started, paused, resumed, completed, canceled, or deleted).

Bug:  747571 , 747563 
Change-Id: Ie9a7ee2950481b5c403e09ede535f763ef44d653
Reviewed-on: https://chromium-review.googlesource.com/572147
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493619}
[add] https://crrev.com/cc46b05b55683ecf63128f6062811093fcf2ad79/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[add] https://crrev.com/cc46b05b55683ecf63128f6062811093fcf2ad79/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/cc46b05b55683ecf63128f6062811093fcf2ad79/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java
[modify] https://crrev.com/cc46b05b55683ecf63128f6062811093fcf2ad79/chrome/android/java_sources.gni
[add] https://crrev.com/cc46b05b55683ecf63128f6062811093fcf2ad79/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManagerTest.java

Comment 16 by jming@chromium.org, Aug 15 2017

Blockedon: 755588
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 15 2017

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

commit 411fb85ffd1c2478c78e9f6e6105d4c88f38ad96
Author: Joy Ming <jming@chromium.org>
Date: Tue Aug 15 20:55:21 2017

Reroute notification interactions directly to DownloadBroadcastManager.

This is part of a larger refactor to make downloads a foreground
service. The DownloadBroadcastManager (DBM) is designed to handle all
interactions with the notifications that needed to be propagated to
native. Before this CL, notification interactions were received by the
DownloadBroadcastReceiver (DBR), sent to the DownloadNotificationService
(DNS), and then propagated to native by DBM. This CL makes the DBM a
service, allowing the notification interactions to directly start the 
DBM so that it no longer has to go through the DBR and DNS.

Bug:  749880 , 747563 
Change-Id: Iaef62292433a53e28366d796f7cfe9932e2fb28a
Reviewed-on: https://chromium-review.googlesource.com/611144
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494535}
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java
[modify] https://crrev.com/411fb85ffd1c2478c78e9f6e6105d4c88f38ad96/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java

Comment 18 by jming@chromium.org, Aug 15 2017

Blockedon: 755771

Comment 19 by jming@chromium.org, Aug 15 2017

Blockedon: 755778

Comment 20 by jming@chromium.org, Aug 15 2017

Blockedon: 755779

Comment 21 by jming@chromium.org, Aug 16 2017

Blockedon: 756090
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 17 2017

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

commit 387e5384fbb148ed35e6fa7480f78c226a1160ea
Author: Joy Ming <jming@chromium.org>
Date: Thu Aug 17 00:43:03 2017

Integrate DownloadForegroundServiceManager.

This is part of the larger refactor to make downloads a foreground
service. The DownloadForegroundServiceManager (DFSM) was created as a
a manager to keep track of which downloads are active/inactive and turn
the DownloadForegroundService (DFS) on/off accordingly. This CL
integrates the previously isolated DFSM/DFS combination so that updates
from native that go through the SystemDownloadNotifier (SDN) propagate
to the DownloadNotificationService (DNS), as it did before, but,
instead of starting the DNS as a service, it notifies the DFSM, which
starts/stops the DFS. This takes out a lot of the service queuing
logic out of the SDN and cleaning up the DNS so to make sure it is no
longer a service (removing summary notification, extends Service, etc.)

Bug:  749883 , 747563 , 755907 
Change-Id: I0186f3d3f26032f19153568173c01b5618ae8bae
Reviewed-on: https://chromium-review.googlesource.com/613671
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495015}
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[delete] https://crrev.com/5dd2e46656941c9bf9e7c7ff57cccea08691aabf/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/java_sources.gni
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManagerTest.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java
[modify] https://crrev.com/387e5384fbb148ed35e6fa7480f78c226a1160ea/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java
[delete] https://crrev.com/5dd2e46656941c9bf9e7c7ff57cccea08691aabf/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java

Comment 23 by jming@chromium.org, Aug 18 2017

Blockedon: 757004

Comment 24 by jming@chromium.org, Aug 22 2017

Blockedon: 757922
Project Member

Comment 25 by bugdroid1@chromium.org, Aug 23 2017

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

commit a7032fcd09efd6a70914bd946f4129f2231d4ebe
Author: Joy Ming <jming@chromium.org>
Date: Wed Aug 23 22:25:54 2017

Create getInstance for DownloadNotificationService.

Before this CL, instantiating the DownloadNotificationService required
creating a new one each time, even though it was keeping state (through
the DownloadForegroundServiceManager and mDownloadsInProgress). This CL
fixes this problem by making the getInstance method public instead.

Bug:  747563 
Change-Id: I143248923b2d41a54c5eb051de1c8059b806050c
Reviewed-on: https://chromium-review.googlesource.com/627258
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496832}
[modify] https://crrev.com/a7032fcd09efd6a70914bd946f4129f2231d4ebe/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/a7032fcd09efd6a70914bd946f4129f2231d4ebe/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/a7032fcd09efd6a70914bd946f4129f2231d4ebe/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler.java
[modify] https://crrev.com/a7032fcd09efd6a70914bd946f4129f2231d4ebe/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 23 2017

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

commit fd660e795c10f879072e734a7a7e1e2aaaef7a6f
Author: Joy Ming <jming@chromium.org>
Date: Wed Aug 23 22:51:52 2017

Fix bug to open downloads home for in-progress download.

There was a bug where clicking on the notification for an in-progress
download (in-progress, paused) would not open downloads home. This CL
fixes that bug and enables the intended behavior.

Bug:  747563 
Change-Id: I38eba063cd26764f18dfd11ed5b30a707fbfe9ed
Reviewed-on: https://chromium-review.googlesource.com/627182
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496843}
[modify] https://crrev.com/fd660e795c10f879072e734a7a7e1e2aaaef7a6f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 24 2017

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

commit db5b34d2ad8a7e934c083bbc14e350924dbc00cf
Author: Joy Ming <jming@chromium.org>
Date: Thu Aug 24 17:12:39 2017

Update download notification immediately after interaction.

Because of the refactor that makes downloads a foreground service,
interactions with download notifications were being propagated straight
to native by DownloadBroadcastManager before manifesting in the
notifications through SystemDownloadNotifier. However, if native takes
too long to load, there can be a delay between the user action and the
notification update. This CL bridges that gap by propagating
notification updates immediately in certain cases (pause, resume,
cancel) in order to give feedback to the user.

Bug:  755588 , 747563 
Change-Id: Ie078f33189cc779825e3fc7216694db412be8eb7
Reviewed-on: https://chromium-review.googlesource.com/621786
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497096}
[modify] https://crrev.com/db5b34d2ad8a7e934c083bbc14e350924dbc00cf/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/db5b34d2ad8a7e934c083bbc14e350924dbc00cf/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/db5b34d2ad8a7e934c083bbc14e350924dbc00cf/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/db5b34d2ad8a7e934c083bbc14e350924dbc00cf/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java
[modify] https://crrev.com/db5b34d2ad8a7e934c083bbc14e350924dbc00cf/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java

Comment 28 by jming@chromium.org, Aug 24 2017

Blockedon: 758688

Comment 29 by jming@chromium.org, Aug 25 2017

Blockedon: 759170

Comment 30 by jming@chromium.org, Aug 25 2017

Blockedon: 759182
Project Member

Comment 31 by bugdroid1@chromium.org, Aug 25 2017

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

commit 3c17501986c6d37bebadbde40f82909b5b5a2d8a
Author: Joy Ming <jming@chromium.org>
Date: Fri Aug 25 23:15:01 2017

Add DownloadsForegroundService in AndroidManifest.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. Before this CL, the Downloads
Foreground Service was not actually activated because its inclusion in
the Android Manifest was accidentally omitted. This CL activates the
Downloads Foreground Service in addition to solving a few of the bugs
that arose while this omission was corrected, namely the double
notification that was displayed because of the use of a notification
tag (filed a bug with the Android notifications team b/65052774) as
well as the logic behind the killing of a cancelled notification.

Bug:  747563 ,758688
Change-Id: Iae27a9e58384d515ed400403c93633facee2e3c0
Reviewed-on: https://chromium-review.googlesource.com/633836
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497587}
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/3c17501986c6d37bebadbde40f82909b5b5a2d8a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManagerTest.java

Comment 32 by jming@chromium.org, Aug 26 2017

Blockedon: 759258
Project Member

Comment 33 by bugdroid1@chromium.org, Aug 30 2017

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

commit 94298d7de8ab29bc0d95970137e4c52886ca1771
Author: Shakti Sahu <shaktisahu@chromium.org>
Date: Wed Aug 30 02:24:38 2017

Separate all refactor code for downloads as a foreground service.

The refactor that makes downloads a foreground service for all versions
of Android is still in progress. In order to maintain stability for M62
branch point, this CL separates all changes related to this refactor
into repeated classes (ie. DownloadsNotificationService2) or new classes
that will not affect the stability of M62. 

The versions without the numbers (ie. DownloadsNotificationService)
should be the "old" version, before the downloads as a foreground
refactor (approx commit 16e22136cb71398b57c282ac4a75ef0de834eac4). The
versions with the numbers (ie. DownloadNotificationService2) is a
snapshot of the most current state in the downloads foreground refactor
(approx commit db5b34d2ad8a7e934c083bbc14e350924dbc00cf). There is
currently no branch point between the two versions.

Bug:  747563 
Change-Id: Ie552e81fa9e04217d03730e62b62433df61ba26d
Reviewed-on: https://chromium-review.googlesource.com/633869
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498347}
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler2.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier2.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/java_sources.gni
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest2.java
[modify] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java
[add] https://crrev.com/94298d7de8ab29bc0d95970137e4c52886ca1771/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService2.java

Project Member

Comment 34 by bugdroid1@chromium.org, Sep 7 2017

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

commit 69d77cd0595f6f01eeeefcda245102a0b73e2206
Author: Joy Ming <jming@chromium.org>
Date: Thu Sep 07 18:50:20 2017

Command line switches to enable download foreground service.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. A previous change split out the
refactored code from the existing code. This change allows the
refactored code to be access through a command line switch in order to
allow for continued development while maintaining a stable version.

Bug:  747563 
Change-Id: I3d7c5e790bce13025fbcced5c4eb14fd101a2212
Reviewed-on: https://chromium-review.googlesource.com/651728
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500347}
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler.java
[delete] https://crrev.com/dff81508a1dd7732d333b8bc5f5c79b0a938d3ff/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler2.java
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/69d77cd0595f6f01eeeefcda245102a0b73e2206/chrome/android/java_sources.gni

Project Member

Comment 35 by bugdroid1@chromium.org, Sep 11 2017

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

commit 2e01255a4b3edd8d0d7f5e3d69cc8ea86026de79
Author: Joy Ming <jming@chromium.org>
Date: Mon Sep 11 22:07:48 2017

Re-launch completed notification during foreground switch.

This is part of the larger refactor to make downloads a foreground
service on all versions of Android. Before this CL, when a download was
completed and another download was in progress, the completed
notification would disappear. This was due to the fact that calling
startForeground on a different notification when there was already a
notification pinned to the foreground would cause the original
notification to be cancelled. This is fixed in later versions of Android
but for older versions, a hack that re-launches the notification during
the foreground switch is required.

Bug:  747563 , 759170 
Change-Id: I171ffa086d3d3c64843bfa3ae9b2737999b75520
Reviewed-on: https://chromium-review.googlesource.com/657242
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501058}
[modify] https://crrev.com/2e01255a4b3edd8d0d7f5e3d69cc8ea86026de79/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/2e01255a4b3edd8d0d7f5e3d69cc8ea86026de79/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java

Project Member

Comment 36 by bugdroid1@chromium.org, Sep 11 2017

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

commit 5f44520f4d33789fe8298786b858e35fcaad2156
Author: Joy Ming <jming@chromium.org>
Date: Mon Sep 11 23:16:07 2017

Make sure cancelDownload notifies observers.

This is part of a larger refactor to make downloads a foreground
service for all versions of Android. Before this CL, cancelling a
download that is not offline pages would not cancel the notification.
This was because the SystemDownloadNotifier was not being informed of a
cancelled download. Cancelling a download through DownloadManagerService
removes the observer and does not inform it of a cancelled download.
This CL circumvents the removal of the observers by calling
onDownloadCancelled at the moment the download is cancelled, which in
turn informs the SystemDownloadNotifier of the cancelled download and
allows for the stopping of the foreground service and removal of the
notification.

Bug:  747563 
Change-Id: I5f1f91834e1ce94f3cdf42e9965035618327df30
Reviewed-on: https://chromium-review.googlesource.com/660996
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501086}
[modify] https://crrev.com/5f44520f4d33789fe8298786b858e35fcaad2156/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

Project Member

Comment 37 by bugdroid1@chromium.org, Sep 12 2017

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

commit 5467a02ec5d072bb9d8c28259930f0cf9fbe7bce
Author: Joy Ming <jming@chromium.org>
Date: Tue Sep 12 23:26:48 2017

Fix issue of not being able to swipe away paused downloads.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. There was a bug that made it so
that paused downloads were not swipeable (and therefore not
cancellable). This CL fixes that bug.

Bug:  747563 
Change-Id: If67d34523e0352aaf3b8f5c78967fd1ec3a697a7
Reviewed-on: https://chromium-review.googlesource.com/661940
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501450}
[modify] https://crrev.com/5467a02ec5d072bb9d8c28259930f0cf9fbe7bce/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/5467a02ec5d072bb9d8c28259930f0cf9fbe7bce/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/5467a02ec5d072bb9d8c28259930f0cf9fbe7bce/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java

Comment 38 by jming@chromium.org, Sep 13 2017

Blockedon: 764939

Comment 39 by jming@chromium.org, Sep 14 2017

Blockedon: 765327
Project Member

Comment 40 by bugdroid1@chromium.org, Sep 20 2017

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

commit ec551c676d717ae37a2130f729dfe6ad9f03ac49
Author: Joy Ming <jming@chromium.org>
Date: Wed Sep 20 21:25:19 2017

Reimplement downloads resumption logic for foreground service.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. During the refactor, the logic for
resumption/crashes was temporarily removed (ie. onDestroy, START_STICKY,
DownloadResumptionScheduler, etc). This CL reimplements the necessarily
logic for unexpected shutdowns and restarts.

Bug:  755771 , 747563 
Change-Id: I7e1c2d0eef6b8765a0636d0d3f5181eb56c892f5
Reviewed-on: https://chromium-review.googlesource.com/658489
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503251}
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[add] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceObservers.java
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[add] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationServiceObserver.java
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/java_sources.gni
[modify] https://crrev.com/ec551c676d717ae37a2130f729dfe6ad9f03ac49/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest2.java

Project Member

Comment 41 by bugdroid1@chromium.org, Oct 2 2017

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

commit 5fd29ca600969c4098e2cf7a554a8ae84647ea1b
Author: Joy Ming <jming@chromium.org>
Date: Mon Oct 02 22:51:22 2017

chrome://flags for downloads foreground.

Add flag in chrome://flags that allows the turning on and off of
downloads a foreground service for all versions of Android.

Bug:  747563 
Change-Id: I27a0495c93560f0a8915162228b9d51d5420e060
Reviewed-on: https://chromium-review.googlesource.com/679607
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505830}
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadResumptionScheduler.java
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/browser/about_flags.cc
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/common/chrome_features.cc
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/chrome/common/chrome_features.h
[modify] https://crrev.com/5fd29ca600969c4098e2cf7a554a8ae84647ea1b/tools/metrics/histograms/enums.xml

Project Member

Comment 42 by bugdroid1@chromium.org, Oct 4 2017

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

commit ebe094c88d60ae2589c479159ce2611cc92c54cc
Author: Joy Ming <jming@chromium.org>
Date: Wed Oct 04 21:19:11 2017

Fix crash from NullPointerError in downloads foreground.

During the refactor to make downloads a foreground service on all
versions of Android, a problem arose where Chrome crashes when
canceling a file that is downloading. This CL fixes that issue.

Bug: 771122, 747563 
Change-Id: Idaa2377ef06df1d30b6947e92b6743ad5e4b16c2
Reviewed-on: https://chromium-review.googlesource.com/700774
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506519}
[modify] https://crrev.com/ebe094c88d60ae2589c479159ce2611cc92c54cc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/ebe094c88d60ae2589c479159ce2611cc92c54cc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

Project Member

Comment 43 by bugdroid1@chromium.org, Oct 12 2017

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

commit 316efc2f951ae4c314565a1cd7c56a29c91a4af9
Author: Joy Ming <jming@chromium.org>
Date: Thu Oct 12 23:19:25 2017

Disable downloads foreground in chrome://flags temporarily.

Disable feature "enable downloads foreground" by default in
chrome://flags for M63 branch point. Flip on later.

Bug:  747563 
Change-Id: I39aedcb14120618c24e9b7f7fed820244a7c4da0
Reviewed-on: https://chromium-review.googlesource.com/717257
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508529}
[modify] https://crrev.com/316efc2f951ae4c314565a1cd7c56a29c91a4af9/chrome/common/chrome_features.cc

Comment 44 by jming@chromium.org, Oct 17 2017

Blockedon: 775613
Project Member

Comment 45 by bugdroid1@chromium.org, Oct 17 2017

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

commit 7748cea2ab5767bef12997c2a5225b566df02d96
Author: Joy Ming <jming@chromium.org>
Date: Tue Oct 17 18:54:03 2017

Update UMA tracking for downloads notifications.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. During the refactor, a few
tradeoffs had to be made (ie. optimizing for single download or
multiple). These metrics can give light into how to make such tradeoffs
in the future, specifically around: notification interactions, number
of notifications (as a proxy for multiple downloads), service
lifecycle, unexpected/expected service stops, etc.

Bug:  747563 
Change-Id: I90ab96c863bfaa77ded0d09c74277f10795dfdf1
Reviewed-on: https://chromium-review.googlesource.com/710579
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509466}
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[add] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationUmaHelper.java
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/chrome/android/java_sources.gni
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/7748cea2ab5767bef12997c2a5225b566df02d96/tools/metrics/histograms/histograms.xml

Project Member

Comment 46 by bugdroid1@chromium.org, Oct 24 2017

Labels: merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f

commit 6d1edeb554fe1d62a5d513f9cf2844d3a025da7f
Author: Joy Ming <jming@chromium.org>
Date: Tue Oct 24 21:47:38 2017

Update UMA tracking for downloads notifications.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. During the refactor, a few
tradeoffs had to be made (ie. optimizing for single download or
multiple). These metrics can give light into how to make such tradeoffs
in the future, specifically around: notification interactions, number
of notifications (as a proxy for multiple downloads), service
lifecycle, unexpected/expected service stops, etc.

TBR=isherman@chromium.org

Bug:  747563 
Change-Id: I90ab96c863bfaa77ded0d09c74277f10795dfdf1
Reviewed-on: https://chromium-review.googlesource.com/710579
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509466}(cherry picked from commit 7748cea2ab5767bef12997c2a5225b566df02d96)
Reviewed-on: https://chromium-review.googlesource.com/734729
Cr-Commit-Position: refs/branch-heads/3239@{#194}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[add] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationUmaHelper.java
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/chrome/android/java_sources.gni
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/6d1edeb554fe1d62a5d513f9cf2844d3a025da7f/tools/metrics/histograms/histograms.xml

Project Member

Comment 47 by bugdroid1@chromium.org, Oct 26 2017

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

commit 431b1802ce6bbce1994a8b67f173530cba914b9b
Author: Joy Ming <jming@chromium.org>
Date: Thu Oct 26 00:22:58 2017

Make flicker less frequent.

This is part of a larger refactor to make downloads a foreground
service on all versions of Android. Before this CL, on Android API <= 24
the downloads notification would flicker on pause and on completion as a
hack to disassociate the notification from the foreground service.
However, this leads to a lot of flickering. This CL reduces the amount
of flickering to be only in the cases of completion and when the
notification that is pinned to the foreground is switched. Basically,
this means that the notification will flicker in all cases of completion
(which is the same as before) and fewer cases of pause, only when the
user has another download running, which is less likely anyways.

Bug:  771802 , 747563 
Change-Id: Ic0d30cb7d91726a7015a6db9b0dc6e113140fd54
Reviewed-on: https://chromium-review.googlesource.com/703486
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511660}
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceObservers.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationServiceObserver.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier2.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/java_sources.gni
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManagerTest.java
[add] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceTest.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest2.java
[modify] https://crrev.com/431b1802ce6bbce1994a8b67f173530cba914b9b/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService2.java

Project Member

Comment 48 by bugdroid1@chromium.org, Nov 10 2017

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

commit fdb9cfcc89a745e1cbb3518ff7c16aaf8346d503
Author: Joy Ming <jming@chromium.org>
Date: Fri Nov 10 23:38:59 2017

Enable downloads foreground in chrome://flags.

Enable feature "enable downloads foreground" by default in
chrome://flags to test for M64.

Bug:  747563 
Change-Id: I34a1cdd85a89864fd495fc7d403cbcfbb0d54d0b
Reviewed-on: https://chromium-review.googlesource.com/758622
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515743}
[modify] https://crrev.com/fdb9cfcc89a745e1cbb3518ff7c16aaf8346d503/chrome/common/chrome_features.cc

Project Member

Comment 49 by bugdroid1@chromium.org, Jan 16 2018

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

commit 84798b4d2ab267af5e6f5a40a27efcf7b3e53033
Author: Joy Ming <jming@chromium.org>
Date: Tue Jan 16 00:40:22 2018

[Downloads foreground] Disable downloads foreground for M64.

Bug: 800637, 747563 
Change-Id: I54590daf0caba01abf0880f0159c6d48ba727359
Reviewed-on: https://chromium-review.googlesource.com/865818
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529349}
[modify] https://crrev.com/84798b4d2ab267af5e6f5a40a27efcf7b3e53033/chrome/common/chrome_features.cc

Project Member

Comment 50 by bugdroid1@chromium.org, Jan 18 2018

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ed29c63f4ba27b8e13eecd1b4353bf003922289

commit 2ed29c63f4ba27b8e13eecd1b4353bf003922289
Author: Joy Ming <jming@chromium.org>
Date: Thu Jan 18 00:27:44 2018

[Downloads foreground] Disable downloads foreground for M64.

Bug: 800637, 747563 
Change-Id: I54590daf0caba01abf0880f0159c6d48ba727359
Reviewed-on: https://chromium-review.googlesource.com/865818
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Joy Ming <jming@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#529349}(cherry picked from commit 84798b4d2ab267af5e6f5a40a27efcf7b3e53033)
Reviewed-on: https://chromium-review.googlesource.com/872150
Reviewed-by: Joy Ming <jming@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#529}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/2ed29c63f4ba27b8e13eecd1b4353bf003922289/chrome/common/chrome_features.cc

Project Member

Comment 51 by bugdroid1@chromium.org, Jan 22 2018

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

commit 351166caac171b54719b55be36a7f93768a75d5a
Author: Joy Ming <jming@chromium.org>
Date: Mon Jan 22 22:42:40 2018

[Downloads notification] Fix bug <= L devices for disappearing complete.

Before this CL, there was a bug on L and lower devices (pre-Marshmallow)
where the notification for the completed download would disappear if
Chrome was swiped away from the recent activities drawer. This is
because on pre-Marshmallow devices, swiping away an activity means
killing the process immediately after it completes, which does not allow
us to properly deal with the notification and it dies with the service.
Even though we have backup measures for downloads in other states (ie.
in-progress or paused) with the service START_STICKY, but since we do
not persist information about completed or failed downloads, we are
unable to recreate the notifications properly. Therefore, in this CL
we fix that bug by pre-emptively launching a notification with a new
notification id for all completed or failed downloads in phones that
are L or below before calling stopForeground.

For more information, see the writeup at go/downloads-on-l-devices.

Bug:  747563 
Change-Id: I079ba924cfa4cb48304e5d94813225ea3eccd8b9
Reviewed-on: https://chromium-review.googlesource.com/869138
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531019}
[modify] https://crrev.com/351166caac171b54719b55be36a7f93768a75d5a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
[modify] https://crrev.com/351166caac171b54719b55be36a7f93768a75d5a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/351166caac171b54719b55be36a7f93768a75d5a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManagerTest.java

Comment 52 by jming@chromium.org, Apr 25 2018

Status: Fixed (was: Assigned)

Comment 53 by jming@chromium.org, Apr 26 2018

Blockedon: -765327

Sign in to add a comment