Close mozilla-mobile#8857 call stopForeground when all downloads are …

…completed
Amejia481 · Oct 30, 2020 · 336898e · 336898e
1 parent 3e7cee0
commit 336898e
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 0 deletions.
diff --git a/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
@@ -588,6 +588,13 @@ abstract class AbstractFetchDownloadService : Service() {
             // This device supports notification groups, we just need to update the summary notification
             updateNotificationGroup()
         }
+        // If all downloads have been completed we don't need the status of
+        // foreground service anymore, we can call stopForeground and let the user
+        // swipe the foreground notification.
+        val finishDownloading = downloadJobs.values.toList().all { it.status == COMPLETED }
+        if (finishDownloading) {
+            stopForeground(false)
+        }
     }
 
     @Suppress("ComplexCondition")

diff --git a/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt
@@ -31,6 +31,7 @@ import mozilla.components.browser.state.state.content.DownloadState
 import mozilla.components.browser.state.state.content.DownloadState.Status.DOWNLOADING
 import mozilla.components.browser.state.state.content.DownloadState.Status.FAILED
 import mozilla.components.browser.state.state.content.DownloadState.Status.INITIATED
+import mozilla.components.browser.state.state.content.DownloadState.Status.COMPLETED
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.fetch.Client
 import mozilla.components.concept.fetch.MutableHeaders
@@ -773,6 +774,58 @@ class AbstractFetchDownloadServiceTest {
         verify(service).removeNotification(testContext, downloadState)
     }
 
+    @Test
+    fun `WHEN all downloads are completed stopForeground must be called`() {
+        val download1 = DownloadState(id = "1", url = "https://example.com/file1.txt", fileName = "file1.txt",
+                status = COMPLETED)
+        val download2 = DownloadState(id = "2", url = "https://example.com/file2.txt", fileName = "file2.txt",
+                status = COMPLETED)
+        val downloadState1 = DownloadJobState(
+            state = download1,
+            status = COMPLETED,
+            foregroundServiceId = Random.nextInt()
+        )
+
+        val downloadState2 = DownloadJobState(
+            state = download2,
+            status = COMPLETED,
+            foregroundServiceId = Random.nextInt()
+        )
+
+        service.downloadJobs["1"] = downloadState1
+        service.downloadJobs["2"] = downloadState2
+
+        service.updateForegroundNotificationIfNeeded(downloadState1)
+
+        verify(service).stopForeground(false)
+    }
+
+    @Test
+    fun `Until all downloads are NOT completed stopForeground must NOT be called`() {
+        val download1 = DownloadState(id = "1", url = "https://example.com/file1.txt", fileName = "file1.txt",
+                status = COMPLETED)
+        val download2 = DownloadState(id = "2", url = "https://example.com/file2.txt", fileName = "file2.txt",
+                status = DOWNLOADING)
+        val downloadState1 = DownloadJobState(
+            state = download1,
+            status = COMPLETED,
+            foregroundServiceId = Random.nextInt()
+        )
+
+        val downloadState2 = DownloadJobState(
+            state = download2,
+            status = DOWNLOADING,
+            foregroundServiceId = Random.nextInt()
+        )
+
+        service.downloadJobs["1"] = downloadState1
+        service.downloadJobs["2"] = downloadState2
+
+        service.updateForegroundNotificationIfNeeded(downloadState1)
+
+        verify(service, never()).stopForeground(false)
+    }
+
     @Test
     fun `removeDownloadJob will stop the service if there are none pending downloads`() {
         val download = DownloadState(id = "1", url = "https://example.com/file.txt", fileName = "file.txt",

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -44,6 +44,7 @@ permalink: /changelog/
 * **feature-downloads**
   * 🚒 Bug fixed [issue #8585](https://github.com/mozilla-mobile/android-components/issues/8784) create download directory when it doesn't exists for more information see [mozilla-mobile/fenix#15527](https://github.com/mozilla-mobile/fenix/issues/5829).
   * 🚒 Bug fixed [issue #8847](https://github.com/mozilla-mobile/android-components/issues/8847) crash when trying to download a file and switching from a normal to a private mode.
+  * 🚒 Bug fixed [issue #8857](https://github.com/mozilla-mobile/android-components/issues/8857)  Sometimes it is not possible to dismiss download notifications see [Fenix#15527](https://github.com/mozilla-mobile/fenix/issues/15527) for more information.
 
 * **feature-top-sites**
   * Added `RenameTopSiteUseCase` to rename pinned site entries. [#8751](https://github.com/mozilla-mobile/android-components/issues/8751)