From 89ee738ffd0b86733812f8ca5d19081052211662 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Fri, 20 Jul 2018 13:26:30 +0100 Subject: [PATCH 1/6] Store last seen global state; likely a small bug that we weren't already --- .../main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 ++ 1 file changed, 2 insertions(+) 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 59b7c1791fca..271faa76a50c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -827,6 +827,8 @@ class BrowserTabFragment : Fragment(), FindListener { fun renderGlobalViewState(viewState: GlobalLayoutViewState) { renderIfChanged(viewState, lastSeenGlobalViewState) { + lastSeenGlobalViewState = viewState + if (viewState.isNewTabState) { newTabLayout.show() browserLayout.hide() From ec198a9791a2e9daff65d29c3ba461eb32953098 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Mon, 23 Jul 2018 10:58:01 +0100 Subject: [PATCH 2/6] Wire tab activity and FireDialog to delete web view sessions --- .../app/browser/BrowserTabViewModelTest.kt | 42 +++++- .../app/browser/WebDataManagerTest.kt | 5 +- .../app/tabs/ui/TabSwitcherViewModelTest.kt | 3 +- .../duckduckgo/app/browser/BrowserActivity.kt | 5 +- .../app/browser/BrowserTabFragment.kt | 13 +- .../app/browser/BrowserTabViewModel.kt | 23 ++++ .../duckduckgo/app/browser/WebDataManager.kt | 7 +- .../app/browser/di/BrowserModule.kt | 12 ++ .../browser/session/WebViewSessionStorage.kt | 129 ++++++++++++++++++ .../duckduckgo/app/global/ViewModelFactory.kt | 7 +- .../duckduckgo/app/global/view/FireDialog.kt | 21 ++- .../app/tabs/ui/TabSwitcherActivity.kt | 6 +- .../app/tabs/ui/TabSwitcherViewModel.kt | 6 +- 13 files changed, 255 insertions(+), 24 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt 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 4d85c6d59e34..139dd926c5cb 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -34,6 +34,7 @@ import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserNotification import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter +import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.global.db.AppConfigurationDao import com.duckduckgo.app.global.db.AppConfigurationEntity import com.duckduckgo.app.global.db.AppDatabase @@ -110,6 +111,9 @@ class BrowserTabViewModelTest { @Mock private lateinit var tabsDao: TabsDao + @Mock + private lateinit var webViewSessionStorage: WebViewSessionStorage + @Captor private lateinit var commandCaptor: ArgumentCaptor @@ -143,7 +147,8 @@ class BrowserTabViewModelTest { defaultBrowserNotification = mockDefaultBrowserNotification, defaultBrowserDetector = mockDefaultBrowserDetector, longPressHandler = mockLongPressHandler, - appConfigurationDao = appConfigurationDao + appConfigurationDao = appConfigurationDao, + webViewSessionStorage = webViewSessionStorage ) testee.loadData("abc", null) @@ -684,6 +689,40 @@ class BrowserTabViewModelTest { verify(mockCommandObserver, never()).onChanged(any()) } + @Test + fun whenWebSessionRestoredThenGlobalLayoutSwitchedToShowingBrowser() { + testee.onWebSessionRestored() + assertFalse(globalLayoutViewState().isNewTabState) + } + + @Test + fun whenWebViewSessionIsToBeSavedThenUnderlyingSessionStoredCalled() { + testee.saveWebViewState(null, "") + verify(webViewSessionStorage).saveSession(anyOrNull(), anyString()) + } + + @Test + fun whenRestoringWebViewSessionNotRestorableThenPreviousUrlLoaded() { + whenever(mockOmnibarConverter.convertQueryToUrl("foo.com")).thenReturn("foo.com") + whenever(webViewSessionStorage.restoreSession(anyOrNull(), anyString())).thenReturn(false) + testee.restoreWebViewState(null, "foo.com") + assertEquals("foo.com", testee.url.value) + } + + @Test + fun whenRestoringWebViewSessionNotRestorableAndNoPreviousUrlThenNoUrlLoaded() { + whenever(webViewSessionStorage.restoreSession(anyOrNull(), anyString())).thenReturn(false) + testee.restoreWebViewState(null, "") + assertNull(testee.url.value) + } + + @Test + fun whenWebViewSessionRestorableThenSessionRestored() { + whenever(webViewSessionStorage.restoreSession(anyOrNull(), anyString())).thenReturn(true) + testee.restoreWebViewState(null, "") + assertFalse(globalLayoutViewState().isNewTabState) + } + private fun captureCommands(): ArgumentCaptor { verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(commandCaptor.capture()) return commandCaptor @@ -694,4 +733,5 @@ class BrowserTabViewModelTest { private fun loadingViewState() = testee.loadingViewState.value!! private fun autoCompleteViewState() = testee.autoCompleteViewState.value!! private fun findInPageViewState() = testee.findInPageViewState.value!! + private fun globalLayoutViewState() = testee.globalLayoutState.value!! } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebDataManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebDataManagerTest.kt index 260a7bec30b4..c469992a8eb9 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebDataManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebDataManagerTest.kt @@ -23,6 +23,7 @@ import android.webkit.CookieManager import android.webkit.ValueCallback import android.webkit.WebStorage import android.webkit.WebView +import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage import com.nhaarman.mockito_kotlin.* import org.junit.Assert.assertTrue import org.junit.Test @@ -36,7 +37,7 @@ class WebDataManagerTest { private val mockStorage: WebStorage = mock() - private val testee = WebDataManager(host) + private val testee = WebDataManager(host, WebViewSessionInMemoryStorage()) @UiThreadTest @Test @@ -60,7 +61,7 @@ class WebDataManagerTest { whenever(mockCookieManager.getCookie(host)).thenReturn("da=abc; dz=zyx") whenever(mockCookieManager.getCookie(externalHost)).thenReturn("ea=abc; ez=zyx") - testee.clearExternalCookies(mockCookieManager, {}) + testee.clearExternalCookies(mockCookieManager) {} val captor = argumentCaptor>() verify(mockCookieManager).removeAllCookies(captor.capture()) diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt index 98d171fe83c2..7be5703f21cb 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt @@ -19,6 +19,7 @@ package com.duckduckgo.app.tabs.ui import android.arch.core.executor.testing.InstantTaskExecutorRule import android.arch.lifecycle.Observer import com.duckduckgo.app.browser.R +import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command @@ -53,7 +54,7 @@ class TabSwitcherViewModelTest { fun before() { MockitoAnnotations.initMocks(this) whenever(mockTabRepository.add()).thenReturn("TAB_ID") - testee = TabSwitcherViewModel(mockTabRepository) + testee = TabSwitcherViewModel(mockTabRepository, WebViewSessionInMemoryStorage()) testee.command.observeForever(mockCommandObserver) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index b1146b1a8b35..5277f29b74ed 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -45,6 +45,9 @@ class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var viewModelFactory: ViewModelFactory + @Inject + lateinit var webDataManager: WebDataManager + private var currentTab: BrowserTabFragment? = null private val viewModel: BrowserViewModel by lazy { @@ -167,7 +170,7 @@ class BrowserActivity : DuckDuckGoActivity() { } fun launchFire() { - FireDialog(context = this, + FireDialog(context = this, webDataManager = webDataManager, clearStarted = { viewModel.onClearRequested() }, clearComplete = { viewModel.onClearComplete() } ).show() 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 271faa76a50c..53e461296db5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -60,6 +60,7 @@ import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload import com.duckduckgo.app.browser.filechooser.FileChooserIntentBuilder import com.duckduckgo.app.browser.omnibar.KeyboardAwareEditText +import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.useragent.UserAgentProvider import com.duckduckgo.app.global.ViewModelFactory import com.duckduckgo.app.global.view.* @@ -104,6 +105,9 @@ class BrowserTabFragment : Fragment(), FindListener { @Inject lateinit var fileDownloadNotificationManager: FileDownloadNotificationManager + @Inject + lateinit var webViewSessionStorage: WebViewSessionStorage + val tabId get() = arguments!![TAB_ID_ARG] as String private val initialUrl get() = arguments!![URL_EXTRA_ARG] as String? @@ -591,14 +595,19 @@ class BrowserTabFragment : Fragment(), FindListener { } } + /** + * Attempting to save the WebView's state can result in a TransactionTooLargeException being thrown. + * This will only happen if the bundle size is too large - but the exact size is undefined. + * Instead of saving using normal Android state mechanism - use our own implementation instead. + */ override fun onSaveInstanceState(bundle: Bundle) { - webView?.saveState(bundle) + viewModel.saveWebViewState(webView, tabId) super.onSaveInstanceState(bundle) } override fun onViewStateRestored(bundle: Bundle?) { + viewModel.restoreWebViewState(webView, omnibarTextInput.text.toString()) super.onViewStateRestored(bundle) - webView?.restoreState(bundle) } override fun onHiddenChanged(hidden: Boolean) { 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 e2733a5e4c23..b02d7aee74d6 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -41,6 +41,7 @@ import com.duckduckgo.app.browser.LongPressHandler.RequiredAction import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserNotification import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter +import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.global.SingleLiveEvent import com.duckduckgo.app.global.db.AppConfigurationDao import com.duckduckgo.app.global.db.AppConfigurationEntity @@ -77,6 +78,7 @@ class BrowserTabViewModel( private val defaultBrowserDetector: DefaultBrowserDetector, private val defaultBrowserNotification: DefaultBrowserNotification, private val longPressHandler: LongPressHandler, + private val webViewSessionStorage: WebViewSessionStorage, appConfigurationDao: AppConfigurationDao ) : WebViewClientListener, SaveBookmarkListener, ViewModel() { @@ -508,6 +510,10 @@ class BrowserTabViewModel( numberMatches = numberOfMatches) } + fun onWebSessionRestored() { + globalLayoutState.value = GlobalLayoutViewState(isNewTabState = false) + } + fun desktopSiteModeToggled(urlString: String?, desktopSiteRequested: Boolean) { val currentBrowserViewState = currentBrowserViewState() browserViewState.value = currentBrowserViewState.copy(isDesktopBrowsingMode = desktopSiteRequested) @@ -559,6 +565,23 @@ class BrowserTabViewModel( val currentDefaultBrowserViewState = currentDefaultBrowserViewState() defaultBrowserViewState.value = currentDefaultBrowserViewState.copy(showHomeScreenCallToActionButton = false) } + + fun saveWebViewState(webView: WebView?, tabId: String) { + webViewSessionStorage.saveSession(webView, tabId) + } + + fun restoreWebViewState(webView: WebView?, lastUrl: String) { + val sessionRestored = webViewSessionStorage.restoreSession(webView, tabId) + if (sessionRestored) { + Timber.v("Successfully restored session") + onWebSessionRestored() + } else { + if (lastUrl.isNotBlank()) { + Timber.w("Restoring last url but page history has been lost - url=[$lastUrl]") + onUserSubmittedQuery(lastUrl) + } + } + } } 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 af88df33cd20..18346f9c3835 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt @@ -22,8 +22,9 @@ import android.webkit.CookieManager import android.webkit.WebStorage import android.webkit.WebView import android.webkit.WebViewDatabase +import com.duckduckgo.app.browser.session.WebViewSessionStorage -class WebDataManager(private val host: String) { +class WebDataManager(private val host: String, private val webViewSessionStorage: WebViewSessionStorage) { fun clearData(webView: WebView, webStorage: WebStorage, context: Context) { webView.clearCache(true) @@ -53,4 +54,8 @@ class WebDataManager(private val host: String) { clearAllCallback() } } + + fun clearWebViewSessions() { + webViewSessionStorage.deleteAllSessions() + } } 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 917249ff4eaf..ddeccfddc5af 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 @@ -20,11 +20,15 @@ import android.content.Context import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.defaultBrowsing.AndroidDefaultBrowserDetector import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserDetector +import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage +import com.duckduckgo.app.browser.session.WebViewSessionStorage +import com.duckduckgo.app.global.AppUrl import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.statistics.VariantManager import com.duckduckgo.app.statistics.store.StatisticsDataStore import dagger.Module import dagger.Provides +import javax.inject.Singleton @Module class BrowserModule { @@ -47,4 +51,12 @@ class BrowserModule { fun defaultWebBrowserCapability(context: Context, appInstallStore: AppInstallStore): DefaultBrowserDetector { return AndroidDefaultBrowserDetector(context, appInstallStore) } + + @Singleton + @Provides + fun webViewSessionStorage(): WebViewSessionStorage = WebViewSessionInMemoryStorage() + + @Singleton + @Provides + fun webDataManager(webViewSessionStorage: WebViewSessionStorage) = WebDataManager(AppUrl.Url.HOST, webViewSessionStorage) } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt new file mode 100644 index 000000000000..0eab6b4a6df1 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt @@ -0,0 +1,129 @@ +/* + * Copyright (c) 2018 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.session + +import android.os.Bundle +import android.os.Parcel +import android.util.LruCache +import android.webkit.WebView +import timber.log.Timber + + +interface WebViewSessionStorage { + fun saveSession(webView: WebView?, tabId: String) + fun restoreSession(webView: WebView?, tabId: String): Boolean + fun deleteSession(tabId: String) + fun deleteAllSessions() +} + +class WebViewSessionInMemoryStorage : WebViewSessionStorage { + + private val map = object: LruCache(CACHE_SIZE) { + + /** + * We can calculate this however we choose, but it should match up with the value we used for cache size. + * i.e., if we specify max cache size in bytes, we should calculate an approximate size of the cache entry in bytes. + */ + override fun sizeOf(key: String, bundle: Bundle) = bundle.sizeInBytes() + + override fun entryRemoved(evicted: Boolean, key: String?, oldValue: Bundle?, newValue: Bundle?) { + if (evicted) { + Timber.v("Evicted $key from WebView session storage") + } + } + } + + override fun saveSession(webView: WebView?, tabId: String) { + if (webView == null) { + Timber.w("WebView is null; cannot save session") + return + } + + Timber.i("Saving WebView session for $tabId") + + val webViewBundle = createWebViewBundle(webView) + + val bundle = Bundle() + bundle.putBundle(CACHE_KEY_WEBVIEW, webViewBundle) + bundle.putInt(CACHE_KEY_SCROLL_POSITION, webView.scrollY) + map.put(tabId, bundle) + + Timber.d("Stored ${bundle.sizeInBytes()} bytes for WebView $webView") + logCacheSize() + } + + private fun createWebViewBundle(webView: WebView): Bundle { + return Bundle().also { + webView.saveState(it) + } + } + + override fun restoreSession(webView: WebView?, tabId: String): Boolean { + if (webView == null) { + Timber.w("WebView is null; cannot restore session") + return false + } + + Timber.i("Restoring WebView session for $tabId") + + val bundle = map[tabId] + if (bundle == null) { + Timber.v("No saved bundle for tab $tabId") + return false + } + + val webViewBundle = bundle.getBundle(CACHE_KEY_WEBVIEW) + webView.restoreState(webViewBundle) + webView.scrollY = bundle.getInt(CACHE_KEY_SCROLL_POSITION) + map.remove(tabId) + + logCacheSize() + + return true + } + + override fun deleteSession(tabId: String) { + map.remove(tabId) + Timber.i("Deleted web session for $tabId") + logCacheSize() + } + + override fun deleteAllSessions() = map.evictAll() + + private fun logCacheSize() { + Timber.v("Cache size is now ~${map.size()} bytes out of a max size of ${map.maxSize()} bytes") + } + + private fun Bundle.sizeInBytes(): Int { + val parcel = Parcel.obtain() + parcel.writeValue(this) + + val bytes = parcel.marshall() + parcel.recycle() + + return bytes.size + } + + companion object { + private const val CACHE_SIZE = 10 * 1024 * 1024 // 10 MB + + private const val CACHE_KEY_WEBVIEW = "webview" + private const val CACHE_KEY_SCROLL_POSITION = "scroll-position" + + } + +} \ No newline at end of file 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 c732b286868d..925d2aa7d0f6 100644 --- a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt @@ -28,6 +28,7 @@ import com.duckduckgo.app.browser.LongPressHandler import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.defaultBrowsing.DefaultBrowserNotification import com.duckduckgo.app.browser.omnibar.QueryUrlConverter +import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.feedback.api.FeedbackSender import com.duckduckgo.app.feedback.ui.FeedbackViewModel import com.duckduckgo.app.global.db.AppConfigurationDao @@ -68,7 +69,8 @@ class ViewModelFactory @Inject constructor( private val webViewLongPressHandler: LongPressHandler, private val defaultBrowserDetector: DefaultBrowserDetector, private val variantManager: VariantManager, - private val feedbackSender: FeedbackSender + private val feedbackSender: FeedbackSender, + private val webViewSessionStorage: WebViewSessionStorage ) : ViewModelProvider.NewInstanceFactory() { @@ -79,7 +81,7 @@ class ViewModelFactory @Inject constructor( isAssignableFrom(OnboardingViewModel::class.java) -> OnboardingViewModel(onboaringStore, defaultBrowserDetector, variantManager) isAssignableFrom(BrowserViewModel::class.java) -> BrowserViewModel(tabRepository, queryUrlConverter) isAssignableFrom(BrowserTabViewModel::class.java) -> browserTabViewModel() - isAssignableFrom(TabSwitcherViewModel::class.java) -> TabSwitcherViewModel(tabRepository) + isAssignableFrom(TabSwitcherViewModel::class.java) -> TabSwitcherViewModel(tabRepository, webViewSessionStorage) isAssignableFrom(PrivacyDashboardViewModel::class.java) -> PrivacyDashboardViewModel(privacySettingsStore, networkLeaderboardDao) isAssignableFrom(ScorecardViewModel::class.java) -> ScorecardViewModel(privacySettingsStore) isAssignableFrom(TrackerNetworksViewModel::class.java) -> TrackerNetworksViewModel() @@ -104,6 +106,7 @@ class ViewModelFactory @Inject constructor( defaultBrowserNotification = defaultBrowserNotification, appConfigurationDao = appConfigurationDao, longPressHandler = webViewLongPressHandler, + webViewSessionStorage = webViewSessionStorage, autoCompleteApi = autoCompleteApi ) } diff --git a/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt index 3521cb3a2851..47ae1f84ac8c 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt @@ -24,29 +24,28 @@ import android.webkit.WebStorage import android.webkit.WebView import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.WebDataManager -import com.duckduckgo.app.global.AppUrl.Url import kotlinx.android.synthetic.main.sheet_fire_clear_data.* -class FireDialog(context: Context, clearStarted: (() -> Unit), clearComplete: (() -> Unit)) : - BottomSheetDialog(context) { +class FireDialog( + context: Context, + webDataManager: WebDataManager, + clearStarted: (() -> Unit), + clearComplete: (() -> Unit) +) : BottomSheetDialog(context) { init { - - val dataManager = WebDataManager(Url.HOST) val contentView = View.inflate(getContext(), R.layout.sheet_fire_clear_data, null) - setContentView(contentView) - clearAllOption.setOnClickListener { clearStarted() - dataManager.clearData(WebView(context), WebStorage.getInstance(), context) - dataManager.clearExternalCookies(CookieManager.getInstance(), clearComplete) + webDataManager.clearData(WebView(context), WebStorage.getInstance(), context) + webDataManager.clearExternalCookies(CookieManager.getInstance(), clearComplete) + webDataManager.clearWebViewSessions() dismiss() } - cancelOption.setOnClickListener { dismiss() } - } + } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index 3f528ddb07e1..295234d04404 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -25,6 +25,7 @@ import android.support.v7.widget.LinearLayoutManager import android.view.Menu import android.view.MenuItem import com.duckduckgo.app.browser.R +import com.duckduckgo.app.browser.WebDataManager import com.duckduckgo.app.global.DuckDuckGoActivity import com.duckduckgo.app.global.ViewModelFactory import com.duckduckgo.app.global.view.FireDialog @@ -42,6 +43,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherAdapter.TabSwitched @Inject lateinit var viewModelFactory: ViewModelFactory + @Inject + lateinit var webDataManager: WebDataManager + private val viewModel: TabSwitcherViewModel by lazy { ViewModelProviders.of(this, viewModelFactory).get(TabSwitcherViewModel::class.java) } @@ -99,7 +103,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherAdapter.TabSwitched } private fun onFire() { - FireDialog(context = this, + FireDialog(context = this, webDataManager = webDataManager, clearStarted = { viewModel.onClearRequested() }, clearComplete = { viewModel.onClearComplete() } ).show() diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt index 9c0b21a11b16..d2c00b78b07b 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt @@ -20,11 +20,12 @@ import android.arch.lifecycle.LiveData import android.arch.lifecycle.ViewModel import android.support.annotation.StringRes import com.duckduckgo.app.browser.R +import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.global.SingleLiveEvent -import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.app.tabs.model.TabEntity +import com.duckduckgo.app.tabs.model.TabRepository -class TabSwitcherViewModel(private val tabRepository: TabRepository) : ViewModel() { +class TabSwitcherViewModel(private val tabRepository: TabRepository, private val webViewSessionStorage: WebViewSessionStorage) : ViewModel() { var tabs: LiveData> = tabRepository.liveTabs val command: SingleLiveEvent = SingleLiveEvent() @@ -46,6 +47,7 @@ class TabSwitcherViewModel(private val tabRepository: TabRepository) : ViewModel fun onTabDeleted(tab: TabEntity) { tabRepository.delete(tab) + webViewSessionStorage.deleteSession(tab.tabId) } fun onClearRequested() { From 887557b799d6f2bab410d9bb29c23a910794ed20 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Mon, 23 Jul 2018 16:26:10 +0100 Subject: [PATCH 3/6] Formatting --- .../java/com/duckduckgo/app/browser/BrowserActivity.kt | 3 ++- .../app/browser/session/WebViewSessionStorage.kt | 2 +- .../main/java/com/duckduckgo/app/global/view/FireDialog.kt | 6 +++--- .../java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 7 ++++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 5277f29b74ed..68388c906045 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -170,7 +170,8 @@ class BrowserActivity : DuckDuckGoActivity() { } fun launchFire() { - FireDialog(context = this, webDataManager = webDataManager, + FireDialog(context = this, + webDataManager = webDataManager, clearStarted = { viewModel.onClearRequested() }, clearComplete = { viewModel.onClearComplete() } ).show() diff --git a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt index 0eab6b4a6df1..ef02204ecf79 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt @@ -35,7 +35,7 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { private val map = object: LruCache(CACHE_SIZE) { /** - * We can calculate this however we choose, but it should match up with the value we used for cache size. + * We can calculate this however we choose, but it should match up with the value we use for cache size. * i.e., if we specify max cache size in bytes, we should calculate an approximate size of the cache entry in bytes. */ override fun sizeOf(key: String, bundle: Bundle) = bundle.sizeInBytes() diff --git a/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt index 47ae1f84ac8c..6ff9d4a4100e 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/FireDialog.kt @@ -31,16 +31,16 @@ class FireDialog( webDataManager: WebDataManager, clearStarted: (() -> Unit), clearComplete: (() -> Unit) -) : BottomSheetDialog(context) { - +) : + BottomSheetDialog(context) { init { val contentView = View.inflate(getContext(), R.layout.sheet_fire_clear_data, null) setContentView(contentView) clearAllOption.setOnClickListener { clearStarted() webDataManager.clearData(WebView(context), WebStorage.getInstance(), context) - webDataManager.clearExternalCookies(CookieManager.getInstance(), clearComplete) webDataManager.clearWebViewSessions() + webDataManager.clearExternalCookies(CookieManager.getInstance(), clearComplete) dismiss() } cancelOption.setOnClickListener { diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index 295234d04404..bca5280f50b0 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -103,10 +103,11 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherAdapter.TabSwitched } private fun onFire() { - FireDialog(context = this, webDataManager = webDataManager, + FireDialog( + context = this, + webDataManager = webDataManager, clearStarted = { viewModel.onClearRequested() }, - clearComplete = { viewModel.onClearComplete() } - ).show() + clearComplete = { viewModel.onClearComplete() }).show() } override fun onNewTabRequested() { From 6a7ec851f493b658deb7f9eb9be663157c45d16b Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 24 Jul 2018 13:28:23 +0100 Subject: [PATCH 4/6] Update cache size comment to indicate 10 MiB instead of 10 MB --- .../duckduckgo/app/browser/session/WebViewSessionStorage.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt index ef02204ecf79..c5788dd8be01 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt @@ -32,11 +32,11 @@ interface WebViewSessionStorage { class WebViewSessionInMemoryStorage : WebViewSessionStorage { - private val map = object: LruCache(CACHE_SIZE) { + private val map = object : LruCache(CACHE_SIZE_BYTES) { /** * We can calculate this however we choose, but it should match up with the value we use for cache size. - * i.e., if we specify max cache size in bytes, we should calculate an approximate size of the cache entry in bytes. + * i.e., we specify the max cache size in bytes, so we need to calculate an approximate size of the cache entry in bytes. */ override fun sizeOf(key: String, bundle: Bundle) = bundle.sizeInBytes() @@ -119,7 +119,7 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { } companion object { - private const val CACHE_SIZE = 10 * 1024 * 1024 // 10 MB + private const val CACHE_SIZE_BYTES = 10 * 1024 * 1024 // 10 MiB private const val CACHE_KEY_WEBVIEW = "webview" private const val CACHE_KEY_SCROLL_POSITION = "scroll-position" From 3759d2813c35c764424e8786d853c38d85379ef0 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 24 Jul 2018 14:23:59 +0100 Subject: [PATCH 5/6] Change variable name from map to cache --- .../app/browser/session/WebViewSessionStorage.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt index c5788dd8be01..d659246165f7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt @@ -32,7 +32,7 @@ interface WebViewSessionStorage { class WebViewSessionInMemoryStorage : WebViewSessionStorage { - private val map = object : LruCache(CACHE_SIZE_BYTES) { + private val cache = object : LruCache(CACHE_SIZE_BYTES) { /** * We can calculate this however we choose, but it should match up with the value we use for cache size. @@ -60,7 +60,7 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { val bundle = Bundle() bundle.putBundle(CACHE_KEY_WEBVIEW, webViewBundle) bundle.putInt(CACHE_KEY_SCROLL_POSITION, webView.scrollY) - map.put(tabId, bundle) + cache.put(tabId, bundle) Timber.d("Stored ${bundle.sizeInBytes()} bytes for WebView $webView") logCacheSize() @@ -80,7 +80,7 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { Timber.i("Restoring WebView session for $tabId") - val bundle = map[tabId] + val bundle = cache[tabId] if (bundle == null) { Timber.v("No saved bundle for tab $tabId") return false @@ -89,7 +89,7 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { val webViewBundle = bundle.getBundle(CACHE_KEY_WEBVIEW) webView.restoreState(webViewBundle) webView.scrollY = bundle.getInt(CACHE_KEY_SCROLL_POSITION) - map.remove(tabId) + cache.remove(tabId) logCacheSize() @@ -97,15 +97,15 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { } override fun deleteSession(tabId: String) { - map.remove(tabId) + cache.remove(tabId) Timber.i("Deleted web session for $tabId") logCacheSize() } - override fun deleteAllSessions() = map.evictAll() + override fun deleteAllSessions() = cache.evictAll() private fun logCacheSize() { - Timber.v("Cache size is now ~${map.size()} bytes out of a max size of ${map.maxSize()} bytes") + Timber.v("Cache size is now ~${cache.size()} bytes out of a max size of ${cache.maxSize()} bytes") } private fun Bundle.sizeInBytes(): Int { From 9a3c25d2c57047aeaa2b33d89d5ce324acd85b16 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 24 Jul 2018 14:25:52 +0100 Subject: [PATCH 6/6] Improve comment on sizeOf function --- .../duckduckgo/app/browser/session/WebViewSessionStorage.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt index d659246165f7..c750c0e42c19 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/session/WebViewSessionStorage.kt @@ -35,8 +35,8 @@ class WebViewSessionInMemoryStorage : WebViewSessionStorage { private val cache = object : LruCache(CACHE_SIZE_BYTES) { /** - * We can calculate this however we choose, but it should match up with the value we use for cache size. - * i.e., we specify the max cache size in bytes, so we need to calculate an approximate size of the cache entry in bytes. + * Size (in bytes) of a single entry in the cache for the given key. + * We specify the max cache size in bytes, so we need to calculate an approximate size of the cache entry in bytes. */ override fun sizeOf(key: String, bundle: Bundle) = bundle.sizeInBytes()