[fenix] Refactor ExternalAppBrowserActivity and ExternalAppBrowserFragment to not use Session(Manager).

pull/600/head
Sebastian Kaspari 3 years ago committed by Christian Sadilek
parent a1246d05f8
commit 086174a280

@ -184,6 +184,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
protected var webAppToolbarShouldBeVisible = true protected var webAppToolbarShouldBeVisible = true
private val sharedViewModel: SharedViewModel by activityViewModels() private val sharedViewModel: SharedViewModel by activityViewModels()
private val homeViewModel: HomeScreenViewModel by activityViewModels()
@VisibleForTesting @VisibleForTesting
internal val onboarding by lazy { FenixOnboarding(requireContext()) } internal val onboarding by lazy { FenixOnboarding(requireContext()) }
@ -220,7 +221,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
} }
final override fun onViewCreated(view: View, savedInstanceState: Bundle?) { final override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
browserInitialized = initializeUI(view) != null initializeUI(view)
if (customTabSessionId == null) { if (customTabSessionId == null) {
// We currently only need this observer to navigate to home // We currently only need this observer to navigate to home
@ -238,12 +239,19 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
requireContext().accessibilityManager.addAccessibilityStateChangeListener(this) requireContext().accessibilityManager.addAccessibilityStateChangeListener(this)
} }
private val homeViewModel: HomeScreenViewModel by activityViewModels() private fun initializeUI(view: View) {
val tab = getCurrentTab()
browserInitialized = if (tab != null) {
initializeUI(view, tab)
true
} else {
false
}
}
@Suppress("ComplexMethod", "LongMethod") @Suppress("ComplexMethod", "LongMethod")
@CallSuper @CallSuper
@VisibleForTesting internal open fun initializeUI(view: View, tab: SessionState) {
internal open fun initializeUI(view: View): Session? {
val context = requireContext() val context = requireContext()
val sessionManager = context.components.core.sessionManager val sessionManager = context.components.core.sessionManager
val store = context.components.core.store val store = context.components.core.store
@ -260,7 +268,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
beginAnimateInIfNecessary() beginAnimateInIfNecessary()
} }
return getSessionById()?.also { _ ->
val openInFenixIntent = Intent(context, IntentReceiverActivity::class.java).apply { val openInFenixIntent = Intent(context, IntentReceiverActivity::class.java).apply {
action = Intent.ACTION_VIEW action = Intent.ACTION_VIEW
putExtra(HomeActivity.OPEN_TO_BROWSER, true) putExtra(HomeActivity.OPEN_TO_BROWSER, true)
@ -289,9 +296,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
) )
}, },
onCloseTab = { closedSession -> onCloseTab = { closedSession ->
val tab = store.state.findTab(closedSession.id) ?: return@DefaultBrowserToolbarController val closedTab = store.state.findTab(closedSession.id) ?: return@DefaultBrowserToolbarController
val snackbarMessage = if (tab.content.private) { val snackbarMessage = if (closedTab.content.private) {
requireContext().getString(R.string.snackbar_private_tab_closed) requireContext().getString(R.string.snackbar_private_tab_closed)
} else { } else {
requireContext().getString(R.string.snackbar_tab_closed) requireContext().getString(R.string.snackbar_tab_closed)
@ -710,7 +717,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
initializeEngineView(toolbarHeight) initializeEngineView(toolbarHeight)
} }
}
@VisibleForTesting @VisibleForTesting
internal fun expandToolbarOnNavigation(store: BrowserStore) { internal fun expandToolbarOnNavigation(store: BrowserStore) {
@ -928,9 +934,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
resumeDownloadDialogState(selectedTab.id, context.components.core.store, view, context, toolbarHeight) resumeDownloadDialogState(selectedTab.id, context.components.core.store, view, context, toolbarHeight)
} }
} else { } else {
view?.let { view -> view?.let { view -> initializeUI(view) }
browserInitialized = initializeUI(view) != null
}
} }
} }
@ -1070,7 +1074,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
sitePermissions: SitePermissions? sitePermissions: SitePermissions?
) )
protected abstract fun navToTrackingProtectionPanel(session: Session) protected abstract fun navToTrackingProtectionPanel(tab: SessionState)
/** /**
* Returns the layout [android.view.Gravity] for the quick settings and ETP dialog. * Returns the layout [android.view.Gravity] for the quick settings and ETP dialog.
@ -1108,9 +1112,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
} }
private fun showTrackingProtectionPanel() { private fun showTrackingProtectionPanel() {
val session = getSessionById() ?: return val tab = getCurrentTab() ?: return
view?.let { view?.let {
navToTrackingProtectionPanel(session) navToTrackingProtectionPanel(tab)
} }
} }

@ -15,7 +15,6 @@ import com.google.android.material.snackbar.Snackbar
import kotlinx.android.synthetic.main.fragment_browser.* import kotlinx.android.synthetic.main.fragment_browser.*
import kotlinx.android.synthetic.main.fragment_browser.view.* import kotlinx.android.synthetic.main.fragment_browser.view.*
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import mozilla.components.browser.session.Session
import mozilla.components.browser.state.selector.findTab import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.SessionState
@ -56,11 +55,12 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
private var pwaOnboardingObserver: PwaOnboardingObserver? = null private var pwaOnboardingObserver: PwaOnboardingObserver? = null
@Suppress("LongMethod") @Suppress("LongMethod")
override fun initializeUI(view: View): Session? { override fun initializeUI(view: View, tab: SessionState) {
super.initializeUI(view, tab)
val context = requireContext() val context = requireContext()
val components = context.components val components = context.components
return super.initializeUI(view)?.also {
if (context.settings().isSwipeToolbarToSwitchTabsEnabled) { if (context.settings().isSwipeToolbarToSwitchTabsEnabled) {
gestureLayout.addGestureListener( gestureLayout.addGestureListener(
ToolbarGestureHandler( ToolbarGestureHandler(
@ -158,7 +158,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
) )
} }
} }
}
override fun onStart() { override fun onStart() {
super.onStart() super.onStart()
@ -219,15 +218,15 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
nav(R.id.browserFragment, directions) nav(R.id.browserFragment, directions)
} }
override fun navToTrackingProtectionPanel(session: Session) { override fun navToTrackingProtectionPanel(tab: SessionState) {
val navController = findNavController() val navController = findNavController()
requireComponents.useCases.trackingProtectionUseCases.containsException(session.id) { contains -> requireComponents.useCases.trackingProtectionUseCases.containsException(tab.id) { contains ->
val isEnabled = session.trackerBlockingEnabled && !contains val isEnabled = tab.trackingProtection.enabled && !contains
val directions = val directions =
BrowserFragmentDirections.actionBrowserFragmentToTrackingProtectionPanelDialogFragment( BrowserFragmentDirections.actionBrowserFragmentToTrackingProtectionPanelDialogFragment(
sessionId = session.id, sessionId = tab.id,
url = session.url, url = tab.content.url,
trackingProtectionEnabled = isEnabled, trackingProtectionEnabled = isEnabled,
gravity = getAppropriateLayoutGravity() gravity = getAppropriateLayoutGravity()
) )

@ -9,7 +9,6 @@ import androidx.annotation.VisibleForTesting
import androidx.navigation.NavDestination import androidx.navigation.NavDestination
import androidx.navigation.NavDirections import androidx.navigation.NavDirections
import kotlinx.android.synthetic.main.activity_home.* import kotlinx.android.synthetic.main.activity_home.*
import mozilla.components.browser.session.runWithSession
import mozilla.components.browser.state.selector.findCustomTab import mozilla.components.browser.state.selector.findCustomTab
import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.SessionState
import mozilla.components.concept.engine.manifest.WebAppManifestParser import mozilla.components.concept.engine.manifest.WebAppManifestParser
@ -102,12 +101,9 @@ open class ExternalAppBrowserActivity : HomeActivity() {
// When this activity finishes, the process is staying around and the session still // When this activity finishes, the process is staying around and the session still
// exists then remove it now to free all its resources. Once this activity is finished // exists then remove it now to free all its resources. Once this activity is finished
// then there's no way to get back to it other than relaunching it. // then there's no way to get back to it other than relaunching it.
components.core.sessionManager.runWithSession(getExternalTabId()) { session -> val tabId = getExternalTabId()
// If the custom tag config has been removed we are opening this in normal browsing if (tabId != null) {
if (session.customTabConfig != null) { components.useCases.customTabsUseCases.remove(tabId)
remove(session)
}
true
} }
} }
} }

@ -14,7 +14,6 @@ import androidx.navigation.fragment.navArgs
import kotlinx.android.synthetic.main.component_browser_top_toolbar.* import kotlinx.android.synthetic.main.component_browser_top_toolbar.*
import kotlinx.android.synthetic.main.fragment_browser.* import kotlinx.android.synthetic.main.fragment_browser.*
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import mozilla.components.browser.session.Session
import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.SessionState
import mozilla.components.concept.engine.manifest.WebAppManifestParser import mozilla.components.concept.engine.manifest.WebAppManifestParser
import mozilla.components.concept.engine.manifest.getOrNull import mozilla.components.concept.engine.manifest.getOrNull
@ -52,16 +51,14 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
private val hideToolbarFeature = ViewBoundFeatureWrapper<WebAppHideToolbarFeature>() private val hideToolbarFeature = ViewBoundFeatureWrapper<WebAppHideToolbarFeature>()
@Suppress("LongMethod", "ComplexMethod") @Suppress("LongMethod", "ComplexMethod")
override fun initializeUI(view: View): Session? { override fun initializeUI(view: View, tab: SessionState) {
return super.initializeUI(view)?.also { super.initializeUI(view, tab)
val customTabSessionId = customTabSessionId ?: return
val activity = requireActivity() val activity = requireActivity()
val components = activity.components val components = activity.components
val manifest = args.webAppManifest?.let { json -> WebAppManifestParser().parse(json).getOrNull() }
val manifest = args.webAppManifest?.let { json ->
WebAppManifestParser().parse(json).getOrNull()
}
customTabSessionId?.let { customTabSessionId ->
customTabsIntegration.set( customTabsIntegration.set(
feature = CustomTabsIntegration( feature = CustomTabsIntegration(
sessionManager = requireComponents.core.sessionManager, sessionManager = requireComponents.core.sessionManager,
@ -71,7 +68,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
sessionId = customTabSessionId, sessionId = customTabSessionId,
activity = activity, activity = activity,
onItemTapped = { browserInteractor.onBrowserToolbarMenuItemTapped(it) }, onItemTapped = { browserInteractor.onBrowserToolbarMenuItemTapped(it) },
isPrivate = it.private, isPrivate = tab.content.private,
shouldReverseItems = !activity.settings().shouldUseBottomToolbar shouldReverseItems = !activity.settings().shouldUseBottomToolbar
), ),
owner = this, owner = this,
@ -159,8 +156,6 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
) )
} }
} }
}
}
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
@ -197,14 +192,14 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
nav(R.id.externalAppBrowserFragment, directions) nav(R.id.externalAppBrowserFragment, directions)
} }
override fun navToTrackingProtectionPanel(session: Session) { override fun navToTrackingProtectionPanel(tab: SessionState) {
requireComponents.useCases.trackingProtectionUseCases.containsException(session.id) { contains -> requireComponents.useCases.trackingProtectionUseCases.containsException(tab.id) { contains ->
val isEnabled = session.trackerBlockingEnabled && !contains val isEnabled = tab.trackingProtection.enabled && !contains
val directions = val directions =
ExternalAppBrowserFragmentDirections ExternalAppBrowserFragmentDirections
.actionGlobalTrackingProtectionPanelDialogFragment( .actionGlobalTrackingProtectionPanelDialogFragment(
sessionId = session.id, sessionId = tab.id,
url = session.url, url = tab.content.url,
trackingProtectionEnabled = isEnabled, trackingProtectionEnabled = isEnabled,
gravity = getAppropriateLayoutGravity() gravity = getAppropriateLayoutGravity()
) )

@ -87,13 +87,13 @@ class BrowserFragmentTest {
every { browserFragment.onboarding } returns onboarding every { browserFragment.onboarding } returns onboarding
every { browserFragment.requireContext() } returns context every { browserFragment.requireContext() } returns context
every { browserFragment.initializeUI(any()) } returns mockk() every { browserFragment.initializeUI(any(), any()) } returns mockk()
every { browserFragment.fullScreenChanged(any()) } returns Unit every { browserFragment.fullScreenChanged(any()) } returns Unit
every { browserFragment.resumeDownloadDialogState(any(), any(), any(), any(), any()) } returns Unit every { browserFragment.resumeDownloadDialogState(any(), any(), any(), any(), any()) } returns Unit
testTab = createTab(url = "https://mozilla.org")
store = BrowserStore() store = BrowserStore()
every { context.components.core.store } returns store every { context.components.core.store } returns store
testTab = createTab(url = "https://mozilla.org")
} }
@After @After
@ -122,10 +122,10 @@ class BrowserFragmentTest {
@Test @Test
fun `GIVEN browser UI is not initialized WHEN selected tab changes THEN browser UI is initialized`() { fun `GIVEN browser UI is not initialized WHEN selected tab changes THEN browser UI is initialized`() {
browserFragment.observeTabSelection(store) browserFragment.observeTabSelection(store)
verify(exactly = 0) { browserFragment.initializeUI(view) } verify(exactly = 0) { browserFragment.initializeUI(view, testTab) }
addAndSelectTab(testTab) addAndSelectTab(testTab)
verify(exactly = 1) { browserFragment.initializeUI(view) } verify(exactly = 1) { browserFragment.initializeUI(view, testTab) }
} }
@Test @Test

Loading…
Cancel
Save