Suppress download dialog if user already confirmed download with a different dialog

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

@@ -800,7 +800,7 @@ class BrowserTabViewModelTest {
     }
 
     @Test
-    fun whenUserSelectsDownloadImageOptionFromContextMenuThenDownloadFileCommandIssued() {
+    fun whenUserSelectsDownloadImageOptionFromContextMenuThenDownloadCommandIssuedWithoutRequirementForFurtherUserConfirmation() {
         whenever(mockLongPressHandler.userSelectedMenuItem(any(), any()))
             .thenReturn(DownloadFile("example.com"))
 
@@ -812,6 +812,7 @@ class BrowserTabViewModelTest {
 
         val lastCommand = commandCaptor.lastValue as Command.DownloadImage
         assertEquals("example.com", lastCommand.url)
+        assertFalse(lastCommand.requestUserConfirmation)
     }
 
     @Test

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

@@ -93,9 +93,8 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.OmnibarViewState
 import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter
 import com.duckduckgo.app.browser.downloader.FileDownloadNotificationManager
 import com.duckduckgo.app.browser.downloader.FileDownloader
+import com.duckduckgo.app.browser.downloader.FileDownloader.FileDownloadListener
 import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload
-import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.DownloadFileData
-import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.UserDownloadAction
 import com.duckduckgo.app.browser.filechooser.FileChooserIntentBuilder
 import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials
 import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
@@ -296,6 +295,14 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         renderer = BrowserTabFragmentRenderer()
+        if(savedInstanceState != null) {
+            updateFragmentListener()
+        }
+    }
+
+    private fun updateFragmentListener() {
+        val fragment = fragmentManager?.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG) as? DownloadConfirmationFragment
+        fragment?.downloadListener = createDownloadListener()
     }
 
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
@@ -387,9 +394,15 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
     override fun onPause() {
         daxDialog = null
         logoHidingListener.onPause()
+        dismissDownloadFragment()
         super.onPause()
     }
 
+    private fun dismissDownloadFragment() {
+        val fragment = fragmentManager?.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG) as? DownloadConfirmationFragment
+        fragment?.dismiss()
+    }
+
     private fun createPopupMenu() {
         popupMenu = BrowserPopupMenu(layoutInflater)
         val view = popupMenu.contentView
@@ -561,7 +574,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
                     )
                 )
             }
-            is Command.DownloadImage -> requestImageDownload(it.url)
+            is Command.DownloadImage -> requestImageDownload(it.url, it.requestUserConfirmation)
             is Command.FindInPageCommand -> webView?.findAllAsync(it.searchTerm)
             is Command.DismissFindInPage -> webView?.findAllAsync("")
             is Command.ShareLink -> launchSharePageChooser(it.url)
@@ -865,7 +878,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
             }
 
             it.setDownloadListener { url, _, contentDisposition, mimeType, _ ->
-                requestFileDownload(url, contentDisposition, mimeType)
+                requestFileDownload(url, contentDisposition, mimeType, true)
             }
 
             it.setOnTouchListener { _, _ ->
@@ -1067,7 +1080,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
         webView = null
     }
 
