From 54da078bd311145383e52b46aae95549cae909a1 Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Tue, 19 Jan 2021 16:14:51 -0500 Subject: [PATCH] Refactor QuickSettingsDialog to use browser store --- .../fenix/browser/BaseBrowserFragment.kt | 2 +- .../quicksettings/QuickSettingsController.kt | 12 ++++++---- .../QuickSettingsSheetDialogFragment.kt | 3 ++- .../DefaultQuickSettingsControllerTest.kt | 24 ++++++++++++------- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 171fabf8f8..1051ba9b27 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -1136,7 +1136,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, } } - protected fun getCurrentTab(): SessionState? { + private fun getCurrentTab(): SessionState? { return requireComponents.core.store.state.findCustomTabOrSelectedTab(customTabSessionId) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt index b4a57978c5..f93b9c69fe 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt @@ -11,6 +11,8 @@ import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import mozilla.components.browser.session.Session +import mozilla.components.browser.state.selector.findTabOrCustomTab +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.session.SessionUseCases.ReloadUrlUseCase import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.tabs.TabsUseCases.AddNewTabUseCase @@ -78,10 +80,11 @@ interface QuickSettingsController { class DefaultQuickSettingsController( private val context: Context, private val quickSettingsStore: QuickSettingsFragmentStore, + private val browserStore: BrowserStore, private val ioScope: CoroutineScope, private val navController: NavController, @VisibleForTesting - internal val session: Session?, + internal val sessionId: String, @VisibleForTesting internal var sitePermissions: SitePermissions?, private val settings: Settings, @@ -136,7 +139,8 @@ class DefaultQuickSettingsController( val permissions = sitePermissions sitePermissions = if (permissions == null) { - val origin = requireNotNull(session?.url?.toUri()?.host) { + val tab = browserStore.state.findTabOrCustomTab(sessionId) + val origin = requireNotNull(tab?.content?.url?.toUri()?.host) { "An origin is required to change a autoplay settings from the door hanger" } val sitePermissions = @@ -175,7 +179,7 @@ class DefaultQuickSettingsController( fun handlePermissionsChange(updatedPermissions: SitePermissions) { ioScope.launch { permissionStorage.updateSitePermissions(updatedPermissions) - reload(session) + reload(sessionId) } } @@ -183,7 +187,7 @@ class DefaultQuickSettingsController( internal fun handleAutoplayAdd(sitePermissions: SitePermissions) { ioScope.launch { permissionStorage.add(sitePermissions) - reload(session) + reload(sessionId) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsSheetDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsSheetDialogFragment.kt index cd8eb7ca1d..e8047d24bc 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsSheetDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsSheetDialogFragment.kt @@ -77,9 +77,10 @@ class QuickSettingsSheetDialogFragment : AppCompatDialogFragment() { quickSettingsController = DefaultQuickSettingsController( context = context, quickSettingsStore = quickSettingsStore, + browserStore = components.core.store, ioScope = viewLifecycleOwner.lifecycleScope + Dispatchers.IO, navController = findNavController(), - session = components.core.sessionManager.findSessionById(args.sessionId), + sessionId = args.sessionId, sitePermissions = args.sitePermissions, settings = components.settings, permissionStorage = components.core.permissionStorage, diff --git a/app/src/test/java/org/mozilla/fenix/settings/quicksettings/DefaultQuickSettingsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/quicksettings/DefaultQuickSettingsControllerTest.kt index bde294bd2e..57f7e8d6dd 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/quicksettings/DefaultQuickSettingsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/quicksettings/DefaultQuickSettingsControllerTest.kt @@ -18,7 +18,10 @@ import io.mockk.impl.annotations.MockK import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineScope import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.session.Session +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.state.state.createTab +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.session.SessionUseCases import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.sitepermissions.SitePermissions.Status.NO_DECISION @@ -43,6 +46,10 @@ import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) class DefaultQuickSettingsControllerTest { private val context = testContext + + private lateinit var browserStore: BrowserStore + private lateinit var tab: TabSessionState + @MockK private lateinit var store: QuickSettingsFragmentStore private val coroutinesScope = TestCoroutineScope() @@ -51,7 +58,6 @@ class DefaultQuickSettingsControllerTest { private lateinit var navController: NavController @MockK(relaxed = true) - private lateinit var browserSession: Session private lateinit var sitePermissions: SitePermissions @MockK(relaxed = true) @@ -81,15 +87,18 @@ class DefaultQuickSettingsControllerTest { fun setUp() { MockKAnnotations.init(this) + tab = createTab("https://mozilla.org") + browserStore = BrowserStore(BrowserState(tabs = listOf(tab))) sitePermissions = SitePermissions(origin = "", savedAt = 123) controller = spyk( DefaultQuickSettingsController( context = context, quickSettingsStore = store, + browserStore = browserStore, + sessionId = tab.id, ioScope = coroutinesScope, navController = navController, - session = browserSession, sitePermissions = sitePermissions, settings = appSettings, permissionStorage = permissionStorage, @@ -158,9 +167,10 @@ class DefaultQuickSettingsControllerTest { val invalidSitePermissionsController = DefaultQuickSettingsController( context = context, quickSettingsStore = store, + browserStore = BrowserStore(), ioScope = coroutinesScope, navController = navController, - session = browserSession, + sessionId = "123", sitePermissions = null, settings = appSettings, permissionStorage = permissionStorage, @@ -189,7 +199,6 @@ class DefaultQuickSettingsControllerTest { val autoplayValue = mockk(relaxed = true) every { store.dispatch(any()) } returns mockk() - every { browserSession.url } returns "https://www.mozilla.org" every { controller.handleAutoplayAdd(any()) } returns Unit controller.sitePermissions = null @@ -207,7 +216,6 @@ class DefaultQuickSettingsControllerTest { val autoplayValue = mockk(relaxed = true) every { store.dispatch(any()) } returns mockk() - every { browserSession.url } returns "https://www.mozilla.org" every { controller.handleAutoplayAdd(any()) } returns Unit every { controller.handlePermissionsChange(any()) } returns Unit every { autoplayValue.updateSitePermissions(any()) } returns mock() @@ -256,7 +264,7 @@ class DefaultQuickSettingsControllerTest { coVerifyOrder { permissionStorage.updateSitePermissions(testPermissions) - reload(browserSession) + reload(tab.id) } } @@ -269,7 +277,7 @@ class DefaultQuickSettingsControllerTest { coVerifyOrder { permissionStorage.add(testPermissions) - reload(browserSession) + reload(tab.id) } } }