Fix crash that could happen when trying to download a large data URI image (#898)

* Fix crash on data uri downloaded callback

This also tidies up some of the code around the DownloadConfirmationFragment, which had a leak and was doing a fair bit; moving some of the logic out of here to keep this dialog focussed on obtaining the user choice makes things simpler.

* Limit length of dialog title to prevent large data URI causing ANRs

Author: CDRussell

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

@@ -40,6 +40,7 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.Command.Navigate
 import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile
 import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab
 import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector
+import com.duckduckgo.app.browser.downloader.FileDownloader
 import com.duckduckgo.app.browser.favicon.FaviconDownloader
 import com.duckduckgo.app.browser.logindetection.LoginDetected
 import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector
@@ -52,27 +53,22 @@ import com.duckduckgo.app.browser.session.WebViewSessionStorage
 import com.duckduckgo.app.cta.db.DismissedCtaDao
 import com.duckduckgo.app.cta.model.CtaId
 import com.duckduckgo.app.cta.model.DismissedCta
+import com.duckduckgo.app.cta.ui.*
 import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao
 import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity
 import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository
-import com.duckduckgo.app.cta.ui.Cta
-import com.duckduckgo.app.cta.ui.CtaViewModel
-import com.duckduckgo.app.cta.ui.DaxBubbleCta
-import com.duckduckgo.app.cta.ui.DaxDialogCta
-import com.duckduckgo.app.cta.ui.HomePanelCta
-import com.duckduckgo.app.cta.ui.UseOurAppCta
-import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_DOMAIN
-import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL
 import com.duckduckgo.app.global.db.AppDatabase
+import com.duckduckgo.app.global.events.db.UserEventEntity
+import com.duckduckgo.app.global.events.db.UserEventKey
+import com.duckduckgo.app.global.events.db.UserEventsStore
 import com.duckduckgo.app.global.install.AppInstallStore
 import com.duckduckgo.app.global.model.Site
 import com.duckduckgo.app.global.model.SiteFactory
-import com.duckduckgo.app.global.events.db.UserEventKey
-import com.duckduckgo.app.notification.model.UseOurAppNotification
-import com.duckduckgo.app.global.events.db.UserEventEntity
-import com.duckduckgo.app.global.events.db.UserEventsStore
 import com.duckduckgo.app.global.useourapp.UseOurAppDetector
+import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_DOMAIN
+import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL
 import com.duckduckgo.app.notification.db.NotificationDao
+import com.duckduckgo.app.notification.model.UseOurAppNotification
 import com.duckduckgo.app.onboarding.store.AppStage
 import com.duckduckgo.app.onboarding.store.OnboardingStore
 import com.duckduckgo.app.onboarding.store.UserStageStore
@@ -209,6 +205,9 @@ class BrowserTabViewModelTest {
     @Mock
     private lateinit var mockNotificationDao: NotificationDao
 
+    @Mock
+    private lateinit var mockFileDownloader: FileDownloader
+
     private lateinit var mockAutoCompleteApi: AutoCompleteApi
 
     private lateinit var ctaViewModel: CtaViewModel
@@ -289,7 +288,8 @@ class BrowserTabViewModelTest {
             userEventsStore = mockUserEventsStore,
             notificationDao = mockNotificationDao,
             useOurAppDetector = UseOurAppDetector(mockUserEventsStore),
-            variantManager = mockVariantManager
+            variantManager = mockVariantManager,
+            fileDownloader = mockFileDownloader
         )
 
         testee.loadData("abc", null, false)

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

@@ -40,14 +40,18 @@ import android.webkit.WebView.HitTestResult
 import android.webkit.WebView.HitTestResult.*
 import android.widget.EditText
 import android.widget.TextView
+import android.widget.Toast
 import androidx.annotation.AnyThread
 import androidx.annotation.StringRes
 import androidx.appcompat.app.AlertDialog
 import androidx.core.content.ContextCompat
+import androidx.core.content.FileProvider
+import androidx.core.net.toUri
 import androidx.core.text.HtmlCompat
 import androidx.core.text.HtmlCompat.FROM_HTML_MODE_LEGACY
 import androidx.core.view.*
 import androidx.fragment.app.Fragment
+import androidx.fragment.app.commitNow
 import androidx.fragment.app.transaction
 import androidx.lifecycle.*
 import androidx.recyclerview.widget.LinearLayoutManager
@@ -56,6 +60,8 @@ import com.duckduckgo.app.bookmarks.ui.EditBookmarkDialogFragment
 import com.duckduckgo.app.brokensite.BrokenSiteActivity
 import com.duckduckgo.app.brokensite.BrokenSiteData
 import com.duckduckgo.app.browser.BrowserTabViewModel.*
+import com.duckduckgo.app.browser.BrowserTabViewModel.Command.DownloadCommand
+import com.duckduckgo.app.browser.DownloadConfirmationFragment.DownloadConfirmationDialogListener
 import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter
 import com.duckduckgo.app.browser.downloader.DownloadFailReason
 import com.duckduckgo.app.browser.downloader.FileDownloadNotificationManager
@@ -107,10 +113,9 @@ import org.jetbrains.anko.share
 import timber.log.Timber
 import java.io.File
 import javax.inject.Inject
-import kotlin.concurrent.thread
 import kotlin.coroutines.CoroutineContext
 
-class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogListener, TrackersAnimatorListener {
+class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogListener, TrackersAnimatorListener, DownloadConfirmationDialogListener {
 
     private val supervisorJob = SupervisorJob()
 
@@ -249,14 +254,6 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
         removeDaxDialogFromActivity()
         renderer = BrowserTabFragmentRenderer()
         decorator = BrowserTabFragmentDecorator()
-        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? {
@@ -556,6 +553,35 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
             is Command.ShowWebContent -> webView?.show()
             is Command.RefreshUserAgent -> refreshUserAgent(it.host, it.isDesktop)
             is Command.AskToFireproofWebsite -> askToFireproofWebsite(requireContext(), it.fireproofWebsite)
+            is DownloadCommand -> processDownloadCommand(it)
+        }
+    }
+
+    private fun processDownloadCommand(it: DownloadCommand) {
+        when (it) {
+            is DownloadCommand.ScanMediaFiles -> {
+                context?.applicationContext?.let { context ->
+                    MediaScannerConnection.scanFile(context, arrayOf(it.file.absolutePath), null, null)
+                }
+            }
+            is DownloadCommand.ShowDownloadFinishedNotification -> {
+                fileDownloadNotificationManager.showDownloadFinishedNotification(it.file.name, it.file.absolutePath.toUri(), it.mimeType)
+            }
+            DownloadCommand.ShowDownloadInProgressNotification -> {
+                fileDownloadNotificationManager.showDownloadInProgressNotification()
+            }
+            is DownloadCommand.ShowDownloadFailedNotification -> {
+                fileDownloadNotificationManager.showDownloadFailedNotification()
+
+                val snackbar = Snackbar.make(toolbar, R.string.downloadFailed, Snackbar.LENGTH_INDEFINITE)
+                if (it.reason == DownloadFailReason.DownloadManagerDisabled) {
+                    snackbar.setText(it.message)
+                    snackbar.setAction(getString(R.string.enable)) {
+                        showDownloadManagerAppSettings()
+                    }
+                }
+                snackbar.show()
+            }
         }
     }
 
@@ -1092,90 +1118,28 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
 
     @AnyThread
     private fun downloadFile(requestUserConfirmation: Boolean) {
-        val pendingDownload = pendingFileDownload
-        pendingFileDownload = null
+        val pendingDownload = pendingFileDownload ?: return
 
-        if (pendingDownload == null) {
-            return
-        }
+        pendingFileDownload = null
 
-        val downloadListener = createDownloadListener()
         if (requestUserConfirmation) {
-            requestDownloadConfirmation(pendingDownload, downloadListener)
+            requestDownloadConfirmation(pendingDownload)
         } else {
-            completeDownload(pendingDownload, downloadListener)
+            continueDownload(pendingDownload)
         }
     }
 
-    private fun closeAndReturnToSourceIfBlankTab() {
-        if (viewModel.url == null) {
-            launch {
-                viewModel.closeAndSelectSourceTab()
-            }
-        }
-    }
-
-    private fun createDownloadListener(): FileDownloadListener {
-        return object : FileDownloadListener {
-            override fun downloadStarted() {
-                fileDownloadNotificationManager.showDownloadInProgressNotification()
-                closeAndReturnToSourceIfBlankTab()
-            }
-
-            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, downloadFailReason: DownloadFailReason) {
-                Timber.w("Failed to download file [$message]")
-
-                fileDownloadNotificationManager.showDownloadFailedNotification()
-
-                val snackbar = Snackbar.make(toolbar, R.string.downloadFailed, Snackbar.LENGTH_INDEFINITE)
-                if (downloadFailReason == DownloadFailReason.DownloadManagerDisabled) {
-                    snackbar.setText(message)
-                    snackbar.setAction(getString(R.string.enable)) {
-                        showDownloadManagerAppSettings()
-                    }
-                }
-                snackbar.show()
-            }
-
-            override fun downloadCancelled() {
-                closeAndReturnToSourceIfBlankTab()
-            }
-
-            override fun downloadOpened() {
-                closeAndReturnToSourceIfBlankTab()
-            }
-
-            private fun showDownloadManagerAppSettings() {
-                try {
-                    val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS)
-                    intent.data = DownloadFailReason.DOWNLOAD_MANAGER_SETTINGS_URI
-                    startActivity(intent)
-                } catch (e: ActivityNotFoundException) {
-                    Timber.w(e, "Could not open DownloadManager settings")
-                    Snackbar.make(toolbar, R.string.downloadManagerIncompatible, Snackbar.LENGTH_INDEFINITE).show()
-                }
-            }
-        }
-    }
-
-    private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener) {
-        fragmentManager?.let {
-            if (!it.isStateSaved) {
-                DownloadConfirmationFragment.instance(pendingDownload, downloadListener).show(it, DOWNLOAD_CONFIRMATION_TAG)
-            }
+    private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload) {
+        val downloadConfirmationFragment = DownloadConfirmationFragment.instance(pendingDownload)
+        childFragmentManager.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG)?.let {
+            Timber.i("Found existing dialog; removing it now")
+            childFragmentManager.commitNow { remove(it) }
         }
+        downloadConfirmationFragment.show(childFragmentManager, DOWNLOAD_CONFIRMATION_TAG)
     }
 
     private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) {
-        thread {
-            fileDownloader.download(pendingDownload, callback)
-        }
+        viewModel.download(pendingDownload)
     }
 
     private fun launchFilePicker(command: Command.ShowFileChooser) {
@@ -1787,4 +1751,55 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
         private fun shouldUpdateOmnibarTextInput(viewState: OmnibarViewState, omnibarInput: String?) =
             (!viewState.isEditing || omnibarInput.isNullOrEmpty()) && omnibarTextInput.isDifferent(omnibarInput)
     }
