From 209830823224bae45b00dba5c77d9686f4d16ece Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Fri, 7 Aug 2020 09:16:10 +0100 Subject: [PATCH 1/2] 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. --- .../app/browser/BrowserTabViewModelTest.kt | 26 +-- .../app/browser/BrowserTabFragment.kt | 181 ++++++++++-------- .../app/browser/BrowserTabViewModel.kt | 79 ++++++-- .../browser/DownloadConfirmationFragment.kt | 68 ++----- .../app/browser/di/BrowserModule.kt | 13 +- .../browser/downloader/DataUriDownloader.kt | 4 +- .../app/browser/downloader/FileDownloader.kt | 42 ++-- .../downloader/NetworkFileDownloader.kt | 2 +- .../duckduckgo/app/global/ViewModelFactory.kt | 5 +- 9 files changed, 235 insertions(+), 185 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index ba957ba2ac48..c577700e4923 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/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) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index c8b2cf7e92f0..9a1bb0663429 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/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() + } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index db73d0c8d4a0..3193ec9f00d4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -50,33 +50,28 @@ import com.duckduckgo.app.browser.LongPressHandler.RequiredAction import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.IntentType import com.duckduckgo.app.browser.WebNavigationStateChange.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector +import com.duckduckgo.app.browser.downloader.DownloadFailReason +import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.favicon.FaviconDownloader -import com.duckduckgo.app.browser.logindetection.NavigationEvent import com.duckduckgo.app.browser.logindetection.LoginDetected import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector +import com.duckduckgo.app.browser.logindetection.NavigationEvent import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials import com.duckduckgo.app.browser.model.BasicAuthenticationRequest import com.duckduckgo.app.browser.model.LongPressTarget import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.ui.HttpAuthenticationDialogFragment.HttpAuthenticationListener -import com.duckduckgo.app.cta.ui.Cta -import com.duckduckgo.app.cta.ui.CtaViewModel -import com.duckduckgo.app.cta.ui.DaxDialogCta -import com.duckduckgo.app.cta.ui.DialogCta -import com.duckduckgo.app.cta.ui.HomePanelCta -import com.duckduckgo.app.cta.ui.HomeTopPanelCta -import com.duckduckgo.app.cta.ui.UseOurAppCta +import com.duckduckgo.app.cta.ui.* import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository import com.duckduckgo.app.global.* +import com.duckduckgo.app.global.events.db.UserEventKey +import com.duckduckgo.app.global.events.db.UserEventsStore import com.duckduckgo.app.global.model.Site import com.duckduckgo.app.global.model.SiteFactory import com.duckduckgo.app.global.model.domain import com.duckduckgo.app.global.model.domainMatchesUrl -import com.duckduckgo.app.global.events.db.UserEventsStore -import com.duckduckgo.app.global.events.db.UserEventKey -import com.duckduckgo.app.global.toDesktopUri import com.duckduckgo.app.global.useourapp.UseOurAppDetector import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_TITLE import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL @@ -106,7 +101,8 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import timber.log.Timber -import java.util.Locale +import java.io.File +import java.util.* import java.util.concurrent.TimeUnit class BrowserTabViewModel( @@ -134,7 +130,8 @@ class BrowserTabViewModel( private val userEventsStore: UserEventsStore, private val notificationDao: NotificationDao, private val useOurAppDetector: UseOurAppDetector, - private val variantManager: VariantManager + private val variantManager: VariantManager, + private val fileDownloader: FileDownloader ) : WebViewClientListener, EditBookmarkListener, HttpAuthenticationListener, ViewModel() { private var buildingSiteFactoryJob: Job? = null @@ -251,6 +248,13 @@ class BrowserTabViewModel( object FinishTrackerAnimation : DaxCommand() class HideDaxDialog(val cta: Cta) : DaxCommand() } + + sealed class DownloadCommand : Command() { + class ScanMediaFiles(val file: File) : DownloadCommand() + class ShowDownloadFailedNotification(val message: String, val reason: DownloadFailReason) : DownloadCommand() + class ShowDownloadFinishedNotification(val file: File, val mimeType: String?) : DownloadCommand() + object ShowDownloadInProgressNotification : DownloadCommand() + } } val autoCompleteViewState: MutableLiveData = MutableLiveData() @@ -505,6 +509,12 @@ class BrowserTabViewModel( viewModelScope.launch { removeCurrentTabFromRepository() } } + fun closeAndReturnToSourceIfBlankTab() { + if (url == null) { + closeAndSelectSourceTab() + } + } + override fun closeAndSelectSourceTab() { viewModelScope.launch { removeAndSelectTabFromRepository() } } @@ -1387,6 +1397,49 @@ class BrowserTabViewModel( navigationAwareLoginDetector.onEvent(NavigationEvent.LoginAttempt(currentUrl)) } + fun download(pendingFileDownload: FileDownloader.PendingFileDownload) { + viewModelScope.launch(dispatchers.io()) { + fileDownloader.download(pendingFileDownload, object : FileDownloader.FileDownloadListener { + + override fun downloadStartedNetworkFile() { + Timber.d("download started: network file") + closeAndReturnToSourceIfBlankTab() + } + + override fun downloadFinishedNetworkFile(file: File, mimeType: String?) { + Timber.i("downloadFinished network file") + } + + override fun downloadStartedDataUri() { + Timber.i("downloadStarted data uri") + command.postValue(DownloadCommand.ShowDownloadInProgressNotification) + closeAndReturnToSourceIfBlankTab() + } + + override fun downloadFinishedDataUri(file: File, mimeType: String?) { + Timber.i("downloadFinished data uri") + command.postValue(DownloadCommand.ScanMediaFiles(file)) + command.postValue(DownloadCommand.ShowDownloadFinishedNotification(file, mimeType)) + } + + override fun downloadFailed(message: String, downloadFailReason: DownloadFailReason) { + Timber.w("Failed to download file [$message]") + command.postValue(DownloadCommand.ShowDownloadFailedNotification(message, downloadFailReason)) + } + + override fun downloadCancelled() { + Timber.i("Download cancelled") + closeAndReturnToSourceIfBlankTab() + } + + override fun downloadOpened() { + closeAndReturnToSourceIfBlankTab() + } + + }) + } + } + companion object { private const val FIXED_PROGRESS = 50 } diff --git a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt index db17eb8b5b8e..9e1177a9f984 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -17,15 +17,10 @@ package com.duckduckgo.app.browser import android.content.Context -import android.content.Intent import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.Toast -import androidx.core.content.FileProvider.getUriForFile -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 @@ -37,23 +32,18 @@ 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 : BottomSheetDialogFragment() { - @Inject - lateinit var downloader: FileDownloader + val listener: DownloadConfirmationDialogListener + get() = parentFragment as DownloadConfirmationDialogListener - lateinit var downloadListener: FileDownloadListener + private var file: File? = null private val pendingDownload: PendingFileDownload by lazy { requireArguments()[PENDING_DOWNLOAD_BUNDLE_KEY] as PendingFileDownload } - private var file: File? = null - override fun onAttach(context: Context) { AndroidSupportInjection.inject(this) super.onAttach(context) @@ -73,21 +63,20 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { private fun setupViews(view: View) { view.downloadMessage.text = getString(R.string.downloadConfirmationSaveFileTitle, file?.name ?: "") view.replace.setOnClickListener { - deleteFile() - completeDownload(pendingDownload, downloadListener) + listener.replaceExistingFile(file, pendingDownload) dismiss() } view.continueDownload.setOnClickListener { - completeDownload(pendingDownload, downloadListener) + listener.continueDownload(pendingDownload) dismiss() } view.openWith.setOnClickListener { - openFile() + listener.openExistingFile(file) dismiss() } view.cancel.setOnClickListener { Timber.i("Cancelled download for url ${pendingDownload.url}") - downloadListener.downloadCancelled() + listener.cancelDownload() dismiss() } @@ -104,52 +93,23 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { } } - 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 -> - if (intent?.resolveActivity(packageManager) != null) { - startActivity(intent) - } else { - Timber.e("No suitable activity found") - Toast.makeText(activity, R.string.downloadConfirmationUnableToOpenFileText, Toast.LENGTH_SHORT).show() - } - downloadListener.downloadOpened() - } - } - - 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) + interface DownloadConfirmationDialogListener { + fun openExistingFile(file: File?) + fun replaceExistingFile(file: File?, pendingFileDownload: PendingFileDownload) + fun continueDownload(pendingFileDownload: PendingFileDownload) + fun cancelDownload() } companion object { private const val PENDING_DOWNLOAD_BUNDLE_KEY = "PENDING_DOWNLOAD_BUNDLE_KEY" - fun instance(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener): DownloadConfirmationFragment { + fun instance(pendingDownload: PendingFileDownload): DownloadConfirmationFragment { val fragment = DownloadConfirmationFragment() + fragment.isCancelable = false val bundle = Bundle() bundle.putSerializable(PENDING_DOWNLOAD_BUNDLE_KEY, pendingDownload) fragment.arguments = bundle - fragment.downloadListener = downloadListener return fragment } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 376ac08a0d85..bbac06264c81 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -26,10 +26,14 @@ import com.duckduckgo.app.browser.addtohome.AddToHomeSystemCapabilityDetector import com.duckduckgo.app.browser.defaultbrowsing.AndroidDefaultBrowserDetector import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserObserver -import com.duckduckgo.app.browser.logindetection.JsLoginDetector +import com.duckduckgo.app.browser.downloader.AndroidFileDownloader +import com.duckduckgo.app.browser.downloader.DataUriDownloader +import com.duckduckgo.app.browser.downloader.FileDownloader +import com.duckduckgo.app.browser.downloader.NetworkFileDownloader import com.duckduckgo.app.browser.logindetection.DOMLoginDetector -import com.duckduckgo.app.browser.logindetection.NextPageLoginDetection +import com.duckduckgo.app.browser.logindetection.JsLoginDetector import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector +import com.duckduckgo.app.browser.logindetection.NextPageLoginDetection import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.tabpreview.FileBasedWebViewPreviewGenerator @@ -229,4 +233,9 @@ class BrowserModule { fun navigationAwareLoginDetector(): NavigationAwareLoginDetector { return NextPageLoginDetection() } + + @Provides + fun fileDownloader(dataUriDownloader: DataUriDownloader, networkFileDownloader: NetworkFileDownloader): FileDownloader { + return AndroidFileDownloader(dataUriDownloader, networkFileDownloader) + } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/DataUriDownloader.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/DataUriDownloader.kt index 57fb67a95266..f549e9f7433d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/DataUriDownloader.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/DataUriDownloader.kt @@ -33,7 +33,7 @@ class DataUriDownloader @Inject constructor( fun download(pending: FileDownloader.PendingFileDownload, callback: FileDownloader.FileDownloadListener?) { try { - callback?.downloadStarted() + callback?.downloadStartedDataUri() when (val parsedDataUri = dataUriParser.generate(pending.url)) { is ParseResult.Invalid -> { @@ -45,7 +45,7 @@ class DataUriDownloader @Inject constructor( val file = initialiseFilesOnDisk(pending, parsedDataUri.filename) writeBytesToFiles(parsedDataUri.data, file) - callback?.downloadFinished(file, parsedDataUri.mimeType) + callback?.downloadFinishedDataUri(file, parsedDataUri.mimeType) } } } catch (e: IOException) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt index 826ed1340be5..72430a69ee90 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt @@ -20,24 +20,17 @@ import android.net.Uri import android.os.Environment import android.webkit.URLUtil import androidx.annotation.WorkerThread +import com.duckduckgo.app.browser.downloader.FileDownloader.FileDownloadListener +import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload import timber.log.Timber import java.io.File import java.io.Serializable import javax.inject.Inject -class FileDownloader @Inject constructor( - private val dataUriDownloader: DataUriDownloader, - private val networkFileDownloader: NetworkFileDownloader -) { +interface FileDownloader { @WorkerThread - fun download(pending: PendingFileDownload, callback: FileDownloadListener) { - when { - pending.isNetworkUrl -> networkFileDownloader.download(pending, callback) - pending.isDataUrl -> dataUriDownloader.download(pending, callback) - else -> callback.downloadFailed("Not supported", DownloadFailReason.UnsupportedUrlType) - } - } + fun download(pending: PendingFileDownload, callback: FileDownloadListener) data class PendingFileDownload( val url: String, @@ -49,23 +42,40 @@ class FileDownloader @Inject constructor( ) : Serializable interface FileDownloadListener { - fun downloadStarted() - fun downloadFinished(file: File, mimeType: String?) + fun downloadStartedDataUri() + fun downloadStartedNetworkFile() + fun downloadFinishedDataUri(file: File, mimeType: String?) + fun downloadFinishedNetworkFile(file: File, mimeType: String?) fun downloadFailed(message: String, downloadFailReason: DownloadFailReason) fun downloadCancelled() fun downloadOpened() } } -fun FileDownloader.PendingFileDownload.guessFileName(): String { +class AndroidFileDownloader @Inject constructor( + private val dataUriDownloader: DataUriDownloader, + private val networkFileDownloader: NetworkFileDownloader +) : FileDownloader { + + @WorkerThread + override fun download(pending: PendingFileDownload, callback: FileDownloadListener) { + when { + pending.isNetworkUrl -> networkFileDownloader.download(pending, callback) + pending.isDataUrl -> dataUriDownloader.download(pending, callback) + else -> callback.downloadFailed("Not supported", DownloadFailReason.UnsupportedUrlType) + } + } +} + +fun PendingFileDownload.guessFileName(): String { val guessedFileName = URLUtil.guessFileName(url, contentDisposition, mimeType) Timber.i("Guessed filename of $guessedFileName for url $url") return guessedFileName } -val FileDownloader.PendingFileDownload.isDataUrl get() = URLUtil.isDataUrl(url) +val PendingFileDownload.isDataUrl get() = URLUtil.isDataUrl(url) -val FileDownloader.PendingFileDownload.isNetworkUrl get() = URLUtil.isNetworkUrl(url) +val PendingFileDownload.isNetworkUrl get() = URLUtil.isNetworkUrl(url) sealed class DownloadFailReason { diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt index 08e1c732a445..23ec543764e8 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt @@ -46,7 +46,7 @@ class NetworkFileDownloader @Inject constructor(private val context: Context) { } val manager = context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager? manager?.enqueue(request) - callback.downloadStarted() + callback.downloadStartedNetworkFile() } private fun downloadManagerAvailable(): Boolean { diff --git a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt index 3d77ff5d4d08..d1f3461c7fbe 100644 --- a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt @@ -26,6 +26,7 @@ import com.duckduckgo.app.brokensite.api.BrokenSiteSender import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector +import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.favicon.FaviconDownloader import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector import com.duckduckgo.app.browser.omnibar.QueryUrlConverter @@ -118,6 +119,7 @@ class ViewModelFactory @Inject constructor( private val notificationDao: NotificationDao, private val userOurAppDetector: UseOurAppDetector, private val dismissedCtaDao: DismissedCtaDao, + private val fileDownloader: FileDownloader, private val dispatcherProvider: DispatcherProvider ) : ViewModelProvider.NewInstanceFactory() { @@ -210,7 +212,8 @@ class ViewModelFactory @Inject constructor( userEventsStore = userEventsStore, notificationDao = notificationDao, useOurAppDetector = userOurAppDetector, - variantManager = variantManager + variantManager = variantManager, + fileDownloader = fileDownloader ) private fun changeAppIconViewModel() = From 313479ce1155a618703b12d2851ef55f15969b36 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Fri, 7 Aug 2020 09:25:24 +0100 Subject: [PATCH 2/2] Limit length of dialog title to prevent large data URI causing ANRs --- .../com/duckduckgo/app/browser/WebViewLongPressHandler.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt index b74dfb335b5c..7a9314361d90 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt @@ -46,7 +46,7 @@ interface LongPressHandler { class WebViewLongPressHandler @Inject constructor(private val context: Context, private val pixel: Pixel) : LongPressHandler { override fun handleLongPress(longPressTargetType: Int, longPressTargetUrl: String?, menu: ContextMenu) { - menu.setHeaderTitle(longPressTargetUrl ?: context.getString(R.string.options)) + menu.setHeaderTitle(longPressTargetUrl?.take(MAX_TITLE_LENGTH) ?: context.getString(R.string.options)) var menuShown = true when (longPressTargetType) { @@ -135,5 +135,7 @@ class WebViewLongPressHandler @Inject constructor(private val context: Context, const val CONTEXT_MENU_ID_SHARE_LINK = 4 const val CONTEXT_MENU_ID_DOWNLOAD_IMAGE = 5 const val CONTEXT_MENU_ID_OPEN_IMAGE_IN_NEW_BACKGROUND_TAB = 6 + + private const val MAX_TITLE_LENGTH = 100 } }