Closes #9789 close the download stream when dismissing the dialog

components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt

@@ -20,6 +20,7 @@ import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.mapNotNull
 import mozilla.components.browser.state.selector.findCustomTabOrSelectedTab
+import mozilla.components.browser.state.selector.findTabOrCustomTab
 import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
 import mozilla.components.browser.state.state.SessionState
 import mozilla.components.browser.state.state.content.DownloadState
@@ -111,6 +112,7 @@ class DownloadsFeature(
                             previousTab?.let { tab ->
                                 // We have an old download request.
                                 tab.content.download?.let { download ->
+                                    closeDownloadResponse(tab.id)
                                     dismissAllDownloadDialogs()
                                     useCases.consumeDownload(tab.id, download.id)
                                     previousTab = null
@@ -209,6 +211,7 @@ class DownloadsFeature(
                     processDownload(tab, download)
                 }
             } else {
+                closeDownloadResponse(tab.id)
                 useCases.consumeDownload(tab.id, download.id)
             }
         }
@@ -239,6 +242,7 @@ class DownloadsFeature(
         }
 
         dialog.onCancelDownload = {
+            closeDownloadResponse(tab.id)
             useCases.consumeDownload.invoke(tab.id, download.id)
         }
 
@@ -284,6 +288,7 @@ class DownloadsFeature(
         }
 
         appChooserDialog.onDismiss = {
+            closeDownloadResponse(tab.id)
             useCases.consumeDownload.invoke(tab.id, download.id)
         }
 
@@ -303,6 +308,12 @@ class DownloadsFeature(
         return findPreviousAppDownloaderDialogFragment() != null
     }
 
+    internal fun closeDownloadResponse(tabId: String) {
+        store.state.findTabOrCustomTab(tabId)?.let {
+            it.content.download?.response?.close()
+        }
+    }
+
     private fun findPreviousAppDownloaderDialogFragment(): DownloadAppChooserDialog? {
         return fragmentManager?.findFragmentByTag(DownloadAppChooserDialog.FRAGMENT_TAG) as? DownloadAppChooserDialog
     }

components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt

@@ -27,6 +27,7 @@ import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.content.DownloadState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.concept.fetch.Response
 import mozilla.components.feature.downloads.DownloadsUseCases.ConsumeDownloadUseCase
 import mozilla.components.feature.downloads.manager.DownloadManager
 import mozilla.components.feature.downloads.ui.DownloadAppChooserDialog
@@ -267,6 +268,36 @@ class DownloadsFeatureTest {
         verify(dialogFragment, never()).showNow(any(), any())
     }
 
+    @Test
+    fun `WHEN dismissing a download dialog THEN the download stream should be closed`() {
+        val downloadsUseCases = spy(DownloadsUseCases(store))
+        val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>()
+        val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab")
+        val dialogFragment = spy(object : DownloadDialogFragment() {})
+        val fragmentManager: FragmentManager = mock()
+
+        doReturn(dialogFragment).`when`(fragmentManager).findFragmentByTag(DownloadDialogFragment.FRAGMENT_TAG)
+        store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download))
+                .joinBlocking()
+        doReturn(consumeDownloadUseCase).`when`(downloadsUseCases).consumeDownload
+
+        val feature = spy(DownloadsFeature(
+            testContext,
+            store,
+            useCases = downloadsUseCases,
+            downloadManager = mock(),
+            fragmentManager = fragmentManager
+        ))
+
+        val tab = store.state.findTab("test-tab")
+
+        feature.showDownloadDialog(tab!!, download)
+
+        dialogFragment.onCancelDownload()
+        verify(feature).closeDownloadResponse(tab.id)
+        verify(consumeDownloadUseCase).invoke(anyString(), anyString())
+    }
+
     @Test
     fun `onPermissionsResult will start download if permissions were granted and thirdParty enabled`() {
         val downloadsUseCases = spy(DownloadsUseCases(store))
@@ -351,12 +382,12 @@ class DownloadsFeatureTest {
             arrayOf(INTERNET, WRITE_EXTERNAL_STORAGE)
         ).`when`(downloadManager).permissions
 
-        val feature = DownloadsFeature(
+        val feature = spy(DownloadsFeature(
             testContext,
             store,
             useCases = DownloadsUseCases(store),
             downloadManager = downloadManager
-        )
+        ))
 
         feature.start()
 
@@ -371,6 +402,7 @@ class DownloadsFeatureTest {
         assertNull(store.state.findTab("test-tab")!!.content.download)
 
         verify(downloadManager, never()).download(any(), anyString())
+        verify(feature).closeDownloadResponse("test-tab")
     }
 
     @Test
@@ -674,6 +706,58 @@ class DownloadsFeatureTest {
         verify(dialog).showNow(fragmentManager, DownloadAppChooserDialog.FRAGMENT_TAG)
     }
 
+    @Test
+    fun `WHEN dismissing a downloader app dialog THEN the download stream should be closed`() {
+        val downloadsUseCases = spy(DownloadsUseCases(store))
+        val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>()
+        val tab = createTab("https://www.mozilla.org", id = "test-tab")
+        val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab")
+        val ourApp = mock<DownloaderApp>()
+        val anotherApp = mock<DownloaderApp>()
+        val apps = listOf(ourApp, anotherApp)
+        val dialog = spy(DownloadAppChooserDialog())
+        val fragmentManager: FragmentManager = mockFragmentManager()
+        val feature = spy(DownloadsFeature(
+            testContext,
+            store,
+            downloadsUseCases,
+            downloadManager = mock(),
+            shouldForwardToThirdParties = { true },
+            fragmentManager = fragmentManager
+        ))
+
+        doReturn(consumeDownloadUseCase).`when`(downloadsUseCases).consumeDownload
+
+        feature.showAppDownloaderDialog(tab, download, apps, dialog)
+        dialog.onDismiss()
+
+        verify(feature).closeDownloadResponse(tab.id)
+        verify(consumeDownloadUseCase).invoke(anyString(), anyString())
+    }
+
+    @Test
+    fun `WHEN calling closeDownloadResponse THEN the response must be closed`() {
+        val downloadsUseCases = spy(DownloadsUseCases(store))
+        val tab = createTab("https://www.mozilla.org")
+        val response = mock<Response>()
+        val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = tab.id, response = response)
+
+        val feature = spy(DownloadsFeature(
+            testContext,
+            store,
+            downloadsUseCases,
+            downloadManager = mock(),
+            shouldForwardToThirdParties = { true },
+            fragmentManager = mock()
+        ))
+
+        store.dispatch(TabListAction.AddTabAction(tab, select = true)).joinBlocking()
+        store.dispatch(ContentAction.UpdateDownloadAction(tab.id, download = download)).joinBlocking()
+
+        feature.closeDownloadResponse(tab.id)
+        verify(response).close()
+    }
+
     @Test
     fun `when isAlreadyAppDownloaderDialog we must NOT show the appChooserDialog`() {
         val tab = createTab("https://www.mozilla.org", id = "test-tab")
@@ -836,6 +920,7 @@ class DownloadsFeatureTest {
         dialog.onDismiss()
 
         verify(consumeDownloadUseCase).invoke(anyString(), anyString())
+        verify(feature).closeDownloadResponse(tab.id)
     }
 
     @Test
@@ -890,6 +975,7 @@ class DownloadsFeatureTest {
         store.dispatch(TabListAction.AddTabAction(tab, select = true)).joinBlocking()
 
         verify(feature).dismissAllDownloadDialogs()
+        verify(feature).closeDownloadResponse(any())
         verify(downloadsUseCases).consumeDownload
         assertNull(feature.previousTab)
     }
@@ -925,6 +1011,7 @@ class DownloadsFeatureTest {
         store.dispatch(TabListAction.AddTabAction(tab, select = true)).joinBlocking()
 
         verify(feature, never()).dismissAllDownloadDialogs()
+        verify(feature, never()).closeDownloadResponse(tab.id)
         verify(downloadsUseCases, never()).consumeDownload
         assertNotNull(feature.previousTab)
     }

docs/changelog.md

@@ -27,6 +27,7 @@ permalink: /changelog/
 * **feature-downloads**:
   * 🌟 New `ShareDownloadFeature` will listen for `AddShareAction` and download, cache locally and then share internet resources.
   * ⚠️ **This is a breaking change**: This is a breaking change with clients expected to create and register a new instance of the this new feature otherwise the "Share image" from the browser contextual menu will do nothing.
+  * 🚒 Bug fixed [issue #9789](https://github.com/mozilla-mobile/android-components/issues/9789) - Canceled first PDF download prevents following attempts from downloading.
 
 * **support-ktx**
   * 🌟 Added `Context.shareMedia` that allows to easily share a specific locally stored file through the Android share menu.