+
+    override fun openExistingFile(file: File?) {
+        if (file == null) {
+            Toast.makeText(activity, R.string.downloadConfirmationUnableToOpenFileText, Toast.LENGTH_SHORT).show()
+            return
+        }
+
+        val intent = context?.let { createIntentToOpenFile(it, file) }
+        activity?.packageManager?.let { packageManager ->
+            if (intent?.resolveActivity(packageManager) != null) {
+                startActivity(intent)
+            } else {
+                Timber.e("No suitable activity found")
+                Toast.makeText(activity, R.string.downloadConfirmationUnableToOpenFileText, Toast.LENGTH_SHORT).show()
+            }
+        }
+    }
+
+    override fun replaceExistingFile(file: File?, pendingFileDownload: PendingFileDownload) {
+        Timber.i("Deleting existing file: $file")
+        runCatching { file?.delete() }
+        continueDownload(pendingFileDownload)
+    }
+
+    private fun showDownloadManagerAppSettings() {
+        try {
+            val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS)
+            intent.data = DownloadFailReason.DOWNLOAD_MANAGER_SETTINGS_URI
+            startActivity(intent)
+        } catch (e: ActivityNotFoundException) {
+            Timber.w(e, "Could not open DownloadManager settings")
+            Snackbar.make(toolbar, R.string.downloadManagerIncompatible, Snackbar.LENGTH_INDEFINITE).show()
+        }
+    }
+
+    private fun createIntentToOpenFile(context: Context, file: File): Intent? {
+        val uri = FileProvider.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)
+    }
+
+    override fun continueDownload(pendingFileDownload: PendingFileDownload) {
+        Timber.i("Continuing to download $pendingFileDownload")
+        viewModel.download(pendingFileDownload)
+    }
+
+    override fun cancelDownload() {
+        viewModel.closeAndReturnToSourceIfBlankTab()
+    }
 }