From ea486d7c6671efce16d9557e8dd9b6c0ecca60c5 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Tue, 17 Dec 2019 12:26:46 -0800 Subject: [PATCH] Split out CustomTabToolbarIntegration (#7107) --- .../components/toolbar/BrowserToolbarView.kt | 33 ++-- .../toolbar/TabCounterToolbarButton.kt | 4 +- .../components/toolbar/ToolbarIntegration.kt | 177 +++++++++--------- .../customtabs/CustomTabToolbarIntegration.kt | 26 +++ .../org/mozilla/fenix/ext/NavController.kt | 17 +- .../crashes/CrashReporterControllerTest.kt | 7 +- .../org/mozilla/fenix/ext/FragmentTest.kt | 39 ++-- .../mozilla/fenix/ext/NavControllerTest.kt | 2 +- .../intent/StartSearchIntentProcessorTest.kt | 5 +- .../bookmarks/BookmarkControllerTest.kt | 12 +- .../fenix/search/SearchInteractorTest.kt | 9 +- .../account/AccountSettingsInteractorTest.kt | 5 +- 12 files changed, 194 insertions(+), 142 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarIntegration.kt diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index 36a2b72be..5e65d24d9 100644 --- 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 + ) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt index baccd3ee0..6cba9e26f 100644 --- 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 = WeakReference(null) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarIntegration.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarIntegration.kt index e21345446..de90e5b78 100644 --- 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) + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarIntegration.kt b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarIntegration.kt new file mode 100644 index 000000000..4d85fdcb6 --- /dev/null +++ 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 +) diff --git a/app/src/main/java/org/mozilla/fenix/ext/NavController.kt b/app/src/main/java/org/mozilla/fenix/ext/NavController.kt index 58a6232cd..efea3e982 100644 --- 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?, diff --git a/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt b/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt index e3dfaad30..2f0d18685 100644 --- 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 diff --git a/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt b/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt index 353063fe0..05b47d732 100644 --- 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) } diff --git a/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt b/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt index 99a6ea0f5..4e22e5fe8 100644 --- 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 diff --git a/app/src/test/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessorTest.kt b/app/src/test/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessorTest.kt index 4efd60f67..821397224 100644 --- 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) } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt index b0bb3c5df..2ea66fb21 100644 --- 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() - 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) } } diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt index 159dfd81d..f0738cf9d 100644 --- 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 ) } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt index f817975d9..fe637735f 100644 --- 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 + ) } } }