From dec8c621bf583f62c43e76ad1fb2faa0a761442e Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Thu, 20 Jun 2019 15:00:03 +0100 Subject: [PATCH 1/3] Tidy up data clearing APIs --- .../app/browser/WebViewDataManagerTest.kt | 74 ++++++++++++++----- .../view/ClearPersonalDataActionTest.kt | 6 -- .../duckduckgo/app/browser/WebDataManager.kt | 33 ++++----- .../global/view/ClearPersonalDataAction.kt | 3 - 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt index d6010f0a539e..6f372fc6d768 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt @@ -20,14 +20,15 @@ import android.content.Context import android.webkit.WebStorage import android.webkit.WebView import android.webkit.WebViewDatabase -import androidx.test.annotation.UiThreadTest import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage import com.duckduckgo.app.fire.DuckDuckGoCookieManager import com.duckduckgo.app.global.file.FileDeleter import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext import org.junit.Assert.assertTrue import org.junit.Test @@ -38,28 +39,67 @@ class WebViewDataManagerTest { private val mockStorage: WebStorage = mock() private val context = InstrumentationRegistry.getInstrumentation().targetContext private val mockFileDeleter: FileDeleter = mock() + private val mockWebViewDatabase: WebViewDatabase = mock() private val testee = WebViewDataManager(context, WebViewSessionInMemoryStorage(), mockCookieManager, mockFileDeleter) - @UiThreadTest @Test - fun whenDataClearedThenCacheHistoryAndStorageDataCleared() { - val context = InstrumentationRegistry.getInstrumentation().targetContext - val webView = TestWebView(context) - val mockWebViewDatabase = mock() - - testee.clearData(webView, mockStorage, mockWebViewDatabase) - - assertTrue(webView.historyCleared) - assertTrue(webView.cacheCleared) - assertTrue(webView.clearedFormData) - verify(mockStorage).deleteAllData() - verify(mockWebViewDatabase).clearHttpAuthUsernamePassword() + fun whenDataClearedThenWebViewHistoryCleared() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + assertTrue(webView.historyCleared) + } + } + + @Test + fun whenDataClearedThenWebViewCacheCleared() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + assertTrue(webView.cacheCleared) + } + } + + @Test + fun whenDataClearedThenWebViewFormDataCleared() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + assertTrue(webView.clearedFormData) + } } @Test - fun whenExternalCookiesClearedThenCookiesRemoved() = runBlocking { - testee.clearExternalCookies() - verify(mockCookieManager).removeExternalCookies() + fun whenDataClearedThenWebViewWebStorageCleared() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + verify(mockStorage).deleteAllData() + } + } + + @Test + fun whenDataClearedThenWebViewAuthCredentialsCleared() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + verify(mockWebViewDatabase).clearHttpAuthUsernamePassword() + } + } + + @Test + fun whenDataClearedThenWebViewCookiesRemoved() = runBlocking { + withContext(Dispatchers.Main) { + val webView = TestWebView(context) + testee.clearData(webView, mockStorage, mockWebViewDatabase) + + verify(mockCookieManager).removeExternalCookies() + } } private class TestWebView(context: Context) : WebView(context) { diff --git a/app/src/androidTest/java/com/duckduckgo/app/global/view/ClearPersonalDataActionTest.kt b/app/src/androidTest/java/com/duckduckgo/app/global/view/ClearPersonalDataActionTest.kt index 283917505714..b585f7243bea 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/global/view/ClearPersonalDataActionTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/global/view/ClearPersonalDataActionTest.kt @@ -80,12 +80,6 @@ class ClearPersonalDataActionTest { verify(mockDataManager).clearData(any(), any(), any()) } - @Test - fun whenClearCalledThenDataManagerClearsCookies() = runBlocking { - testee.clearTabsAndAllDataAsync(false, false) - verify(mockDataManager).clearExternalCookies() - } - @Test fun whenClearCalledThenAppCacheClearerClearsCache() = runBlocking { testee.clearTabsAndAllDataAsync(false, false) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt index 7f66df228dd7..b65a26859240 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt @@ -28,10 +28,8 @@ import java.io.File import javax.inject.Inject interface WebDataManager { - suspend fun clearExternalCookies() - fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) + suspend fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) fun clearWebViewSessions() - suspend fun deleteWebViewDirectory() } class WebViewDataManager @Inject constructor( @@ -41,20 +39,17 @@ class WebViewDataManager @Inject constructor( private val fileDeleter: FileDeleter ) : WebDataManager { - override fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) { - clearCache(webView) + override suspend fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) { + clearWebViewCacheCache(webView) clearHistory(webView) clearWebStorage(webStorage) - clearFormData(webView) - - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { - clearFormData(webViewDatabase) - } - + clearFormData(webView, webViewDatabase) clearAuthentication(webViewDatabase) + clearExternalCookies() + clearWebViewDirectory(exclusions = WEBVIEW_FILES_EXCLUDED_FROM_DELETION) } - private fun clearCache(webView: WebView) { + private fun clearWebViewCacheCache(webView: WebView) { webView.clearCache(true) } @@ -66,13 +61,17 @@ class WebViewDataManager @Inject constructor( webStorage.deleteAllData() } - private fun clearFormData(webView: WebView) { + private fun clearFormData(webView: WebView, webViewDatabase: WebViewDatabase) { webView.clearFormData() + + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { + clearFormData(webViewDatabase) + } } - override suspend fun deleteWebViewDirectory() { + private suspend fun clearWebViewDirectory(exclusions: List) { val webViewDataDirectory = File(context.applicationInfo.dataDir, WEBVIEW_DATA_DIRECTORY_NAME) - fileDeleter.deleteContents(webViewDataDirectory, FILENAMES_EXCLUDED_FROM_DELETION) + fileDeleter.deleteContents(webViewDataDirectory, exclusions) } /** @@ -87,7 +86,7 @@ class WebViewDataManager @Inject constructor( webViewDatabase.clearHttpAuthUsernamePassword() } - override suspend fun clearExternalCookies() { + private suspend fun clearExternalCookies() { cookieManager.removeExternalCookies() } @@ -98,7 +97,7 @@ class WebViewDataManager @Inject constructor( companion object { private const val WEBVIEW_DATA_DIRECTORY_NAME = "app_webview" - private val FILENAMES_EXCLUDED_FROM_DELETION = listOf( + private val WEBVIEW_FILES_EXCLUDED_FROM_DELETION = listOf( "Cookies", "Local Storage" ) diff --git a/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt b/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt index 80a544a99f40..68f6295d372a 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt @@ -105,9 +105,6 @@ class ClearPersonalDataAction @Inject constructor( } dataManager.clearData(createWebView(), createWebStorage(), WebViewDatabase.getInstance(context)) - dataManager.clearExternalCookies() - dataManager.deleteWebViewDirectory() - appCacheClearer.clearCache() Timber.i("Finished clearing data") From 4a61ccf1be4b117f4f509ccf6a9d89dc8b89ac8d Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Thu, 20 Jun 2019 15:06:34 +0100 Subject: [PATCH 2/3] Removing empty lines --- .../com/duckduckgo/app/browser/WebViewDataManagerTest.kt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt index 6f372fc6d768..167db53cb369 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt @@ -47,7 +47,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - assertTrue(webView.historyCleared) } } @@ -57,7 +56,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - assertTrue(webView.cacheCleared) } } @@ -67,7 +65,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - assertTrue(webView.clearedFormData) } } @@ -77,7 +74,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - verify(mockStorage).deleteAllData() } } @@ -87,7 +83,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - verify(mockWebViewDatabase).clearHttpAuthUsernamePassword() } } @@ -97,7 +92,6 @@ class WebViewDataManagerTest { withContext(Dispatchers.Main) { val webView = TestWebView(context) testee.clearData(webView, mockStorage, mockWebViewDatabase) - verify(mockCookieManager).removeExternalCookies() } } From 3aabc8525782ee1ec71b4e13a7b043252a991b79 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Thu, 20 Jun 2019 15:11:19 +0100 Subject: [PATCH 3/3] Remove typo from method name --- .../main/java/com/duckduckgo/app/browser/WebDataManager.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt index b65a26859240..e58839eea54e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt @@ -40,7 +40,7 @@ class WebViewDataManager @Inject constructor( ) : WebDataManager { override suspend fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) { - clearWebViewCacheCache(webView) + clearWebViewCache(webView) clearHistory(webView) clearWebStorage(webStorage) clearFormData(webView, webViewDatabase) @@ -49,7 +49,7 @@ class WebViewDataManager @Inject constructor( clearWebViewDirectory(exclusions = WEBVIEW_FILES_EXCLUDED_FROM_DELETION) } - private fun clearWebViewCacheCache(webView: WebView) { + private fun clearWebViewCache(webView: WebView) { webView.clearCache(true) }