Split out CustomTabToolbarIntegration (#7107)

2019-12-17 12:26:46 -08:00 · 2019-12-17 12:26:46 -08:00 · ea486d7c66
parent b4e10965b7
commit ea486d7c66
12 changed files with 194 additions and 142 deletions
--- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt
@ -26,6 +26,7 @@ import mozilla.components.support.ktx.android.util.dpToFloat
 import org.jetbrains.anko.dimen
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FenixSnackbar
+import org.mozilla.fenix.customtabs.CustomTabToolbarIntegration
 import org.mozilla.fenix.customtabs.CustomTabToolbarMenu
 import org.mozilla.fenix.ext.bookmarkStorage
 import org.mozilla.fenix.ext.components
@ -200,17 +201,27 @@ class BrowserToolbarView(
                )
            }

-            toolbarIntegration = ToolbarIntegration(
-                this,
-                view,
-                menuToolbar,
-                ShippedDomainsProvider().also { it.initialize(this) },
-                components.core.historyStorage,
-                components.core.sessionManager,
-                customTabSession?.id,
-                customTabSession?.private ?: sessionManager.selectedSession?.private ?: false,
-                interactor
-            )
+            toolbarIntegration = if (customTabSession != null) {
+                CustomTabToolbarIntegration(
+                    this,
+                    view,
+                    menuToolbar,
+                    customTabSession.id,
+                    isPrivate = customTabSession.private
+                )
+            } else {
+                DefaultToolbarIntegration(
+                    this,
+                    view,
+                    menuToolbar,
+                    ShippedDomainsProvider().also { it.initialize(this) },
+                    components.core.historyStorage,
+                    components.core.sessionManager,
+                    sessionId = null,
+                    isPrivate = sessionManager.selectedSession?.private ?: false,
+                    interactor = interactor
+                )
+            }
        }
    }

