Bug 1973324 - Part 2: Refactor setting the appropriate browsing mode into BrowsingModeManger r=Roger,android-reviewers

- Refactors `HomeActivity.getModeFromIntentOrLastKnown` into `BrowsingModeManger`. This moves more of the logic related to computing the browsing mode from the provided intent, last known mode settings and number of private tabs opened into BrowsingModeManger and out of the HomeActivity.
- Introduces a public `BrowsingModeManager.updateMode` function for updating the browsing mode with a new Intent.

Differential Revision: https://phabricator.services.mozilla.com/D255455

Author: gabrielluong

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt

@@ -321,7 +321,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
         // 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))
+            browsingModeManager = createBrowsingModeManager(intent)
             setupTheme()
 
             super.onCreate(savedInstanceState)
@@ -887,7 +887,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
             ) + externalSourceIntentProcessors
         val intentHandled =
             intentProcessors.any { it.process(intent, navHost.navController, this.intent, settings()) }
-        browsingModeManager.mode = getModeFromIntentOrLastKnown(intent)
+        browsingModeManager.updateMode(intent)
 
         if (intentHandled) {
             supportFragmentManager
@@ -1104,29 +1104,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
         super.onUserLeaveHint()
     }
 
-    /**
-     * External sources such as 3rd party links and shortcuts use this function to enter
-     * private mode directly before the content view is created. Returns the mode set by the intent
-     * otherwise falls back to normal browsing mode.
-     */
-    @VisibleForTesting
-    internal fun getModeFromIntentOrLastKnown(intent: Intent?): BrowsingMode {
-        intent?.toSafeIntent()?.let {
-            if (it.hasExtra(PRIVATE_BROWSING_MODE)) {
-                val startPrivateMode = it.getBooleanExtra(PRIVATE_BROWSING_MODE, false)
-                return BrowsingMode.fromBoolean(isPrivate = startPrivateMode)
-            }
-        }
-
-        if (settings().lastKnownMode.isPrivate &&
-            components.core.store.state.getNormalOrPrivateTabs(private = true).isNotEmpty()
-        ) {
-            return BrowsingMode.Private
-        }
-
-        return BrowsingMode.Normal
-    }
-
     /**
      * Determines whether the activity should be pushed to be backstack (i.e., 'minimized' to the recents
      * screen) upon starting.
@@ -1140,11 +1117,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 exclusively handles it's own theming unless in private mode.
@@ -1295,9 +1267,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
         return super.getSystemService(name)
     }
 
-    private fun createBrowsingModeManager(initialMode: BrowsingMode): BrowsingModeManager {
+    private fun createBrowsingModeManager(intent: Intent?): BrowsingModeManager {
         return DefaultBrowsingModeManager(
-            initialMode = initialMode,
+            intent = intent,
+            store = components.core.store,
             settings = components.settings,
             modeDidChange = { newMode ->
                 updateSecureWindowFlags(newMode)
@@ -1308,7 +1281,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
                 components.appStore.dispatch(AppAction.BrowsingModeManagerModeChanged(mode = newMode))
             },
         ).also {
-            updateSecureWindowFlags(initialMode)
+            updateSecureWindowFlags(it.mode)
         }
     }
 

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/browsingmode/BrowsingModeManager.kt

@@ -4,6 +4,11 @@
 
 package org.mozilla.fenix.browser.browsingmode
 
+import android.content.Intent
+import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
+import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.support.utils.toSafeIntent
+import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppState
 import org.mozilla.fenix.utils.Settings
@@ -31,28 +36,64 @@ enum class BrowsingMode {
 
 interface BrowsingModeManager {
     var mode: BrowsingMode
+
+    /**
+     * Updates the [BrowsingMode] based on the [Intent] that started the activity.
+     *
+     * @param intent The [Intent] that started the activity.
+     */
+    fun updateMode(intent: Intent? = null)
 }
 
 /**
  * Default implementation of [BrowsingModeManager] that tracks the current [BrowsingMode],
  * persists it to [Settings], and synchronizes it with [AppStore].
  *
- * @param initialMode The initial [BrowsingMode]
+ * @param intent The [Intent] that started the activity.
+ * @param store The [BrowserStore] to observe the private tabs state from.
  * @param settings Used to persist the last known mode across sessions.
  * @param modeDidChange Callback that is invoked whenever the browsing mode changes.
  * @param updateAppStateMode Callback used to update the [AppState.mode].
  */
 class DefaultBrowsingModeManager(
-    private var initialMode: BrowsingMode,
+    intent: Intent?,
+    private val store: BrowserStore,
     private val settings: Settings,
     private val modeDidChange: (BrowsingMode) -> Unit,
     private val updateAppStateMode: (BrowsingMode) -> Unit,
 ) : BrowsingModeManager {
-    override var mode: BrowsingMode = initialMode
+    override var mode: BrowsingMode = getModeFromIntentOrLastKnown(intent)
         set(value) {
             field = value
             modeDidChange(value)
             settings.lastKnownMode = value
             updateAppStateMode(value)
         }
+
+    override fun updateMode(intent: Intent?) {
+        val mode = getModeFromIntentOrLastKnown(intent)
+        if (this.mode != mode) {
+            this.mode = mode
+        }
+    }
+
+    /**
+     * Returns the [BrowsingMode] set by the [intent] or the last known browsing mode based on
+     * whether or not the user was in private mode and has any private tabs, otherwise fallback to
+     * [BrowsingMode.Normal].
+     */
+    private fun getModeFromIntentOrLastKnown(intent: Intent?): BrowsingMode {
+        intent?.toSafeIntent()?.let {
+            if (it.hasExtra(PRIVATE_BROWSING_MODE)) {
+                val startPrivateMode = it.getBooleanExtra(PRIVATE_BROWSING_MODE, false)
+                return BrowsingMode.fromBoolean(isPrivate = startPrivateMode)
+            }
+        }
+
+        if (settings.lastKnownMode.isPrivate && store.state.getNormalOrPrivateTabs(private = true).isNotEmpty()) {
+            return BrowsingMode.Private
+        }
+
+        return BrowsingMode.Normal
+    }
 }

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt

