From ea62d47ae2a017a30a2e2b6afcdeea35e6b25381 Mon Sep 17 00:00:00 2001 From: Matthew Tighe Date: Tue, 28 Nov 2023 17:20:22 -0800 Subject: [PATCH] Bug 1861459 - Remove Home Screen usages of BrowsingModeManager --- .../perf/StartupExcessiveResourceUseTest.kt | 2 +- .../org/mozilla/fenix/BrowsingModeBinding.kt | 56 ++++++++++ .../BrowsingModePersistenceMiddleware.kt | 49 +++++++++ .../java/org/mozilla/fenix/HomeActivity.kt | 84 ++++++--------- .../browsingmode/BrowsingModeManager.kt | 20 ++++ .../org/mozilla/fenix/components/AppStore.kt | 6 +- .../mozilla/fenix/components/Components.kt | 4 + .../fenix/components/appstate/AppAction.kt | 23 ++++ .../components/appstate/AppStoreReducer.kt | 4 + .../org/mozilla/fenix/home/HomeFragment.kt | 33 +++--- .../fenix/home/PrivateBrowsingButtonView.kt | 14 +-- .../fenix/home/intent/IntentReducer.kt | 21 ++++ .../mozilla/fenix/BrowsingModeBindingTest.kt | 101 ++++++++++++++++++ .../BrowsingModePersistenceMiddlewareTest.kt | 65 +++++++++++ .../org/mozilla/fenix/HomeActivityTest.kt | 36 ++++--- .../ExternalAppBrowserActivityTest.kt | 3 - .../home/PrivateBrowsingButtonViewTest.kt | 21 ++-- 17 files changed, 426 insertions(+), 116 deletions(-) create mode 100644 fenix/app/src/main/java/org/mozilla/fenix/BrowsingModeBinding.kt create mode 100644 fenix/app/src/main/java/org/mozilla/fenix/BrowsingModePersistenceMiddleware.kt create mode 100644 fenix/app/src/main/java/org/mozilla/fenix/home/intent/IntentReducer.kt create mode 100644 fenix/app/src/test/java/org/mozilla/fenix/BrowsingModeBindingTest.kt create mode 100644 fenix/app/src/test/java/org/mozilla/fenix/BrowsingModePersistenceMiddlewareTest.kt diff --git a/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt index 1d17b8e040d5..5a386eefcc9f 100644 --- a/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt +++ b/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -33,7 +33,7 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule * * Say no to main thread IO! 🙅 */ -private const val EXPECTED_SUPPRESSION_COUNT = 16 +private const val EXPECTED_SUPPRESSION_COUNT = 15 /** * The number of times we call the `runBlocking` coroutine method on the main thread during this diff --git a/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModeBinding.kt b/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModeBinding.kt new file mode 100644 index 000000000000..3bcc2dc8fc54 --- /dev/null +++ b/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModeBinding.kt @@ -0,0 +1,56 @@ +/* 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 + +import android.view.Window +import android.view.WindowManager +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChangedBy +import kotlinx.coroutines.withContext +import mozilla.components.lib.state.helpers.AbstractBinding +import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppState +import org.mozilla.fenix.theme.ThemeManager +import org.mozilla.fenix.utils.Settings + +/** + * Binding to react to Private Browsing Mode changes in AppState. + * + * @param appStore AppStore to observe state changes from. + * @param themeManager Theme will be updated based on state changes. + * @param retrieveWindow Get window to update privacy flags for. + * @param settings Determine user settings for privacy features. + * @param ioDispatcher Dispatcher to launch disk reads. Exposed for test. + */ +class BrowsingModeBinding( + appStore: AppStore, + private val themeManager: ThemeManager, + private val retrieveWindow: () -> Window, + private val settings: Settings, + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, +) : AbstractBinding(appStore) { + override suspend fun onState(flow: Flow) { + flow.distinctUntilChangedBy { it.mode }.collect { + themeManager.currentTheme = it.mode + setWindowPrivacy(it.mode) + } + } + + private suspend fun setWindowPrivacy(mode: BrowsingMode) { + if (mode == BrowsingMode.Private) { + val allowScreenshots = withContext(ioDispatcher) { + settings.allowScreenshotsInPrivateMode + } + if (!allowScreenshots) { + retrieveWindow().addFlags(WindowManager.LayoutParams.FLAG_SECURE) + } + } else { + retrieveWindow().clearFlags(WindowManager.LayoutParams.FLAG_SECURE) + } + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModePersistenceMiddleware.kt b/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModePersistenceMiddleware.kt new file mode 100644 index 000000000000..67ec71e9da14 --- /dev/null +++ b/fenix/app/src/main/java/org/mozilla/fenix/BrowsingModePersistenceMiddleware.kt @@ -0,0 +1,49 @@ +/* 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 + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import mozilla.components.lib.state.Middleware +import mozilla.components.lib.state.MiddlewareContext +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.components.appstate.AppState +import org.mozilla.fenix.utils.Settings + +/** + * Middleware for controlling side-effects relating to Private Browsing Mode. + * + * @param settings Used to update disk-cache related PBM data. + * @param scope Scope used for disk writes and reads. Exposed for test overrides. + */ +class BrowsingModePersistenceMiddleware( + private val settings: Settings, + private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO), +) : Middleware { + override fun invoke( + context: MiddlewareContext, + next: (AppAction) -> Unit, + action: AppAction, + ) { + val initialMode = context.state.mode + next(action) + val updatedMode = context.state.mode + if (initialMode != context.state.mode) { + scope.launch { + settings.lastKnownMode = updatedMode + } + } + when (action) { + is AppAction.Init -> { + scope.launch { + val mode = settings.lastKnownMode + context.store.dispatch(AppAction.BrowsingModeLoaded(mode)) + } + } + else -> Unit + } + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 659c832c5c95..7f1804347108 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -23,7 +23,6 @@ import android.view.LayoutInflater import android.view.MotionEvent import android.view.View import android.view.ViewConfiguration -import android.view.WindowManager.LayoutParams.FLAG_SECURE import androidx.annotation.CallSuper import androidx.annotation.IdRes import androidx.annotation.RequiresApi @@ -86,9 +85,10 @@ import org.mozilla.fenix.GleanMetrics.SplashScreen import org.mozilla.fenix.GleanMetrics.StartOnHome import org.mozilla.fenix.addons.ExtensionsProcessDisabledBackgroundController import org.mozilla.fenix.addons.ExtensionsProcessDisabledForegroundController +import org.mozilla.fenix.bindings.BrowserStoreBinding +import org.mozilla.fenix.browser.browsingmode.AppStoreBrowsingModeManagerWrapper import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.BreadcrumbsRecorder import org.mozilla.fenix.components.metrics.GrowthDataWorker @@ -152,8 +152,12 @@ import java.util.Locale @SuppressWarnings("TooManyFunctions", "LargeClass", "LongMethod") open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { private lateinit var binding: ActivityHomeBinding - lateinit var themeManager: ThemeManager - lateinit var browsingModeManager: BrowsingModeManager + val browsingModeManager: BrowsingModeManager by lazy { + AppStoreBrowsingModeManagerWrapper(components.appStore) + } + val themeManager by lazy { + createThemeManager() + } private var isVisuallyComplete = false @@ -231,14 +235,13 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { maybeShowSplashScreen() - // There is disk read violations on some devices such as samsung and pixel for android 9/10 - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { - // Browsing mode & theme setup should always be called before super.onCreate. - setupBrowsingMode(getModeFromIntentOrLastKnown(intent)) - setupTheme() - - super.onCreate(savedInstanceState) + setModeFromIntent(intent) + // Theme setup should always be called before super.onCreate + if (this !is ExternalAppBrowserActivity) { + themeManager.setActivityTheme(this) + themeManager.applyStatusBarTheme(this) } + super.onCreate(savedInstanceState) // Checks if Activity is currently in PiP mode if launched from external intents, then exits it checkAndExitPiP() @@ -369,6 +372,13 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { extensionsProcessDisabledBackgroundController, serviceWorkerSupport, webExtensionPromptFeature, + BrowserStoreBinding(components.core.store, components.appStore), + BrowsingModeBinding( + components.appStore, + themeManager, + { window }, + components.settings, + ), ) if (shouldAddToRecentsScreen(intent)) { @@ -677,7 +687,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { ) + externalSourceIntentProcessors val intentHandled = intentProcessors.any { it.process(intent, navHost.navController, this.intent) } - browsingModeManager.mode = getModeFromIntentOrLastKnown(intent) + setModeFromIntent(intent) if (intentHandled) { supportFragmentManager @@ -848,15 +858,18 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { * private mode directly before the content view is created. Returns the mode set by the intent * otherwise falls back to the last known mode. */ - @VisibleForTesting - internal fun getModeFromIntentOrLastKnown(intent: Intent?): BrowsingMode { + internal fun setModeFromIntent(intent: Intent?) { intent?.toSafeIntent()?.let { if (it.hasExtra(PRIVATE_BROWSING_MODE)) { val startPrivateMode = it.getBooleanExtra(PRIVATE_BROWSING_MODE, false) - return BrowsingMode.fromBoolean(isPrivate = startPrivateMode) + // Since the mode is initially set from settings during AppAction.Init, this action + // will overwrite the state once the action is processed in the queue if called from + // onCreate. + if (startPrivateMode) { + components.appStore.dispatch(AppAction.IntentAction.EnterPrivateBrowsing) + } } } - return settings().lastKnownMode } /** @@ -872,20 +885,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { return false } - private fun setupBrowsingMode(mode: BrowsingMode) { - settings().lastKnownMode = mode - browsingModeManager = createBrowsingModeManager(mode) - } - - private fun setupTheme() { - themeManager = createThemeManager() - // ExternalAppBrowserActivity handles it's own theming as it can be customized. - if (this !is ExternalAppBrowserActivity) { - themeManager.setActivityTheme(this) - themeManager.applyStatusBarTheme(this) - } - } - // Stop active media when activity is destroyed. private fun stopMediaSession() { if (isFinishing) { @@ -1009,7 +1008,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { additionalHeaders: Map? = null, ) { val startTime = components.core.engine.profiler?.getProfilerTime() - val mode = browsingModeManager.mode + val mode = components.appStore.state.mode val private = when (mode) { BrowsingMode.Private -> true @@ -1090,7 +1089,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { // Normal tabs + cold start -> Should go back to browser if we had any tabs open when we left last // except for PBM + Cold Start there won't be any tabs since they're evicted so we never will navigate - if (settings().shouldReturnToBrowser && !browsingModeManager.mode.isPrivate) { + if (settings().shouldReturnToBrowser && !components.appStore.state.mode.isPrivate) { // Navigate to home first (without rendering it) to add it to the back stack. openToBrowser(BrowserDirection.FromGlobal, null) } @@ -1125,25 +1124,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { return super.getSystemService(name) } - private fun createBrowsingModeManager(initialMode: BrowsingMode): BrowsingModeManager { - return DefaultBrowsingModeManager(initialMode, components.settings) { newMode -> - updateSecureWindowFlags(newMode) - themeManager.currentTheme = newMode - }.also { - updateSecureWindowFlags(initialMode) - } - } - - private fun updateSecureWindowFlags(mode: BrowsingMode = browsingModeManager.mode) { - if (mode == BrowsingMode.Private && !settings().allowScreenshotsInPrivateMode) { - window.addFlags(FLAG_SECURE) - } else { - window.clearFlags(FLAG_SECURE) - } - } - - private fun createThemeManager(): ThemeManager { - return DefaultThemeManager(browsingModeManager.mode, this) + protected open fun createThemeManager(): ThemeManager { + return DefaultThemeManager(components.appStore.state.mode, this) } private fun openPopup(webExtensionState: WebExtensionState) { diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/browsingmode/BrowsingModeManager.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/browsingmode/BrowsingModeManager.kt index 1b31d596e04d..c9f59a5bc20d 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/browser/browsingmode/BrowsingModeManager.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/browser/browsingmode/BrowsingModeManager.kt @@ -4,6 +4,8 @@ package org.mozilla.fenix.browser.browsingmode +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.utils.Settings /** @@ -17,6 +19,11 @@ enum class BrowsingMode { */ val isPrivate get() = this == Private + val inverted get() = when (this) { + Private -> Normal + Normal -> Private + } + companion object { /** @@ -31,6 +38,19 @@ interface BrowsingModeManager { var mode: BrowsingMode } +/** + * Wraps an [appStore] and keeps its [AppState.mode] in sync with external changes. + */ +class AppStoreBrowsingModeManagerWrapper( + private val appStore: AppStore, +) : BrowsingModeManager { + override var mode: BrowsingMode + get() = appStore.state.mode + set(value) { + appStore.dispatch(AppAction.ModeChange(value)) + } +} + /** * Wraps a [BrowsingMode] and executes a callback whenever [mode] is updated. */ diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/AppStore.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/AppStore.kt index c5214dc4e496..93e94bdcced2 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/AppStore.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/AppStore.kt @@ -19,4 +19,8 @@ import org.mozilla.fenix.components.appstate.AppStoreReducer class AppStore( initialState: AppState = AppState(), middlewares: List> = emptyList(), -) : Store(initialState, AppStoreReducer::reduce, middlewares) +) : Store(initialState, AppStoreReducer::reduce, middlewares) { + init { + dispatch(AppAction.Init) + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt index 6a4ea2328c2e..08a3f74b4843 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -20,6 +20,7 @@ import mozilla.components.feature.downloads.manager.FetchDownloadManager import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.support.base.android.NotificationsDelegate import mozilla.components.support.base.worker.Frequency +import org.mozilla.fenix.BrowsingModePersistenceMiddleware import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags @@ -234,6 +235,9 @@ class Components(private val context: Context) { messagingStorage = analytics.messagingStorage, ), MetricsMiddleware(metrics = analytics.metrics), + BrowsingModePersistenceMiddleware( + settings = settings, + ), ), ) } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index 07eb907e7cb6..39e0819573d2 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -32,6 +32,19 @@ import org.mozilla.fenix.wallpapers.Wallpaper * [Action] implementation related to [AppStore]. */ sealed class AppAction : Action { + /** + * [AppAction] dispatched to indicate that the store is initialized and + * ready to use. This action is dispatched automatically before any other + * action is processed. Its main purpose is to trigger initialization logic + * in middlewares. The action itself should have no effect on the [AppState]. + */ + object Init : AppAction() + + /** + * The browsing [mode] has been loaded from a persistence layer. + */ + data class BrowsingModeLoaded(val mode: BrowsingMode) : AppAction() + data class UpdateInactiveExpanded(val expanded: Boolean) : AppAction() /** @@ -267,4 +280,14 @@ sealed class AppAction : Action { val key: ShoppingState.ProductRecommendationImpressionKey, ) : ShoppingAction() } + + /** + * Actions related to Intents. + */ + sealed class IntentAction : AppAction() { + /** + * Private browsing mode should be entered. + */ + object EnterPrivateBrowsing : IntentAction() + } } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index bf0ab3245ff5..c819619a4d1a 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -13,6 +13,7 @@ import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.shopping.ShoppingStateReducer import org.mozilla.fenix.ext.filterOutTab import org.mozilla.fenix.ext.getFilteredStories +import org.mozilla.fenix.home.intent.IntentReducer import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory import org.mozilla.fenix.home.recentsyncedtabs.RecentSyncedTabState import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem @@ -25,6 +26,8 @@ import org.mozilla.fenix.messaging.state.MessagingReducer internal object AppStoreReducer { @Suppress("LongMethod") fun reduce(state: AppState, action: AppAction): AppState = when (action) { + is AppAction.Init -> state + is AppAction.BrowsingModeLoaded -> state.copy(mode = action.mode) is AppAction.UpdateInactiveExpanded -> state.copy(inactiveTabsExpanded = action.expanded) is AppAction.UpdateFirstFrameDrawn -> { @@ -239,6 +242,7 @@ internal object AppStoreReducer { ) is AppAction.ShoppingAction -> ShoppingStateReducer.reduce(state, action) + is AppAction.IntentAction -> IntentReducer.reduce(state, action) } } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 588737e35a1b..f5fc726635e6 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -157,8 +157,6 @@ class HomeFragment : Fragment() { ) } - private val browsingModeManager get() = (activity as HomeActivity).browsingModeManager - private val collectionStorageObserver = object : TabCollectionStorage.Observer { @SuppressLint("NotifyDataSetChanged") override fun onCollectionRenamed(tabCollection: TabCollection, title: String) { @@ -254,8 +252,6 @@ class HomeFragment : Fragment() { orientation = requireContext().resources.configuration.orientation, ) - components.appStore.dispatch(AppAction.ModeChange(browsingModeManager.mode)) - lifecycleScope.launch(IO) { if (requireContext().settings().showPocketRecommendationsFeature) { val categories = components.core.pocketStoriesService.getStories() @@ -516,16 +512,8 @@ class HomeFragment : Fragment() { * doesn't get run right away which means that we won't draw on the first layout pass. */ private fun updateSessionControlView() { - if (browsingModeManager.mode == BrowsingMode.Private) { - binding.root.consumeFrom(requireContext().components.appStore, viewLifecycleOwner) { - sessionControlView?.update(it) - } - } else { - sessionControlView?.update(requireContext().components.appStore.state) - - binding.root.consumeFrom(requireContext().components.appStore, viewLifecycleOwner) { - sessionControlView?.update(it, shouldReportMetrics = true) - } + binding.root.consumeFrom(requireContext().components.appStore, viewLifecycleOwner) { + sessionControlView?.update(it, shouldReportMetrics = it.mode != BrowsingMode.Private) } } @@ -553,7 +541,7 @@ class HomeFragment : Fragment() { super.onViewCreated(view, savedInstanceState) HomeScreen.homeScreenDisplayed.record(NoExtras()) HomeScreen.homeScreenViewCount.add() - if (!browsingModeManager.mode.isPrivate) { + if (!requireComponents.appStore.state.mode.isPrivate) { HomeScreen.standardHomepageViewCount.add() } @@ -571,14 +559,17 @@ class HomeFragment : Fragment() { tabCounterView = TabCounterView( context = requireContext(), - browsingModeManager = browsingModeManager, + browsingModeManager = (activity as HomeActivity).browsingModeManager, navController = findNavController(), tabCounter = binding.tabButton, ) toolbarView?.build() - PrivateBrowsingButtonView(binding.privateBrowsingButton, browsingModeManager) { newMode -> + PrivateBrowsingButtonView( + button = binding.privateBrowsingButton, + mode = requireComponents.appStore.state.mode, + ) { newMode -> sessionControlInteractor.onPrivateModeButtonClicked(newMode) Homepage.privateModeIconTapped.record(mozilla.telemetry.glean.private.NoExtras()) } @@ -783,7 +774,7 @@ class HomeFragment : Fragment() { ) } - if (browsingModeManager.mode.isPrivate && + if (requireComponents.appStore.state.mode.isPrivate && // We will be showing the search dialog and don't want to show the CFR while the dialog shows !bundleArgs.getBoolean(FOCUS_ON_ADDRESS_BAR) && context.settings().shouldShowPrivateModeCfr @@ -822,7 +813,7 @@ class HomeFragment : Fragment() { override fun onResume() { super.onResume() - if (browsingModeManager.mode == BrowsingMode.Private) { + if (requireComponents.appStore.state.mode == BrowsingMode.Private) { activity?.window?.setBackgroundDrawableResource(R.drawable.private_home_background_gradient) } @@ -838,7 +829,7 @@ class HomeFragment : Fragment() { override fun onPause() { super.onPause() - if (browsingModeManager.mode == BrowsingMode.Private) { + if (requireComponents.appStore.state.mode == BrowsingMode.Private) { activity?.window?.setBackgroundDrawable( ColorDrawable( ContextCompat.getColor( @@ -992,7 +983,7 @@ class HomeFragment : Fragment() { } private fun showCollectionsPlaceholder(browserState: BrowserState) { - val tabCount = if (browsingModeManager.mode.isPrivate) { + val tabCount = if (requireComponents.appStore.state.mode.isPrivate) { browserState.privateTabs.size } else { browserState.normalTabs.size diff --git a/fenix/app/src/main/java/org/mozilla/fenix/home/PrivateBrowsingButtonView.kt b/fenix/app/src/main/java/org/mozilla/fenix/home/PrivateBrowsingButtonView.kt index a06d733d4974..85e543db189e 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/home/PrivateBrowsingButtonView.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/home/PrivateBrowsingButtonView.kt @@ -8,19 +8,22 @@ import android.view.View import androidx.annotation.StringRes import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager /** * Sets up the private browsing toggle button on the [HomeFragment]. + * + * @param button The button to bind content descriptions and click listeners to. + * @param mode The current [BrowsingMode]. + * @param onClick Click handler for the button. */ class PrivateBrowsingButtonView( button: View, - private val browsingModeManager: BrowsingModeManager, + private val mode: BrowsingMode, private val onClick: (BrowsingMode) -> Unit, ) : View.OnClickListener { init { - button.contentDescription = button.context.getString(getContentDescription(browsingModeManager.mode)) + button.contentDescription = button.context.getString(getContentDescription(mode)) button.setOnClickListener(this) } @@ -28,10 +31,7 @@ class PrivateBrowsingButtonView( * Calls [onClick] with the new [BrowsingMode] and updates the [browsingModeManager]. */ override fun onClick(v: View) { - val invertedMode = BrowsingMode.fromBoolean(!browsingModeManager.mode.isPrivate) - onClick(invertedMode) - - browsingModeManager.mode = invertedMode + onClick(mode.inverted) } companion object { diff --git a/fenix/app/src/main/java/org/mozilla/fenix/home/intent/IntentReducer.kt b/fenix/app/src/main/java/org/mozilla/fenix/home/intent/IntentReducer.kt new file mode 100644 index 000000000000..34272428e659 --- /dev/null +++ b/fenix/app/src/main/java/org/mozilla/fenix/home/intent/IntentReducer.kt @@ -0,0 +1,21 @@ +/* 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.home.intent + +import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.components.appstate.AppAction.IntentAction +import org.mozilla.fenix.components.appstate.AppState + +/** + * Reducer to handle updating [AppState] with the result of an [IntentAction]. + */ +object IntentReducer { + /** + * Reducer to handle updating [AppState] with the result of an [IntentAction]. + */ + fun reduce(state: AppState, action: IntentAction): AppState = when (action) { + is IntentAction.EnterPrivateBrowsing -> state.copy(mode = BrowsingMode.Private) + } +} diff --git a/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModeBindingTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModeBindingTest.kt new file mode 100644 index 000000000000..1a175986d4f4 --- /dev/null +++ b/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModeBindingTest.kt @@ -0,0 +1,101 @@ +/* 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 + +import android.view.Window +import android.view.WindowManager +import kotlinx.coroutines.test.runTest +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.mock +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.rule.runTestOnMain +import mozilla.components.support.test.whenever +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.mockito.Mockito.anyInt +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.theme.ThemeManager +import org.mozilla.fenix.utils.Settings + +class BrowsingModeBindingTest { + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + + private lateinit var themeManager: ThemeManager + private lateinit var window: Window + private lateinit var settings: Settings + private val retrieveWindow = { window } + + private lateinit var appStore: AppStore + + private lateinit var binding: BrowsingModeBinding + + @Before + fun setup() { + themeManager = mock() + window = mock() + settings = mock() + whenever(window.clearFlags(anyInt())).then { } + + appStore = AppStore() + // wait for Init action + appStore.waitUntilIdle() + binding = BrowsingModeBinding( + appStore, + themeManager, + retrieveWindow, + settings, + coroutinesTestRule.testDispatcher, + ) + binding.start() + } + + @After + fun teardown() { + binding.stop() + } + + @Test + fun `WHEN mode updated THEN theme manager is also updated`() = runTest { + appStore.dispatch(AppAction.ModeChange(BrowsingMode.Private)).joinBlocking() + + verify(themeManager).currentTheme = BrowsingMode.Private + } + + @Test + fun `GIVEN screenshots not allowed in private mode WHEN mode changes to private THEN secure flag added to window`() = runTestOnMain { + whenever(window.addFlags(anyInt())).then { } + whenever(settings.allowScreenshotsInPrivateMode).thenReturn(false) + + appStore.dispatch(AppAction.ModeChange(BrowsingMode.Private)).joinBlocking() + + verify(window).addFlags(WindowManager.LayoutParams.FLAG_SECURE) + } + + @Test + fun `GIVEN screenshots allowed in private mode when mode changes to private THEN secure flag not added to window`() = runTest { + whenever(settings.allowScreenshotsInPrivateMode).thenReturn(true) + + appStore.dispatch(AppAction.ModeChange(BrowsingMode.Private)).joinBlocking() + + verify(window, never()).addFlags(WindowManager.LayoutParams.FLAG_SECURE) + } + + @Test + fun `WHEN mode changed to normal THEN secure flag cleared from window`() { + appStore.dispatch(AppAction.ModeChange(BrowsingMode.Private)).joinBlocking() + appStore.dispatch(AppAction.ModeChange(BrowsingMode.Normal)).joinBlocking() + + verify(window, times(2)).clearFlags(WindowManager.LayoutParams.FLAG_SECURE) + } +} diff --git a/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModePersistenceMiddlewareTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModePersistenceMiddlewareTest.kt new file mode 100644 index 000000000000..2f0c40a68ff2 --- /dev/null +++ b/fenix/app/src/test/java/org/mozilla/fenix/BrowsingModePersistenceMiddlewareTest.kt @@ -0,0 +1,65 @@ +/* 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 + +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.mock +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.whenever +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.mockito.Mockito.verify +import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.utils.Settings + +class BrowsingModePersistenceMiddlewareTest { + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + + private lateinit var settings: Settings + + @Before + fun setup() { + settings = mock() + } + + @Test + fun `WHEN store initialization intercepted THEN mode read from settings and update dispatched`() = runTest { + val cachedMode = BrowsingMode.Private + whenever(settings.lastKnownMode).thenReturn(cachedMode) + + val middleware = BrowsingModePersistenceMiddleware(settings, this) + val store = AppStore(middlewares = listOf(middleware)) + // Wait for Init action + store.waitUntilIdle() + // Wait for middleware launched coroutine + this.advanceUntilIdle() + // Wait for ModeChange action + store.waitUntilIdle() + + assertEquals(cachedMode, store.state.mode) + } + + @Test + fun `WHEN mode change THEN mode updated on disk`() = runTest { + val cachedMode = BrowsingMode.Normal + whenever(settings.lastKnownMode).thenReturn(cachedMode) + val updatedMode = BrowsingMode.Private + + val middleware = BrowsingModePersistenceMiddleware(settings, this) + val store = AppStore(middlewares = listOf(middleware)) + store.dispatch(AppAction.ModeChange(updatedMode)).joinBlocking() + this.advanceUntilIdle() + + verify(settings).lastKnownMode = updatedMode + } +} diff --git a/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt index cb6cff486a38..3215c8f04638 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt @@ -10,11 +10,11 @@ import io.mockk.every import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import mozilla.components.support.test.libstate.ext.waitUntilIdle import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.utils.toSafeIntent import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before @@ -22,7 +22,8 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppState import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getIntentSource import org.mozilla.fenix.ext.settings @@ -33,11 +34,13 @@ import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) class HomeActivityTest { + private var appStore = AppStore() private lateinit var activity: HomeActivity @Before fun setup() { activity = spyk(HomeActivity()) + every { activity.components.appStore } returns appStore } @Test @@ -55,24 +58,29 @@ class HomeActivityTest { } @Test - fun `getModeFromIntentOrLastKnown returns mode from settings when intent does not set`() { + fun `GIVEN intent not set WHEN setModeFromIntent THEN mode not updated`() { every { testContext.settings() } returns Settings(testContext) every { activity.applicationContext } returns testContext - testContext.settings().lastKnownMode = BrowsingMode.Private + testContext.settings().lastKnownMode = BrowsingMode.Normal + + activity.setModeFromIntent(null) - assertEquals(testContext.settings().lastKnownMode, activity.getModeFromIntentOrLastKnown(null)) + assertEquals(BrowsingMode.Normal, appStore.state.mode) } @Test - fun `getModeFromIntentOrLastKnown returns mode from intent when set`() { + fun `GIVEN intent set WHEN setModeFromIntent THEN mode updated`() { every { testContext.settings() } returns Settings(testContext) + every { testContext.components.appStore } returns appStore + every { activity.applicationContext } returns testContext testContext.settings().lastKnownMode = BrowsingMode.Normal val intent = Intent() intent.putExtra(PRIVATE_BROWSING_MODE, true) + activity.setModeFromIntent(intent) + appStore.waitUntilIdle() - assertNotEquals(testContext.settings().lastKnownMode, activity.getModeFromIntentOrLastKnown(intent)) - assertEquals(BrowsingMode.Private, activity.getModeFromIntentOrLastKnown(intent)) + assertEquals(BrowsingMode.Private, appStore.state.mode) } @Test @@ -96,15 +104,11 @@ class HomeActivityTest { @Test fun `navigateToBrowserOnColdStart in normal mode navigates to browser`() { - val browsingModeManager: BrowsingModeManager = mockk() - every { browsingModeManager.mode } returns BrowsingMode.Normal - val settings: Settings = mockk() every { settings.shouldReturnToBrowser } returns true - every { activity.components.settings.shouldReturnToBrowser } returns true + every { activity.components.settings } returns settings every { activity.openToBrowser(any(), any()) } returns Unit - activity.browsingModeManager = browsingModeManager activity.navigateToBrowserOnColdStart() verify(exactly = 1) { activity.openToBrowser(BrowserDirection.FromGlobal, null) } @@ -112,15 +116,13 @@ class HomeActivityTest { @Test fun `navigateToBrowserOnColdStart in private mode does not navigate to browser`() { - val browsingModeManager: BrowsingModeManager = mockk() - every { browsingModeManager.mode } returns BrowsingMode.Private - + val privateAppStore = AppStore(AppState(mode = BrowsingMode.Private)) val settings: Settings = mockk() every { settings.shouldReturnToBrowser } returns true + every { activity.components.appStore } returns privateAppStore every { activity.components.settings.shouldReturnToBrowser } returns true every { activity.openToBrowser(any(), any()) } returns Unit - activity.browsingModeManager = browsingModeManager activity.navigateToBrowserOnColdStart() verify(exactly = 0) { activity.openToBrowser(BrowserDirection.FromGlobal, null) } diff --git a/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt index 4f9abfaac268..cd17de98546e 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt @@ -57,15 +57,12 @@ class ExternalAppBrowserActivityTest { @Test fun `navigateToBrowserOnColdStart does nothing for external app browser activity`() { val activity = spyk(ExternalAppBrowserActivity()) - val browsingModeManager: BrowsingModeManager = mockk() - every { browsingModeManager.mode } returns BrowsingMode.Normal val settings: Settings = mockk() every { settings.shouldReturnToBrowser } returns true every { activity.components.settings.shouldReturnToBrowser } returns true every { activity.openToBrowser(any(), any()) } returns Unit - activity.browsingModeManager = browsingModeManager activity.navigateToBrowserOnColdStart() verify(exactly = 0) { activity.openToBrowser(BrowserDirection.FromGlobal, null) } diff --git a/fenix/app/src/test/java/org/mozilla/fenix/home/PrivateBrowsingButtonViewTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/home/PrivateBrowsingButtonViewTest.kt index 34e0fd0f88d5..cc0554569a4b 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/home/PrivateBrowsingButtonViewTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/home/PrivateBrowsingButtonViewTest.kt @@ -13,7 +13,6 @@ import org.junit.Before import org.junit.Test import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager class PrivateBrowsingButtonViewTest { @@ -21,27 +20,23 @@ class PrivateBrowsingButtonViewTest { private val disable = "Disable private browsing" private lateinit var button: View - private lateinit var browsingModeManager: BrowsingModeManager @Before fun setup() { button = mockk(relaxed = true) - browsingModeManager = mockk(relaxed = true) every { button.context.getString(R.string.content_description_private_browsing_button) } returns enable every { button.context.getString(R.string.content_description_disable_private_browsing_button) } returns disable - every { browsingModeManager.mode } returns BrowsingMode.Normal } @Test fun `constructor sets contentDescription and click listener`() { - val view = PrivateBrowsingButtonView(button, browsingModeManager) {} + val view = PrivateBrowsingButtonView(button, BrowsingMode.Normal) {} verify { button.context.getString(R.string.content_description_private_browsing_button) } verify { button.contentDescription = enable } verify { button.setOnClickListener(view) } - every { browsingModeManager.mode } returns BrowsingMode.Private - val privateView = PrivateBrowsingButtonView(button, browsingModeManager) {} + val privateView = PrivateBrowsingButtonView(button, BrowsingMode.Private) {} verify { button.context.getString(R.string.content_description_disable_private_browsing_button) } verify { button.contentDescription = disable } verify { button.setOnClickListener(privateView) } @@ -49,25 +44,21 @@ class PrivateBrowsingButtonViewTest { @Test fun `click listener calls onClick with inverted mode from normal mode`() { - every { browsingModeManager.mode } returns BrowsingMode.Normal - var mode: BrowsingMode? = null - val view = PrivateBrowsingButtonView(button, browsingModeManager) { mode = it } + var mode = BrowsingMode.Normal + val view = PrivateBrowsingButtonView(button, mode) { mode = it } view.onClick(button) assertEquals(BrowsingMode.Private, mode) - verify { browsingModeManager.mode = BrowsingMode.Private } } @Test fun `click listener calls onClick with inverted mode from private mode`() { - every { browsingModeManager.mode } returns BrowsingMode.Private - var mode: BrowsingMode? = null - val view = PrivateBrowsingButtonView(button, browsingModeManager) { mode = it } + var mode = BrowsingMode.Private + val view = PrivateBrowsingButtonView(button, mode) { mode = it } view.onClick(button) assertEquals(BrowsingMode.Normal, mode) - verify { browsingModeManager.mode = BrowsingMode.Normal } } }