Closes #16949: Refactor OpenInAppOnboardingObserver to use browser store

upstream-sync
Christian Sadilek 4 years ago
parent 0a0dbeb132
commit 0b99669753

@ -51,9 +51,9 @@ import org.mozilla.fenix.trackingprotection.TrackingProtectionOverlay
class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
private val windowFeature = ViewBoundFeatureWrapper<WindowFeature>()
private val openInAppOnboardingObserver = ViewBoundFeatureWrapper<OpenInAppOnboardingObserver>()
private var readerModeAvailable = false
private var openInAppOnboardingObserver: OpenInAppOnboardingObserver? = null
private var pwaOnboardingObserver: PwaOnboardingObserver? = null
override fun onCreateView(
@ -142,6 +142,22 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
owner = this,
view = view
)
if (context.settings().shouldShowOpenInAppCfr) {
openInAppOnboardingObserver.set(
feature = OpenInAppOnboardingObserver(
context = context,
store = context.components.core.store,
lifecycleOwner = this,
navController = findNavController(),
settings = context.settings(),
appLinksUseCases = context.components.useCases.appLinksUseCases,
container = browserLayout as ViewGroup
),
owner = this,
view = view
)
}
}
}
@ -163,23 +179,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
// TODO Use browser store instead of session observer: https://github.com/mozilla-mobile/fenix/issues/16945
session?.register(toolbarSessionObserver, viewLifecycleOwner, autoPause = true)
if (settings.shouldShowOpenInAppCfr && session != null) {
openInAppOnboardingObserver = OpenInAppOnboardingObserver(
context = context,
navController = findNavController(),
settings = settings,
appLinksUseCases = context.components.useCases.appLinksUseCases,
container = browserLayout as ViewGroup
)
@Suppress("DEPRECATION")
// TODO Use browser store instead of session observer: https://github.com/mozilla-mobile/fenix/issues/16949
session.register(
openInAppOnboardingObserver!!,
owner = this,
autoPause = true
)
}
if (!settings.userKnowsAboutPwas) {
pwaOnboardingObserver = PwaOnboardingObserver(
store = context.components.core.store,
@ -197,14 +196,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
override fun onStop() {
super.onStop()
// This observer initialized in onStart has a reference to fragment's view.
// Prevent it leaking the view after the latter onDestroyView.
if (openInAppOnboardingObserver != null) {
@Suppress("DEPRECATION")
// TODO Use browser store instead of session observer: https://github.com/mozilla-mobile/fenix/issues/16949
getSessionById()?.unregister(openInAppOnboardingObserver!!)
openInAppOnboardingObserver = null
}
pwaOnboardingObserver?.stop()
}

@ -7,10 +7,20 @@ package org.mozilla.fenix.browser
import android.content.Context
import android.view.ViewGroup
import androidx.annotation.VisibleForTesting
import androidx.lifecycle.LifecycleOwner
import androidx.navigation.NavController
import mozilla.components.browser.session.Session
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.mapNotNull
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.app.links.AppLinksUseCases
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.utils.Settings
@ -18,51 +28,79 @@ import org.mozilla.fenix.utils.Settings
/**
* Displays an [InfoBanner] when a user visits a website that can be opened in an installed native app.
*/
@ExperimentalCoroutinesApi
@Suppress("LongParameterList")
class OpenInAppOnboardingObserver(
private val context: Context,
private val store: BrowserStore,
private val lifecycleOwner: LifecycleOwner,
private val navController: NavController,
private val settings: Settings,
private val appLinksUseCases: AppLinksUseCases,
private val container: ViewGroup
) : Session.Observer {
) : LifecycleAwareFeature {
private var scope: CoroutineScope? = null
private var currentUrl: String? = null
private var sessionDomainForDisplayedBanner: String? = null
@VisibleForTesting
internal var sessionDomainForDisplayedBanner: String? = null
@VisibleForTesting
internal var infoBanner: InfoBanner? = null
override fun onUrlChanged(session: Session, url: String) {
sessionDomainForDisplayedBanner?.let {
if (url.tryGetHostFromUrl() != it) {
infoBanner?.dismiss()
override fun start() {
scope = store.flowScoped(lifecycleOwner) { flow ->
flow.mapNotNull { state ->
state.selectedTab
}
.ifAnyChanged {
tab -> arrayOf(tab.content.url, tab.content.loading)
}
.collect { tab ->
if (tab.content.url != currentUrl) {
sessionDomainForDisplayedBanner?.let {
if (tab.content.url.tryGetHostFromUrl() != it) {
infoBanner?.dismiss()
}
}
currentUrl = tab.content.url
} else {
// Loading state has changed
maybeShowOpenInAppBanner(tab.content.url, tab.content.loading)
}
}
}
}
override fun onLoadingStateChanged(session: Session, loading: Boolean) {
override fun stop() {
scope?.cancel()
}
private fun maybeShowOpenInAppBanner(url: String, loading: Boolean) {
if (loading || settings.openLinksInExternalApp || !settings.shouldShowOpenInAppCfr) {
return
}
val appLink = appLinksUseCases.appLinkRedirect
if (appLink(session.url).hasExternalApp()) {
infoBanner = InfoBanner(
context = context,
message = context.getString(R.string.open_in_app_cfr_info_message),
dismissText = context.getString(R.string.open_in_app_cfr_negative_button_text),
actionText = context.getString(R.string.open_in_app_cfr_positive_button_text),
container = container
) {
val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment(
preferenceToScrollTo = context.getString(R.string.pref_key_open_links_in_external_app)
)
navController.nav(R.id.browserFragment, directions)
}
if (appLink(url).hasExternalApp()) {
infoBanner = createInfoBanner()
infoBanner?.showBanner()
sessionDomainForDisplayedBanner = session.url.tryGetHostFromUrl()
sessionDomainForDisplayedBanner = url.tryGetHostFromUrl()
settings.shouldShowOpenInAppBanner = false
}
}
@VisibleForTesting
internal fun createInfoBanner(): InfoBanner {
return InfoBanner(
context = context,
message = context.getString(R.string.open_in_app_cfr_info_message),
dismissText = context.getString(R.string.open_in_app_cfr_negative_button_text),
actionText = context.getString(R.string.open_in_app_cfr_positive_button_text),
container = container
) {
val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment(
preferenceToScrollTo = context.getString(R.string.pref_key_open_links_in_external_app)
)
navController.nav(R.id.browserFragment, directions)
}
}
}

@ -5,16 +5,26 @@
package org.mozilla.fenix.browser
import android.content.Context
import io.mockk.MockKAnnotations
import android.view.ViewGroup
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.LifecycleRegistry
import androidx.navigation.NavController
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import mozilla.components.browser.session.Session
import mozilla.components.feature.app.links.AppLinkRedirect
import kotlinx.coroutines.test.TestCoroutineDispatcher
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.app.links.AppLinksUseCases
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.rule.MainCoroutineRule
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -24,90 +34,124 @@ import org.mozilla.fenix.utils.Settings
@RunWith(FenixRobolectricTestRunner::class)
class OpenInAppOnboardingObserverTest {
@MockK(relaxed = true) private lateinit var context: Context
@MockK(relaxed = true) private lateinit var settings: Settings
@MockK(relaxed = true) private lateinit var session: Session
@MockK(relaxed = true) private lateinit var appLinksUseCases: AppLinksUseCases
@MockK(relaxed = true) private lateinit var applinksRedirect: AppLinkRedirect
@MockK(relaxed = true) private lateinit var getAppLinkRedirect: AppLinksUseCases.GetAppLinkRedirect
@MockK(relaxed = true) private lateinit var infoBanner: InfoBanner
private lateinit var store: BrowserStore
private lateinit var lifecycleOwner: MockedLifecycleOwner
private lateinit var openInAppOnboardingObserver: OpenInAppOnboardingObserver
private lateinit var navigationController: NavController
private lateinit var settings: Settings
private lateinit var appLinksUseCases: AppLinksUseCases
private lateinit var context: Context
private lateinit var container: ViewGroup
private lateinit var infoBanner: InfoBanner
private val testDispatcher = TestCoroutineDispatcher()
@get:Rule
val coroutinesTestRule = MainCoroutineRule(testDispatcher)
@Before
fun setup() {
MockKAnnotations.init(this)
fun setUp() {
store = BrowserStore(
BrowserState(
tabs = listOf(
createTab(url = "https://www.mozilla.org", id = "1")
), selectedTabId = "1"
)
)
lifecycleOwner = MockedLifecycleOwner(Lifecycle.State.STARTED)
navigationController = mockk(relaxed = true)
settings = mockk(relaxed = true)
appLinksUseCases = mockk(relaxed = true)
container = mockk(relaxed = true)
context = mockk(relaxed = true)
infoBanner = mockk(relaxed = true)
openInAppOnboardingObserver = spyk(OpenInAppOnboardingObserver(
context = context,
store = store,
lifecycleOwner = lifecycleOwner,
navController = navigationController,
settings = settings,
appLinksUseCases = appLinksUseCases,
container = container
))
every { openInAppOnboardingObserver.createInfoBanner() } returns infoBanner
}
@Test
fun `do not show banner when openLinksInExternalApp is set to true`() {
fun `GIVEN user configured to open links in external app WHEN page finishes loading THEN do not show banner`() {
every { settings.openLinksInExternalApp } returns true
every { settings.shouldShowOpenInAppCfr } returns true
every { appLinksUseCases.appLinkRedirect.invoke(any()).hasExternalApp() } returns true
store.dispatch(ContentAction.UpdateLoadingStateAction("1", true)).joinBlocking()
val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
observer.onLoadingStateChanged(session, false)
verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
openInAppOnboardingObserver.start()
store.dispatch(ContentAction.UpdateLoadingStateAction("1", false)).joinBlocking()
verify(exactly = 0) { infoBanner.showBanner() }
}
@Test
fun `GIVEN user has not configured to open links in external app WHEN page finishes loading THEN show banner`() {
every { settings.openLinksInExternalApp } returns false
observer.onLoadingStateChanged(session, false)
every { settings.shouldShowOpenInAppCfr } returns true
every { appLinksUseCases.appLinkRedirect.invoke(any()).hasExternalApp() } returns true
store.dispatch(ContentAction.UpdateLoadingStateAction("1", true)).joinBlocking()
verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
openInAppOnboardingObserver.start()
store.dispatch(ContentAction.UpdateLoadingStateAction("1", false)).joinBlocking()
verify(exactly = 1) { infoBanner.showBanner() }
}
@Test
fun `do not show banner when shouldShowOpenInAppCfr is set to false`() {
fun `GIVEN banner was already displayed WHEN page finishes loading THEN do not show banner`() {
every { settings.openLinksInExternalApp } returns false
every { settings.shouldShowOpenInAppCfr } returns false
every { appLinksUseCases.appLinkRedirect.invoke(any()).hasExternalApp() } returns true
store.dispatch(ContentAction.UpdateLoadingStateAction("1", true)).joinBlocking()
val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
observer.onLoadingStateChanged(session, false)
verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
every { settings.shouldShowOpenInAppCfr } returns true
observer.onLoadingStateChanged(session, false)
verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
openInAppOnboardingObserver.start()
store.dispatch(ContentAction.UpdateLoadingStateAction("1", false)).joinBlocking()
verify(exactly = 0) { infoBanner.showBanner() }
}
@Test
fun `do not show banner when URL is loading`() {
fun `GIVEN banner should be displayed WHEN no application found THEN do not show banner`() {
every { settings.openLinksInExternalApp } returns false
every { settings.shouldShowOpenInAppCfr } returns true
every { appLinksUseCases.appLinkRedirect.invoke(any()).hasExternalApp() } returns false
store.dispatch(ContentAction.UpdateLoadingStateAction("1", true)).joinBlocking()
val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
openInAppOnboardingObserver.start()
observer.onLoadingStateChanged(session, true)
verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
observer.onLoadingStateChanged(session, false)
verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
store.dispatch(ContentAction.UpdateLoadingStateAction("1", false)).joinBlocking()
verify(exactly = 0) { infoBanner.showBanner() }
}
@Test
fun `do not show banner when external app is not found`() {
fun `GIVEN banner is displayed WHEN user navigates to different domain THEN banner is dismissed`() {
every { settings.openLinksInExternalApp } returns false
every { settings.shouldShowOpenInAppCfr } returns true
every { appLinksUseCases.appLinkRedirect } returns getAppLinkRedirect
every { getAppLinkRedirect.invoke(any()) } returns applinksRedirect
every { appLinksUseCases.appLinkRedirect.invoke(any()).hasExternalApp() } returns true
store.dispatch(ContentAction.UpdateLoadingStateAction("1", true)).joinBlocking()
val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
observer.onLoadingStateChanged(session, false)
verify(exactly = 0) { settings.shouldShowOpenInAppBanner }
}
openInAppOnboardingObserver.start()
@Test
fun `do not dismiss banner when URL is the same`() {
val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
observer.infoBanner = infoBanner
observer.sessionDomainForDisplayedBanner = "mozilla.com"
observer.onUrlChanged(session, "https://mozilla.com")
store.dispatch(ContentAction.UpdateLoadingStateAction("1", false)).joinBlocking()
verify(exactly = 1) { infoBanner.showBanner() }
verify(exactly = 0) { infoBanner.dismiss() }
store.dispatch(ContentAction.UpdateUrlAction("1", "https://www.mozilla.org/en-US/")).joinBlocking()
verify(exactly = 0) { infoBanner.dismiss() }
observer.onUrlChanged(session, "https://abc.com")
store.dispatch(ContentAction.UpdateUrlAction("1", "https://www.firefox.com")).joinBlocking()
verify(exactly = 1) { infoBanner.dismiss() }
}
internal class MockedLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner {
val lifecycleRegistry = LifecycleRegistry(this).apply {
currentState = initialState
}
override fun getLifecycle(): Lifecycle = lifecycleRegistry
}
}

Loading…
Cancel
Save