@@ -20,7 +20,6 @@ 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.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
@@ -29,7 +28,6 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.GleanMetrics.Metrics
-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
@@ -85,54 +83,6 @@ class HomeActivityTest {
         assertNull(activity.getIntentSource(otherIntent))
     }
 
-    @Test
-    fun `GIVEN browsing mode is not set by intent and private mode with a tab persisted WHEN getModeFromIntentOrLastKnown is called THEN returns normal browsing mode`() {
-        val browserStore = BrowserStore(
-            BrowserState(
-                tabs = listOf(
-                    createTab(url = "https://mozilla.org", private = true),
-                ),
-            ),
-        )
-
-        every { testContext.settings() } returns Settings(testContext)
-        every { activity.applicationContext } returns testContext
-        every { testContext.components.core.store } returns browserStore
-
-        testContext.settings().lastKnownMode = BrowsingMode.Private
-
-        assertEquals(BrowsingMode.Private, activity.getModeFromIntentOrLastKnown(null))
-
-        testContext.settings().lastKnownMode = BrowsingMode.Normal
-
-        assertEquals(BrowsingMode.Normal, activity.getModeFromIntentOrLastKnown(null))
-    }
-
-    @Test
-    fun `GIVEN last known mode is private mode and no tabs persisted WHEN getModeFromIntentOrLastKnown is called THEN returns normal browsing mode`() {
-        val browserStore = BrowserStore()
-
-        every { testContext.settings() } returns Settings(testContext)
-        every { activity.applicationContext } returns testContext
-        every { testContext.components.core.store } returns browserStore
-
-        testContext.settings().lastKnownMode = BrowsingMode.Private
-
-        assertEquals(BrowsingMode.Normal, activity.getModeFromIntentOrLastKnown(null))
-    }
-
-    @Test
-    fun `getModeFromIntentOrLastKnown returns mode from intent when set`() {
-        every { testContext.settings() } returns Settings(testContext)
-        testContext.settings().lastKnownMode = BrowsingMode.Normal
-
-        val intent = Intent()
-        intent.putExtra(PRIVATE_BROWSING_MODE, true)
-
-        assertNotEquals(testContext.settings().lastKnownMode, activity.getModeFromIntentOrLastKnown(intent))
-        assertEquals(BrowsingMode.Private, activity.getModeFromIntentOrLastKnown(intent))
-    }
-
     @Test
     fun `isActivityColdStarted returns true for null savedInstanceState and not launched from history`() {
         assertTrue(activity.isActivityColdStarted(Intent(), null))