-    private fun requestFileDownload(url: String, contentDisposition: String, mimeType: String) {
+    private fun requestFileDownload(url: String, contentDisposition: String, mimeType: String, requestUserConfirmation: Boolean) {
         pendingFileDownload = PendingFileDownload(
             url = url,
             contentDisposition = contentDisposition,
@@ -1076,55 +1089,72 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
             subfolder = Environment.DIRECTORY_DOWNLOADS
         )
 
-        downloadFileWithPermissionCheck()
+        if (hasWriteStoragePermission()) {
+            downloadFile(requestUserConfirmation)
+        } else {
+            requestWriteStoragePermission()
+        }
     }
 
-    private fun requestImageDownload(url: String) {
+    private fun requestImageDownload(url: String, requestUserConfirmation: Boolean) {
         pendingFileDownload = PendingFileDownload(
             url = url,
             userAgent = userAgentProvider.getUserAgent(),
             subfolder = Environment.DIRECTORY_PICTURES
         )
 
-        downloadFileWithPermissionCheck()
-    }
-
-    private fun downloadFileWithPermissionCheck() {
         if (hasWriteStoragePermission()) {
-            downloadFile()
+            downloadFile(requestUserConfirmation)
         } else {
             requestWriteStoragePermission()
         }
     }
 
     @AnyThread
-    private fun downloadFile() {
+    private fun downloadFile(requestUserConfirmation: Boolean) {
         val pendingDownload = pendingFileDownload
         pendingFileDownload = null
-        thread {
-            fileDownloader.download(pendingDownload, object : FileDownloader.FileDownloadListener {
-                override fun confirmDownload(downloadFileData: DownloadFileData, userDownloadAction: UserDownloadAction) {
-                    val downloadConfirmationFragment = DownloadConfirmationFragment(downloadFileData, userDownloadAction)
-                    fragmentManager?.let {
-                        downloadConfirmationFragment.show(it, DOWNLAOD_CONFIRM_TAG)
-                    }
-                }
 
-                override fun downloadStarted() {
-                    fileDownloadNotificationManager.showDownloadInProgressNotification()
-                }
+        if (pendingDownload == null) {
+            return
+        }
 
-                override fun downloadFinished(file: File, mimeType: String?) {
-                    MediaScannerConnection.scanFile(context, arrayOf(file.absolutePath), null) { _, uri ->
-                        fileDownloadNotificationManager.showDownloadFinishedNotification(file.name, uri, mimeType)
-                    }
-                }
+        val downloadListener = createDownloadListener()
+        if (requestUserConfirmation) {
+            requestDownloadConfirmation(pendingDownload, downloadListener)
+        } else {
+            completeDownload(pendingDownload, downloadListener)
+        }
+    }
 
-                override fun downloadFailed(message: String) {
-                    Timber.w("Failed to download file [$message]")
-                    fileDownloadNotificationManager.showDownloadFailedNotification()
+    private fun createDownloadListener(): FileDownloadListener {
+        return object : FileDownloadListener {
+            override fun downloadStarted() {
+                fileDownloadNotificationManager.showDownloadInProgressNotification()
+            }
+
+            override fun downloadFinished(file: File, mimeType: String?) {
+                MediaScannerConnection.scanFile(context, arrayOf(file.absolutePath), null) { _, uri ->
+                    fileDownloadNotificationManager.showDownloadFinishedNotification(file.name, uri, mimeType)
                 }
-            })
+            }
+
+            override fun downloadFailed(message: String) {
+                Timber.w("Failed to download file [$message]")
+                fileDownloadNotificationManager.showDownloadFailedNotification()
+            }
+        }
+    }
+
+    private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener) {
+        fragmentManager?.let {
+            DownloadConfirmationFragment.instance(pendingDownload, downloadListener).show(it, DOWNLOAD_CONFIRMATION_TAG)
+        }
+    }
+
+    private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) {
+        thread {
+            fileDownloader.download(pendingDownload, callback)
         }
     }
 
@@ -1147,7 +1177,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
         if (requestCode == PERMISSION_REQUEST_WRITE_EXTERNAL_STORAGE) {
             if ((grantResults.isNotEmpty()) && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
                 Timber.i("Write external storage permission granted")
-                downloadFile()
+                downloadFile(requestUserConfirmation = false)
             } else {
                 Timber.i("Write external storage permission refused")
                 Snackbar.make(toolbar, R.string.permissionRequiredToDownload, Snackbar.LENGTH_LONG).show()
@@ -1189,7 +1219,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
         private const val URL_BUNDLE_KEY = "url"
 
         private const val AUTHENTICATION_DIALOG_TAG = "AUTH_DIALOG_TAG"
-        private const val DOWNLAOD_CONFIRM_TAG = "DOWNLAOD_CONFIRM_TAG"
+        private const val DOWNLOAD_CONFIRMATION_TAG = "DOWNLOAD_CONFIRMATION_TAG"
         private const val DAX_DIALOG_DIALOG_TAG = "DAX_DIALOG_TAG"
 
         private const val MAX_PROGRESS = 100

app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

@@ -175,7 +175,7 @@ class BrowserTabViewModel(
         object ShowKeyboard : Command()
         object HideKeyboard : Command()
         class ShowFullScreen(val view: View) : Command()
-        class DownloadImage(val url: String) : Command()
+        class DownloadImage(val url: String, val requestUserConfirmation: Boolean) : Command()
         class ShowBookmarkAddedConfirmation(val bookmarkId: Long, val title: String?, val url: String?) : Command()
         class ShareLink(val url: String) : Command()
         class CopyLink(val url: String) : Command()
@@ -734,7 +734,7 @@ class BrowserTabViewModel(
                 true
             }
             is RequiredAction.DownloadFile -> {
-                command.value = DownloadImage(requiredAction.url)
+                command.value = DownloadImage(requiredAction.url, false)
                 true
             }
             is RequiredAction.ShareLink -> {

app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt

@@ -24,46 +24,73 @@ import android.view.View
 import android.view.ViewGroup
 import android.widget.Toast
 import androidx.core.content.FileProvider.getUriForFile
-import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.DownloadFileData
-import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.UserDownloadAction
+import com.duckduckgo.app.browser.downloader.FileDownloader
+import com.duckduckgo.app.browser.downloader.FileDownloader.FileDownloadListener
+import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload
+import com.duckduckgo.app.browser.downloader.guessFileName
+import com.duckduckgo.app.browser.downloader.isDataUrl
 import com.duckduckgo.app.global.view.gone
 import com.duckduckgo.app.global.view.leftDrawable
 import com.duckduckgo.app.global.view.show
 import com.google.android.material.bottomsheet.BottomSheetDialogFragment
+import dagger.android.support.AndroidSupportInjection
 import kotlinx.android.synthetic.main.download_confirmation.view.*
 import timber.log.Timber
+import java.io.File
+import java.io.IOException
+import javax.inject.Inject
+import kotlin.concurrent.thread
 
-class DownloadConfirmationFragment(
-    private val downloadFileData: DownloadFileData,
-    private val userDownloadAction: UserDownloadAction
-) : BottomSheetDialogFragment() {
+class DownloadConfirmationFragment : BottomSheetDialogFragment() {
+
+    @Inject
+    lateinit var downloader: FileDownloader
+
+    lateinit var downloadListener: FileDownloadListener
+
+    private val pendingDownload: PendingFileDownload by lazy {
+        arguments!![PENDING_DOWNLOAD_BUNDLE_KEY] as PendingFileDownload
+    }
+
+    private var file: File? = null
+
+    override fun onAttach(context: Context) {
+        AndroidSupportInjection.inject(this)
+        super.onAttach(context)
+    }
 
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
         val view = inflater.inflate(R.layout.download_confirmation, container, false)
+        setupDownload()
         setupViews(view)
         return view
     }
 
+    private fun setupDownload() {
+        file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, pendingDownload.guessFileName()) else null
+    }
+
     private fun setupViews(view: View) {
-        view.downloadMessage.text = getString(R.string.downloadConfirmationSaveFileTitle, downloadFileData.file.name)
-        view.openWith.setOnClickListener {
-            openFile()
-            dismiss()
-        }
+        view.downloadMessage.text = getString(R.string.downloadConfirmationSaveFileTitle, file?.name ?: "")
         view.replace.setOnClickListener {
-            userDownloadAction.acceptAndReplace()
+            deleteFile()
+            completeDownload(pendingDownload, downloadListener)
             dismiss()
         }
         view.continueDownload.setOnClickListener {
-            userDownloadAction.accept()
+            completeDownload(pendingDownload, downloadListener)
+            dismiss()
+        }
+        view.openWith.setOnClickListener {
+            openFile()
             dismiss()
         }
         view.cancel.setOnClickListener {
-            userDownloadAction.cancel()
+            Timber.i("Cancelled download for url ${pendingDownload.url}")
             dismiss()
         }
 
-        if (downloadFileData.alreadyDownloaded) {
+        if (file?.exists() == true) {
             view.openWith.show()
             view.replace.show()
             view.continueDownload.text = getString(R.string.downloadConfirmationKeepBothFilesText)
@@ -76,6 +103,20 @@ class DownloadConfirmationFragment(
         }
     }
 
+    private fun deleteFile() {
+        try {
+            file?.delete()
+        } catch (e: IOException) {
+            Toast.makeText(activity, R.string.downloadConfirmationUnableToDeleteFileText, Toast.LENGTH_SHORT).show()
+        }
+    }
+
+    private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) {
+        thread {
+            downloader.download(pendingDownload, callback)
+        }
+    }
+
     private fun openFile() {
         val intent = context?.let { createIntentToOpenFile(it) }
         activity?.packageManager?.let { packageManager ->
@@ -88,11 +129,26 @@ class DownloadConfirmationFragment(
         }
     }
 
-    private fun createIntentToOpenFile(context: Context): Intent {
-        val uri = getUriForFile(context, "${BuildConfig.APPLICATION_ID}.provider", downloadFileData.file)
-        val mime = activity?.contentResolver?.getType(uri)
+    private fun createIntentToOpenFile(context: Context): Intent? {
+        val file = file ?: return null
+        val uri = getUriForFile(context, "${BuildConfig.APPLICATION_ID}.provider", file)
+        val mime = activity?.contentResolver?.getType(uri) ?: return null
         val intent = Intent(Intent.ACTION_VIEW)
         intent.setDataAndType(uri, mime)
         return intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
     }
+
+    companion object {
+
+        private const val PENDING_DOWNLOAD_BUNDLE_KEY = "PENDING_DOWNLOAD_BUNDLE_KEY"
+
+        fun instance(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener): DownloadConfirmationFragment {
+            val fragment = DownloadConfirmationFragment()
+            val bundle = Bundle()
+            bundle.putSerializable(PENDING_DOWNLOAD_BUNDLE_KEY, pendingDownload)
+            fragment.arguments = bundle
+            fragment.downloadListener = downloadListener
+            return fragment
+        }
+    }
 }

app/src/main/java/com/duckduckgo/app/browser/downloader/DataUriDownloader.kt

@@ -35,8 +35,7 @@ class DataUriDownloader @Inject constructor(
         try {
             callback?.downloadStarted()
 
-            val parsedDataUri = dataUriParser.generate(pending.url)
-            when (parsedDataUri) {
+            when (val parsedDataUri = dataUriParser.generate(pending.url)) {
                 is ParseResult.Invalid -> {
                     Timber.w("Failed to extract data from data URI")
                     callback?.downloadFailed("Failed to extract data from data URI")