From 6485f64ff26716668e10994639c4aedb06459193 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Fri, 3 Apr 2020 20:02:51 +0100 Subject: [PATCH 1/9] Suppress dialog if download already requested via a dialog by user --- .../app/browser/BrowserTabFragment.kt | 106 ++++++++++++------ .../browser/DownloadConfirmationFragment.kt | 20 +++- .../app/browser/downloader/FileDownloader.kt | 18 +-- .../downloader/NetworkFileDownloadManager.kt | 58 ---------- .../downloader/NetworkFileDownloader.kt | 14 +-- 5 files changed, 101 insertions(+), 115 deletions(-) delete mode 100644 app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloadManager.kt 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 dda212a2f6c0..08b70c383ee4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -91,14 +91,14 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.FindInPageViewState import com.duckduckgo.app.browser.BrowserTabViewModel.GlobalLayoutViewState import com.duckduckgo.app.browser.BrowserTabViewModel.LoadingViewState import com.duckduckgo.app.browser.BrowserTabViewModel.OmnibarViewState +import com.duckduckgo.app.browser.DownloadConfirmationFragment.* import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserNavigator import com.duckduckgo.app.browser.defaultbrowsing.TopInstructionsCard import com.duckduckgo.app.browser.downloader.FileDownloadNotificationManager import com.duckduckgo.app.browser.downloader.FileDownloader 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.downloader.guessFileName import com.duckduckgo.app.browser.filechooser.FileChooserIntentBuilder import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials import com.duckduckgo.app.browser.model.BasicAuthenticationRequest @@ -579,7 +579,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { ) ) } - is Command.DownloadImage -> requestImageDownload(it.url) + is Command.DownloadImage -> requestImageDownload(it.url, requestUserConfirmation = false) is Command.FindInPageCommand -> webView?.findAllAsync(it.searchTerm) is Command.DismissFindInPage -> webView?.findAllAsync("") is Command.ShareLink -> launchSharePageChooser(it.url) @@ -905,7 +905,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { } it.setDownloadListener { url, _, contentDisposition, mimeType, _ -> - requestFileDownload(url, contentDisposition, mimeType) + requestFileDownload(url, contentDisposition, mimeType, true) } it.setOnTouchListener { _, _ -> @@ -1107,7 +1107,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, @@ -1116,55 +1116,95 @@ 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 = object : FileDownloader.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() + } + } - override fun downloadFailed(message: String) { - Timber.w("Failed to download file [$message]") - fileDownloadNotificationManager.showDownloadFailedNotification() + if (requestUserConfirmation) { + requestDownloadConfirmation(pendingDownload, downloadListener) + } else { + completeDownload(pendingDownload, downloadListener) + } + } + + private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloader.FileDownloadListener) { + val guessedFileName = pendingDownload.guessFileName() + + val data = if (guessedFileName != null) { + val fileToDownload = File(pendingDownload.directory, guessedFileName) + DownloadFileData(fileToDownload, fileToDownload.exists()) + } else { + DownloadFileData(null, false) + } + + val downloadConfirmationFragment = DownloadConfirmationFragment(data, object : UserDownloadAction { + override fun acceptAndReplace() { + if (guessedFileName != null) { + File(pendingDownload.directory, guessedFileName).delete() } - }) + completeDownload(pendingDownload, downloadListener) + } + + override fun accept() { + completeDownload(pendingDownload, downloadListener) + } + + override fun cancel() { + Timber.i("Cancelled download for url ${pendingDownload.url}") + } + }) + + fragmentManager?.let { + downloadConfirmationFragment.show(it, DOWNLAOD_CONFIRM_TAG) + } + } + + fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + thread { + fileDownloader.download(pendingDownload, callback) } } @@ -1187,7 +1227,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() 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 036b20a7eabd..0a811f42feb3 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -24,20 +24,27 @@ 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.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 kotlinx.android.synthetic.main.download_confirmation.view.* import timber.log.Timber +import java.io.File class DownloadConfirmationFragment( private val downloadFileData: DownloadFileData, private val userDownloadAction: UserDownloadAction ) : BottomSheetDialogFragment() { + interface UserDownloadAction { + fun accept() + fun acceptAndReplace() + fun cancel() + } + + class DownloadFileData(val file: File?, val alreadyDownloaded: Boolean) + override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.download_confirmation, container, false) setupViews(view) @@ -45,7 +52,7 @@ class DownloadConfirmationFragment( } private fun setupViews(view: View) { - view.downloadMessage.text = getString(R.string.downloadConfirmationSaveFileTitle, downloadFileData.file.name) + view.downloadMessage.text = getString(R.string.downloadConfirmationSaveFileTitle, downloadFileData.file?.name ?: "") view.openWith.setOnClickListener { openFile() dismiss() @@ -88,9 +95,10 @@ 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 = downloadFileData.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) 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 d0003e2651ba..7bfc8d0f2524 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 @@ -19,14 +19,13 @@ package com.duckduckgo.app.browser.downloader import android.os.Environment import android.webkit.URLUtil import androidx.annotation.WorkerThread -import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.DownloadFileData -import com.duckduckgo.app.browser.downloader.NetworkFileDownloadManager.UserDownloadAction +import timber.log.Timber import java.io.File import javax.inject.Inject class FileDownloader @Inject constructor( private val dataUriDownloader: DataUriDownloader, - private val networkFileDownloadManager: NetworkFileDownloadManager + private val networkFileDownloader: NetworkFileDownloader ) { @WorkerThread @@ -37,7 +36,7 @@ class FileDownloader @Inject constructor( } when { - URLUtil.isNetworkUrl(pending.url) -> networkFileDownloadManager.download(pending, callback) + URLUtil.isNetworkUrl(pending.url) -> networkFileDownloader.download(pending) URLUtil.isDataUrl(pending.url) -> dataUriDownloader.download(pending, callback) else -> callback.downloadFailed("Not supported") } @@ -53,12 +52,15 @@ class FileDownloader @Inject constructor( ) interface FileDownloadListener { - fun confirmDownload( - downloadFileData: DownloadFileData, - userDownloadAction: UserDownloadAction - ) fun downloadStarted() fun downloadFinished(file: File, mimeType: String?) fun downloadFailed(message: String) } +} + +fun FileDownloader.PendingFileDownload.guessFileName(): String? { + if (URLUtil.isDataUrl(url)) return null + val guessedFileName = URLUtil.guessFileName(url, contentDisposition, mimeType) + Timber.i("Guessed filename of $guessedFileName for url ${url}") + return guessedFileName } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloadManager.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloadManager.kt deleted file mode 100644 index 750a23815ea7..000000000000 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloadManager.kt +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2019 DuckDuckGo - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.duckduckgo.app.browser.downloader - -import timber.log.Timber -import java.io.File -import javax.inject.Inject - - -class NetworkFileDownloadManager @Inject constructor(private val networkDownloader: NetworkFileDownloader) { - - fun download( - pendingDownload: FileDownloader.PendingFileDownload, - callback: FileDownloader.FileDownloadListener - ) { - val guessedFileName = networkDownloader.guessFileName(pendingDownload) - val fileToDownload = File(pendingDownload.directory, guessedFileName) - val alreadyDownloaded = fileToDownload.exists() - callback.confirmDownload( - DownloadFileData(fileToDownload, alreadyDownloaded), - object : UserDownloadAction { - override fun acceptAndReplace() { - File(pendingDownload.directory, guessedFileName).delete() - networkDownloader.download(pendingDownload) - } - - override fun accept() { - networkDownloader.download(pendingDownload) - } - - override fun cancel() { - Timber.i("Cancelled download for url ${pendingDownload.url}") - } - }) - } - - interface UserDownloadAction { - fun accept() - fun acceptAndReplace() - fun cancel() - } - - class DownloadFileData(val file: File, val alreadyDownloaded: Boolean) -} \ No newline at end of file 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 861e448a8ac1..6434c1eeef6a 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 @@ -19,15 +19,14 @@ package com.duckduckgo.app.browser.downloader import android.app.DownloadManager import android.content.Context import android.webkit.CookieManager -import android.webkit.URLUtil import androidx.core.net.toUri -import timber.log.Timber +import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload import javax.inject.Inject class NetworkFileDownloader @Inject constructor(private val context: Context) { - fun download(pendingDownload: FileDownloader.PendingFileDownload) { - val guessedFileName = guessFileName(pendingDownload) + fun download(pendingDownload: PendingFileDownload) { + val guessedFileName = pendingDownload.guessFileName() val request = DownloadManager.Request(pendingDownload.url.toUri()).apply { allowScanningByMediaScanner() @@ -40,10 +39,5 @@ class NetworkFileDownloader @Inject constructor(private val context: Context) { val manager = context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager? manager?.enqueue(request) } +} - fun guessFileName(pending: FileDownloader.PendingFileDownload): String? { - val guessedFileName = URLUtil.guessFileName(pending.url, pending.contentDisposition, pending.mimeType) - Timber.i("Guessed filename of $guessedFileName for url ${pending.url}") - return guessedFileName - } -} \ No newline at end of file From 0f5a54c5545cd1a348dc00f51536469daaf4e889 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Fri, 3 Apr 2020 23:27:53 +0100 Subject: [PATCH 2/9] Simplify logic --- .../app/browser/BrowserTabFragment.kt | 31 +--------- .../browser/DownloadConfirmationFragment.kt | 59 ++++++++++++------- .../main/res/values/string-untranslated.xml | 4 +- 3 files changed, 43 insertions(+), 51 deletions(-) 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 08b70c383ee4..3c2467eed7cf 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -91,14 +91,12 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.FindInPageViewState import com.duckduckgo.app.browser.BrowserTabViewModel.GlobalLayoutViewState import com.duckduckgo.app.browser.BrowserTabViewModel.LoadingViewState import com.duckduckgo.app.browser.BrowserTabViewModel.OmnibarViewState -import com.duckduckgo.app.browser.DownloadConfirmationFragment.* import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserNavigator import com.duckduckgo.app.browser.defaultbrowsing.TopInstructionsCard import com.duckduckgo.app.browser.downloader.FileDownloadNotificationManager import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload -import com.duckduckgo.app.browser.downloader.guessFileName import com.duckduckgo.app.browser.filechooser.FileChooserIntentBuilder import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials import com.duckduckgo.app.browser.model.BasicAuthenticationRequest @@ -1171,38 +1169,13 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { } private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloader.FileDownloadListener) { - val guessedFileName = pendingDownload.guessFileName() - - val data = if (guessedFileName != null) { - val fileToDownload = File(pendingDownload.directory, guessedFileName) - DownloadFileData(fileToDownload, fileToDownload.exists()) - } else { - DownloadFileData(null, false) - } - - val downloadConfirmationFragment = DownloadConfirmationFragment(data, object : UserDownloadAction { - override fun acceptAndReplace() { - if (guessedFileName != null) { - File(pendingDownload.directory, guessedFileName).delete() - } - completeDownload(pendingDownload, downloadListener) - } - - override fun accept() { - completeDownload(pendingDownload, downloadListener) - } - - override fun cancel() { - Timber.i("Cancelled download for url ${pendingDownload.url}") - } - }) - fragmentManager?.let { + val downloadConfirmationFragment = DownloadConfirmationFragment(pendingDownload, fileDownloader, downloadListener) downloadConfirmationFragment.show(it, DOWNLAOD_CONFIRM_TAG) } } - fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + fun completeDownload(pendingDownload: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { thread { fileDownloader.download(pendingDownload, callback) } 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 0a811f42feb3..5cc783dfce48 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -24,6 +24,8 @@ 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.guessFileName import com.duckduckgo.app.global.view.gone import com.duckduckgo.app.global.view.leftDrawable import com.duckduckgo.app.global.view.show @@ -31,46 +33,49 @@ import com.google.android.material.bottomsheet.BottomSheetDialogFragment import kotlinx.android.synthetic.main.download_confirmation.view.* import timber.log.Timber import java.io.File +import java.io.IOException +import kotlin.concurrent.thread class DownloadConfirmationFragment( - private val downloadFileData: DownloadFileData, - private val userDownloadAction: UserDownloadAction + private val pendingDownload: FileDownloader.PendingFileDownload, + private val downloader: FileDownloader, + private val downloadListener: FileDownloader.FileDownloadListener, + private var file: File? = null ) : BottomSheetDialogFragment() { - interface UserDownloadAction { - fun accept() - fun acceptAndReplace() - fun cancel() - } - - class DownloadFileData(val file: File?, val alreadyDownloaded: Boolean) - override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.download_confirmation, container, false) + setupFile() setupViews(view) return view } + private fun setupFile() { + val guessedFileName = pendingDownload.guessFileName() + file = if (guessedFileName != null) File(pendingDownload.directory, guessedFileName) 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) @@ -83,6 +88,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: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + thread { + downloader.download(pendingDownload, callback) + } + } + private fun openFile() { val intent = context?.let { createIntentToOpenFile(it) } activity?.packageManager?.let { packageManager -> @@ -96,7 +115,7 @@ class DownloadConfirmationFragment( } private fun createIntentToOpenFile(context: Context): Intent? { - val file = downloadFileData.file ?: return null + 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) diff --git a/app/src/main/res/values/string-untranslated.xml b/app/src/main/res/values/string-untranslated.xml index 557719e21c5c..8e47acad205c 100644 --- a/app/src/main/res/values/string-untranslated.xml +++ b/app/src/main/res/values/string-untranslated.xml @@ -44,8 +44,8 @@ Keep both Replace Open - Can\'t open file - + Could not open file + Could not delete old file Search From 31497fcf4a3b861b6708e728a55fbaba23f03701 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Mon, 6 Apr 2020 14:18:05 +0100 Subject: [PATCH 3/9] Add test to check that confirmation suppressed on image download --- .../com/duckduckgo/app/browser/BrowserTabViewModelTest.kt | 3 ++- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 4 ++-- 3 files changed, 5 insertions(+), 4 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 5b43df010b87..25aa57574d63 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/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 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 c735ec4b7c26..d250d02d861c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -559,7 +559,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { ) ) } - is Command.DownloadImage -> requestImageDownload(it.url, requestUserConfirmation = false) + 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) 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 05850c553ff4..f26cc7e795c8 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/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 -> { From 985804ff594ac35bccddffac2c10f5f6704fef19 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Thu, 9 Apr 2020 13:37:37 +0100 Subject: [PATCH 4/9] Move data check to fragment --- .../app/browser/DownloadConfirmationFragment.kt | 4 ++-- .../app/browser/downloader/FileDownloader.kt | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) 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 5cc783dfce48..f865e7224f08 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -26,6 +26,7 @@ import android.widget.Toast import androidx.core.content.FileProvider.getUriForFile import com.duckduckgo.app.browser.downloader.FileDownloader 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 @@ -51,8 +52,7 @@ class DownloadConfirmationFragment( } private fun setupFile() { - val guessedFileName = pendingDownload.guessFileName() - file = if (guessedFileName != null) File(pendingDownload.directory, guessedFileName) else null + file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, pendingDownload.guessFileName()) else null } private fun setupViews(view: View) { 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 7bfc8d0f2524..de880228ac2b 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 @@ -36,8 +36,8 @@ class FileDownloader @Inject constructor( } when { - URLUtil.isNetworkUrl(pending.url) -> networkFileDownloader.download(pending) - URLUtil.isDataUrl(pending.url) -> dataUriDownloader.download(pending, callback) + pending.isNetworkUrl -> networkFileDownloader.download(pending) + pending.isDataUrl -> dataUriDownloader.download(pending, callback) else -> callback.downloadFailed("Not supported") } } @@ -58,9 +58,12 @@ class FileDownloader @Inject constructor( } } -fun FileDownloader.PendingFileDownload.guessFileName(): String? { - if (URLUtil.isDataUrl(url)) return null +fun FileDownloader.PendingFileDownload.guessFileName(): String { val guessedFileName = URLUtil.guessFileName(url, contentDisposition, mimeType) Timber.i("Guessed filename of $guessedFileName for url ${url}") return guessedFileName -} \ No newline at end of file +} + +val FileDownloader.PendingFileDownload.isDataUrl get() = URLUtil.isDataUrl(url) + +val FileDownloader.PendingFileDownload.isNetworkUrl get() = URLUtil.isNetworkUrl(url) \ No newline at end of file From bda10260eb20251c1a809250b029a5d036c4e6a3 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Thu, 9 Apr 2020 13:42:39 +0100 Subject: [PATCH 5/9] Make method private --- .../main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d250d02d861c..833316855c12 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1135,7 +1135,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { } } - fun completeDownload(pendingDownload: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + private fun completeDownload(pendingDownload: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { thread { fileDownloader.download(pendingDownload, callback) } From affcbdfca1e0e684efb847e9c248d79d04ce2ea8 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Thu, 9 Apr 2020 13:45:34 +0100 Subject: [PATCH 6/9] Move file var out of constructor --- .../duckduckgo/app/browser/DownloadConfirmationFragment.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 f865e7224f08..b21148ccd238 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -40,10 +40,11 @@ import kotlin.concurrent.thread class DownloadConfirmationFragment( private val pendingDownload: FileDownloader.PendingFileDownload, private val downloader: FileDownloader, - private val downloadListener: FileDownloader.FileDownloadListener, - private var file: File? = null + private val downloadListener: FileDownloader.FileDownloadListener ) : BottomSheetDialogFragment() { + private var file: File? = null + override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.download_confirmation, container, false) setupFile() From f9c6011ab60b5d6602cc194947153bc4ee03024c Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Thu, 9 Apr 2020 21:50:02 +0100 Subject: [PATCH 7/9] Dismiss confirmation dialog on pause --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 8 +++++++- .../app/browser/DownloadConfirmationFragment.kt | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) 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 833316855c12..f66951ebbb6e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -385,9 +385,15 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { override fun onPause() { daxDialog = null logoHidingListener.onPause() + dismissDownloadFragment() super.onPause() } + private fun dismissDownloadFragment() { + val fragment = fragmentManager?.findFragmentByTag(DOWNLAOD_CONFIRM_TAG) as? DownloadConfirmationFragment + fragment?.dismiss() + } + private fun createPopupMenu() { popupMenu = BrowserPopupMenu(layoutInflater) val view = popupMenu.contentView @@ -1130,7 +1136,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloader.FileDownloadListener) { fragmentManager?.let { - val downloadConfirmationFragment = DownloadConfirmationFragment(pendingDownload, fileDownloader, downloadListener) + val downloadConfirmationFragment = DownloadConfirmationFragment(pendingDownload, downloadListener) downloadConfirmationFragment.show(it, DOWNLAOD_CONFIRM_TAG) } } 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 b21148ccd238..91b110c76214 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -35,14 +35,17 @@ 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 pendingDownload: FileDownloader.PendingFileDownload, - private val downloader: FileDownloader, private val downloadListener: FileDownloader.FileDownloadListener ) : BottomSheetDialogFragment() { + @Inject + lateinit var downloader: FileDownloader + private var file: File? = null override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { From 89864e1ff86f98b75093289b20c4f92f20cfb7dc Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Fri, 10 Apr 2020 00:08:03 +0100 Subject: [PATCH 8/9] Use correct no arg constructor for fragment --- .../app/browser/BrowserTabFragment.kt | 37 +++++++++++------- .../browser/DownloadConfirmationFragment.kt | 38 +++++++++++++++---- .../app/browser/downloader/FileDownloader.kt | 3 +- .../duckduckgo/app/di/AndroidBindingModule.kt | 4 ++ 4 files changed, 61 insertions(+), 21 deletions(-) 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 f66951ebbb6e..998a39874cd8 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -93,6 +93,7 @@ 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.filechooser.FileChooserIntentBuilder import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials @@ -294,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? { @@ -390,7 +399,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { } private fun dismissDownloadFragment() { - val fragment = fragmentManager?.findFragmentByTag(DOWNLAOD_CONFIRM_TAG) as? DownloadConfirmationFragment + val fragment = fragmentManager?.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG) as? DownloadConfirmationFragment fragment?.dismiss() } @@ -1110,7 +1119,16 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { return } - val downloadListener = object : FileDownloader.FileDownloadListener { + val downloadListener = createDownloadListener() + if (requestUserConfirmation) { + requestDownloadConfirmation(pendingDownload, downloadListener) + } else { + completeDownload(pendingDownload, downloadListener) + } + } + + private fun createDownloadListener(): FileDownloadListener { + return object : FileDownloadListener { override fun downloadStarted() { fileDownloadNotificationManager.showDownloadInProgressNotification() } @@ -1126,22 +1144,15 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { fileDownloadNotificationManager.showDownloadFailedNotification() } } - - if (requestUserConfirmation) { - requestDownloadConfirmation(pendingDownload, downloadListener) - } else { - completeDownload(pendingDownload, downloadListener) - } } - private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloader.FileDownloadListener) { + private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener) { fragmentManager?.let { - val downloadConfirmationFragment = DownloadConfirmationFragment(pendingDownload, downloadListener) - downloadConfirmationFragment.show(it, DOWNLAOD_CONFIRM_TAG) + DownloadConfirmationFragment.instance(pendingDownload, downloadListener).show(it, DOWNLOAD_CONFIRMATION_TAG) } } - private fun completeDownload(pendingDownload: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + private fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloadListener) { thread { fileDownloader.download(pendingDownload, callback) } @@ -1208,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 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 91b110c76214..55e932abfda9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -25,12 +25,15 @@ 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 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 @@ -38,24 +41,31 @@ import java.io.IOException import javax.inject.Inject import kotlin.concurrent.thread -class DownloadConfirmationFragment( - private val pendingDownload: FileDownloader.PendingFileDownload, - private val downloadListener: FileDownloader.FileDownloadListener -) : BottomSheetDialogFragment() { +class DownloadConfirmationFragment : BottomSheetDialogFragment() { @Inject lateinit var downloader: FileDownloader + lateinit var downloadListener: FileDownloadListener + + private lateinit var pendingDownload: 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) - setupFile() + setupDownload() setupViews(view) return view } - private fun setupFile() { + private fun setupDownload() { + pendingDownload = arguments!![PENDING_DOWNLOAD_BUNDLE_KEY] as PendingFileDownload file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, pendingDownload.guessFileName()) else null } @@ -100,7 +110,7 @@ class DownloadConfirmationFragment( } } - private fun completeDownload(pendingDownload: FileDownloader.PendingFileDownload?, callback: FileDownloader.FileDownloadListener) { + private fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloadListener) { thread { downloader.download(pendingDownload, callback) } @@ -126,4 +136,18 @@ class DownloadConfirmationFragment( 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 + } + } } \ No newline at end of file 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 de880228ac2b..1c21d002716c 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 @@ -21,6 +21,7 @@ import android.webkit.URLUtil import androidx.annotation.WorkerThread import timber.log.Timber import java.io.File +import java.io.Serializable import javax.inject.Inject class FileDownloader @Inject constructor( @@ -49,7 +50,7 @@ class FileDownloader @Inject constructor( val subfolder: String, val userAgent: String, val directory: File = Environment.getExternalStoragePublicDirectory(subfolder) - ) + ): Serializable interface FileDownloadListener { fun downloadStarted() diff --git a/app/src/main/java/com/duckduckgo/app/di/AndroidBindingModule.kt b/app/src/main/java/com/duckduckgo/app/di/AndroidBindingModule.kt index 4d7ac174deaf..c47d80824cf0 100644 --- a/app/src/main/java/com/duckduckgo/app/di/AndroidBindingModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/AndroidBindingModule.kt @@ -21,6 +21,7 @@ import com.duckduckgo.app.bookmarks.ui.BookmarksActivity import com.duckduckgo.app.brokensite.BrokenSiteActivity import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment +import com.duckduckgo.app.browser.DownloadConfirmationFragment import com.duckduckgo.app.browser.rating.ui.AppEnjoymentDialogFragment import com.duckduckgo.app.browser.rating.ui.GiveFeedbackDialogFragment import com.duckduckgo.app.browser.rating.ui.RateAppDialogFragment @@ -133,6 +134,9 @@ abstract class AndroidBindingModule { @ContributesAndroidInjector abstract fun browserTabFragment(): BrowserTabFragment + @ContributesAndroidInjector + abstract fun downloadConfirmationFragment(): DownloadConfirmationFragment + @ContributesAndroidInjector abstract fun onboardingDefaultBrowserFragment(): DefaultBrowserPage From 46527180a859df04153ba2949b3fc280ac3eb326 Mon Sep 17 00:00:00 2001 From: Mia Alexiou Date: Fri, 10 Apr 2020 12:30:42 +0100 Subject: [PATCH 9/9] Make pending download non-nullable --- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- .../app/browser/DownloadConfirmationFragment.kt | 7 ++++--- .../app/browser/downloader/DataUriDownloader.kt | 3 +-- .../duckduckgo/app/browser/downloader/FileDownloader.kt | 9 ++------- 4 files changed, 8 insertions(+), 13 deletions(-) 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 998a39874cd8..e2b59724e9d4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1152,7 +1152,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope { } } - private fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloadListener) { + private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) { thread { fileDownloader.download(pendingDownload, callback) } 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 55e932abfda9..ccec131898f1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -48,7 +48,9 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { lateinit var downloadListener: FileDownloadListener - private lateinit var pendingDownload: PendingFileDownload + private val pendingDownload: PendingFileDownload by lazy { + arguments!![PENDING_DOWNLOAD_BUNDLE_KEY] as PendingFileDownload + } private var file: File? = null @@ -65,7 +67,6 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { } private fun setupDownload() { - pendingDownload = arguments!![PENDING_DOWNLOAD_BUNDLE_KEY] as PendingFileDownload file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, pendingDownload.guessFileName()) else null } @@ -110,7 +111,7 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { } } - private fun completeDownload(pendingDownload: PendingFileDownload?, callback: FileDownloadListener) { + private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) { thread { downloader.download(pendingDownload, callback) } 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 2ae9bc331385..9b684783faa0 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 @@ -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") 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 1c21d002716c..f1824c92e802 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 @@ -30,12 +30,7 @@ class FileDownloader @Inject constructor( ) { @WorkerThread - fun download(pending: PendingFileDownload?, callback: FileDownloadListener) { - - if (pending == null) { - return - } - + fun download(pending: PendingFileDownload, callback: FileDownloadListener) { when { pending.isNetworkUrl -> networkFileDownloader.download(pending) pending.isDataUrl -> dataUriDownloader.download(pending, callback) @@ -61,7 +56,7 @@ class FileDownloader @Inject constructor( fun FileDownloader.PendingFileDownload.guessFileName(): String { val guessedFileName = URLUtil.guessFileName(url, contentDisposition, mimeType) - Timber.i("Guessed filename of $guessedFileName for url ${url}") + Timber.i("Guessed filename of $guessedFileName for url $url") return guessedFileName }