--- a/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt
@ -20,8 +20,8 @@ import java.lang.ref.WeakReference
 */
 class TabCounterToolbarButton(
    private val sessionManager: SessionManager,
-    private val showTabs: () -> Unit,
-    private val isPrivate: Boolean
+    private val isPrivate: Boolean,
+    private val showTabs: () -> Unit
 ) : Toolbar.Action {
    private var reference: WeakReference<TabCounter> = WeakReference<TabCounter>(null)

--- a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarIntegration.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarIntegration.kt
@ -10,7 +10,6 @@ import com.airbnb.lottie.LottieCompositionFactory
 import com.airbnb.lottie.LottieDrawable
 import mozilla.components.browser.domains.autocomplete.DomainAutocompleteProvider
 import mozilla.components.browser.session.SessionManager
-import mozilla.components.browser.session.runWithSession
 import mozilla.components.browser.toolbar.BrowserToolbar
 import mozilla.components.browser.toolbar.display.DisplayToolbar
 import mozilla.components.concept.storage.HistoryStorage
@ -25,109 +24,34 @@ import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.theme.ThemeManager

-class ToolbarIntegration(
+abstract class ToolbarIntegration(
    context: Context,
    toolbar: BrowserToolbar,
    toolbarMenu: ToolbarMenu,
-    domainAutocompleteProvider: DomainAutocompleteProvider,
-    historyStorage: HistoryStorage,
-    sessionManager: SessionManager,
-    sessionId: String? = null,
+    sessionId: String?,
    isPrivate: Boolean,
-    interactor: BrowserToolbarViewInteractor
+    renderStyle: ToolbarFeature.RenderStyle
 ) : LifecycleAwareFeature {

-    private var renderStyle: ToolbarFeature.RenderStyle = ToolbarFeature.RenderStyle.UncoloredUrl
-
-    init {
-        toolbar.display.menuBuilder = toolbarMenu.menuBuilder
-        toolbar.private = isPrivate
-
-        run {
-            sessionManager.runWithSession(sessionId) {
-                it.isCustomTabSession()
-            }.also { isCustomTab ->
-                if (isCustomTab) {
-                    renderStyle = ToolbarFeature.RenderStyle.RegistrableDomain
-                    return@run
-                }
-
-                val task = LottieCompositionFactory
-                    .fromRawRes(
-                        context,
-                        ThemeManager.resolveAttribute(R.attr.shieldLottieFile, context)
-                    )
-                task.addListener { result ->
-                    val lottieDrawable = LottieDrawable()
-                    lottieDrawable.composition = result
-
-                    toolbar.display.indicators =
-                        if (context.settings().shouldUseTrackingProtection) {
-                            listOf(
-                                DisplayToolbar.Indicators.TRACKING_PROTECTION,
-                                DisplayToolbar.Indicators.SECURITY,
-                                DisplayToolbar.Indicators.EMPTY
-                            )
-                        } else {
-                            listOf(
-                                DisplayToolbar.Indicators.SECURITY,
-                                DisplayToolbar.Indicators.EMPTY
-                            )
-                        }
-
-                    toolbar.display.displayIndicatorSeparator =
-                        context.settings().shouldUseTrackingProtection
-
-                    toolbar.display.icons = toolbar.display.icons.copy(
-                        emptyIcon = AppCompatResources.getDrawable(
-                            context,
-                            R.drawable.ic_bookmark_filled
-                        )!!,
-                        trackingProtectionTrackersBlocked = lottieDrawable,
-                        trackingProtectionNothingBlocked = AppCompatResources.getDrawable(
-                            context,
-                            R.drawable.ic_tracking_protection_enabled
-                        )!!,
-                        trackingProtectionException = AppCompatResources.getDrawable(
-                            context,
-                            R.drawable.ic_tracking_protection_disabled
-                        )!!
-                    )
-                }
-
-                val tabsAction = TabCounterToolbarButton(
-                    sessionManager,
-                    {
-                        toolbar.hideKeyboard()
-                        interactor.onTabCounterClicked()
-                    },
-                    isPrivate
-                )
-                toolbar.addBrowserAction(tabsAction)
-            }
-        }
-
-        ToolbarAutocompleteFeature(toolbar).apply {
-            addDomainProvider(domainAutocompleteProvider)
-            if (context.settings().shouldShowHistorySuggestions) {
-                addHistoryStorageProvider(historyStorage)
-            }
-        }
-    }
-
    private val toolbarPresenter: ToolbarPresenter = ToolbarPresenter(
        toolbar,
        context.components.core.store,
        sessionId,
        ToolbarFeature.UrlRenderConfiguration(
            PublicSuffixList(context),
-            ThemeManager.resolveAttribute(R.attr.primaryText, context), renderStyle = renderStyle
+            ThemeManager.resolveAttribute(R.attr.primaryText, context),
+            renderStyle = renderStyle
        )
    )

    private var menuPresenter =
        MenuPresenter(toolbar, context.components.core.sessionManager, sessionId)

+    init {
+        toolbar.display.menuBuilder = toolbarMenu.menuBuilder
+        toolbar.private = isPrivate
+    }
+
    override fun start() {
        menuPresenter.start()
        toolbarPresenter.start()
@ -138,3 +62,84 @@ class ToolbarIntegration(
        toolbarPresenter.stop()
    }
 }
+
+class DefaultToolbarIntegration(
+    context: Context,
+    toolbar: BrowserToolbar,
+    toolbarMenu: ToolbarMenu,
+    domainAutocompleteProvider: DomainAutocompleteProvider,
+    historyStorage: HistoryStorage,
+    sessionManager: SessionManager,
+    sessionId: String? = null,
+    isPrivate: Boolean,
+    interactor: BrowserToolbarViewInteractor
+) : ToolbarIntegration(
+    context = context,
+    toolbar = toolbar,
+    toolbarMenu = toolbarMenu,
+    sessionId = sessionId,
+    isPrivate = isPrivate,
+    renderStyle = ToolbarFeature.RenderStyle.UncoloredUrl
+) {
+
+    init {
+        toolbar.display.menuBuilder = toolbarMenu.menuBuilder
+        toolbar.private = isPrivate
+
+        val task = LottieCompositionFactory
+            .fromRawRes(
+                context,
+                ThemeManager.resolveAttribute(R.attr.shieldLottieFile, context)
+            )
+        task.addListener { result ->
+            val lottieDrawable = LottieDrawable()
+            lottieDrawable.composition = result
+
+            toolbar.display.indicators =
+                if (context.settings().shouldUseTrackingProtection) {
+                    listOf(
+                        DisplayToolbar.Indicators.TRACKING_PROTECTION,
+                        DisplayToolbar.Indicators.SECURITY,
+                        DisplayToolbar.Indicators.EMPTY
+                    )
+                } else {
+                    listOf(
+                        DisplayToolbar.Indicators.SECURITY,
+                        DisplayToolbar.Indicators.EMPTY
+                    )
+                }
+
+            toolbar.display.displayIndicatorSeparator =
+                context.settings().shouldUseTrackingProtection
+
+            toolbar.display.icons = toolbar.display.icons.copy(
+                emptyIcon = AppCompatResources.getDrawable(
+                    context,
+                    R.drawable.ic_bookmark_filled
+                )!!,
+                trackingProtectionTrackersBlocked = lottieDrawable,
+                trackingProtectionNothingBlocked = AppCompatResources.getDrawable(
+                    context,
+                    R.drawable.ic_tracking_protection_enabled
+                )!!,
+                trackingProtectionException = AppCompatResources.getDrawable(
+                    context,
+                    R.drawable.ic_tracking_protection_disabled
+                )!!
+            )
+        }
+
+        val tabsAction = TabCounterToolbarButton(sessionManager, isPrivate) {
+            toolbar.hideKeyboard()
+            interactor.onTabCounterClicked()
+        }
+        toolbar.addBrowserAction(tabsAction)
+
+        ToolbarAutocompleteFeature(toolbar).apply {
+            addDomainProvider(domainAutocompleteProvider)
+            if (context.settings().shouldShowHistorySuggestions) {
+                addHistoryStorageProvider(historyStorage)
+            }
+        }
+    }
+}
--- a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarIntegration.kt
+++ b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarIntegration.kt
@ -0,0 +1,26 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.customtabs
+
+import android.content.Context
+import mozilla.components.browser.toolbar.BrowserToolbar
+import mozilla.components.feature.toolbar.ToolbarFeature
+import org.mozilla.fenix.components.toolbar.ToolbarIntegration
+import org.mozilla.fenix.components.toolbar.ToolbarMenu
+
+class CustomTabToolbarIntegration(
+    context: Context,
+    toolbar: BrowserToolbar,
+    toolbarMenu: ToolbarMenu,
+    sessionId: String,
+    isPrivate: Boolean
+) : ToolbarIntegration(
+    context = context,
+    toolbar = toolbar,
+    toolbarMenu = toolbarMenu,
+    sessionId = sessionId,
+    isPrivate = isPrivate,
+    renderStyle = ToolbarFeature.RenderStyle.RegistrableDomain
+)
--- a/app/src/main/java/org/mozilla/fenix/ext/NavController.kt
+++ b/app/src/main/java/org/mozilla/fenix/ext/NavController.kt
@ -13,9 +13,9 @@ import androidx.navigation.Navigator
 import io.sentry.Sentry
 import org.mozilla.fenix.BuildConfig

-fun NavController.nav(@IdRes id: Int?, directions: NavDirections) {
+fun NavController.nav(@IdRes id: Int?, directions: NavDirections, navOptions: NavOptions? = null) {
    if (id == null || this.currentDestination?.id == id) {
-        this.navigate(directions)
+        this.navigate(directions, navOptions)
    } else {
        recordIdException(this.currentDestination?.id, id)
    }
@ -29,13 +29,12 @@ fun NavController.nav(@IdRes id: Int?, directions: NavDirections, extras: Naviga
    }
 }

-fun NavController.nav(@IdRes id: Int?, directions: NavDirections, options: NavOptions) {
-    if (id == null || this.currentDestination?.id == id) {
-        this.navigate(directions, options)
-    } else {
-        recordIdException(this.currentDestination?.id, id)
-    }
-}
+fun NavController.nav(
+    @IdRes id: Int?,
+    directions: NavDirections,
+    navOptions: NavOptions? = null,
+    extras: Navigator.Extras? = null
+) = nav(id, directions.actionId, directions.arguments, navOptions, extras)

 fun NavController.nav(
    @IdRes id: Int?,
--- a/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt
@ -65,7 +65,12 @@ class CrashReporterControllerTest {
        verify { components.analytics.metrics.track(Event.CrashReporterClosed(false)) }
        verify { components.useCases.tabsUseCases.removeTab(session) }
        verify { components.useCases.sessionUseCases.crashRecovery.invoke() }
-        verify { navContoller.navigate(CrashReporterFragmentDirections.actionCrashReporterFragmentToHomeFragment()) }
+        verify {
+            navContoller.navigate(
+                CrashReporterFragmentDirections.actionCrashReporterFragmentToHomeFragment(),
+                null
+            )
+        }
    }

    @Test
--- a/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt
@ -1,28 +1,27 @@
 package org.mozilla.fenix.ext

-import mozilla.components.support.test.robolectric.testContext
-import org.mozilla.fenix.TestApplication
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
-import org.robolectric.annotation.Config
-import io.mockk.mockk
-import io.mockk.spyk
-import io.mockk.mockkStatic
-import io.mockk.verify
-import io.mockk.every
-import io.mockk.Runs
-import io.mockk.just
-import io.mockk.confirmVerified
 import androidx.fragment.app.Fragment
+import androidx.navigation.NavController
+import androidx.navigation.NavDestination
 import androidx.navigation.NavDirections
 import androidx.navigation.NavOptions
-import androidx.navigation.fragment.NavHostFragment.findNavController
-import androidx.navigation.fragment.NavHostFragment
-import androidx.navigation.NavDestination
-import androidx.navigation.NavController
 import androidx.navigation.Navigator.Extras
+import androidx.navigation.fragment.NavHostFragment
+import io.mockk.Runs
+import io.mockk.confirmVerified
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.mockkStatic
+import io.mockk.spyk
+import io.mockk.verify
+import mozilla.components.support.test.robolectric.testContext
 import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.TestApplication
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config

@RunWith(RobolectricTestRunner::class)
@Config(application = TestApplication::class)
@ -49,11 +48,11 @@ class FragmentTest {

    @Test
    fun `Test nav fun with ID and directions`() {
-        every { (NavHostFragment.findNavController(mockFragment).navigate(navDirections)) } just Runs
+        every { (NavHostFragment.findNavController(mockFragment).navigate(navDirections, null)) } just Runs

        mockFragment.nav(mockId, navDirections)
        verify { (NavHostFragment.findNavController(mockFragment).getCurrentDestination()) }
-        verify { (NavHostFragment.findNavController(mockFragment).navigate(navDirections)) }
+        verify { (NavHostFragment.findNavController(mockFragment).navigate(navDirections, null)) }
        confirmVerified(mockFragment)
    }

--- a/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt
@ -38,7 +38,7 @@ class NavControllerTest {
    fun `Nav with id and directions args`() {
        navController.nav(4, navDirections)
        verify { (navController.currentDestination) }
-        verify { (navController.navigate(navDirections)) }
+        verify { (navController.navigate(navDirections, null)) }
    }

    @Test
--- a/app/src/test/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessorTest.kt
@ -63,9 +63,8 @@ class StartSearchIntentProcessorTest {
        verify { metrics.track(Event.SearchWidgetNewTabPressed) }
        verify {
            navController.navigate(
-                NavGraphDirections.actionGlobalSearch(
-                    sessionId = null
-                )
+                NavGraphDirections.actionGlobalSearch(sessionId = null),
+                null
            )
        }
        verify { out.removeExtra(HomeActivity.OPEN_TO_SEARCH) }
--- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt
@ -112,7 +112,10 @@ class BookmarkControllerTest {

        verify {
            invokePendingDeletion.invoke()
-            navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid))
+            navController.navigate(
+                BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid),
+                null
+            )
        }
    }

@ -134,7 +137,8 @@ class BookmarkControllerTest {
            navController.navigate(
                BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(
                    item.guid
-                )
+                ),
+                null
            )
        }
    }
@ -168,12 +172,12 @@ class BookmarkControllerTest {
    @Test
    fun `handleBookmarkSharing should navigate to the 'Share' fragment`() {
        val navDirectionsSlot = slot<NavDirections>()
-        every { navController.navigate(capture(navDirectionsSlot)) } just Runs
+        every { navController.navigate(capture(navDirectionsSlot), null) } just Runs

        controller.handleBookmarkSharing(item)

        verify {
-            navController.navigate(navDirectionsSlot.captured)
+            navController.navigate(navDirectionsSlot.captured, null)
        }
    }

--- a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt
@ -13,8 +13,8 @@ import io.mockk.Runs
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
-import io.mockk.verify
 import io.mockk.mockkObject
+import io.mockk.verify
 import mozilla.components.browser.search.SearchEngine
 import mozilla.components.browser.search.SearchEngineManager
 import mozilla.components.browser.session.Session
@ -23,9 +23,9 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.FenixApplication
-import org.mozilla.fenix.TestApplication
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
+import org.mozilla.fenix.TestApplication
 import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.searchEngineManager
 import org.mozilla.fenix.ext.settings
@ -264,8 +264,9 @@ class SearchInteractorTest {
        verify {
            navController.navigate(
                SearchFragmentDirections.actionSearchFragmentToBrowserFragment(
-                    null
-                )
+                    activeSessionId = null
+                ),
+                null
            )
        }
    }
--- a/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt
@ -62,7 +62,10 @@ class AccountSettingsInteractorTest {
        interactor.onSignOut()

        verify {
-            navController.navigate(AccountSettingsFragmentDirections.actionAccountSettingsFragmentToSignOutFragment())
+            navController.navigate(
+                AccountSettingsFragmentDirections.actionAccountSettingsFragmentToSignOutFragment(),
+                null
+            )
        }
    }
 }