For #12383 #15407 #12860 - Switch to using shared view model for session to delete on home

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -19,6 +19,7 @@ import androidx.core.net.toUri
 import androidx.core.view.isVisible
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
+import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
 import androidx.preference.PreferenceManager
@@ -109,6 +110,7 @@ import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
+import org.mozilla.fenix.home.HomeScreenViewModel
 import org.mozilla.fenix.home.SharedViewModel
 import org.mozilla.fenix.theme.ThemeManager
 import org.mozilla.fenix.utils.allowUndo
@@ -205,6 +207,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
         requireContext().accessibilityManager.addAccessibilityStateChangeListener(this)
     }
 
+    private val homeViewModel: HomeScreenViewModel by activityViewModels {
+        ViewModelProvider.NewInstanceFactory() // this is a workaround for #4652
+    }
+
     @Suppress("ComplexMethod", "LongMethod")
     @CallSuper
     protected open fun initializeUI(view: View): Session? {
@@ -244,7 +250,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                 readerModeController = readerMenuController,
                 sessionManager = requireComponents.core.sessionManager,
                 engineView = engineView,
-                browserAnimator = browserAnimator,
+                homeViewModel = homeViewModel,
                 customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) },
                 onTabCounterClicked = {
                     thumbnailsFeature.get()?.requestScreenshot()
@@ -811,7 +817,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
 
     @CallSuper
     override fun onSessionSelected(session: Session) {
-        updateThemeForSession(session)
+        if (!this.isRemoving) {
+            updateThemeForSession(session)
+        }
         if (!browserInitialized) {
             // Initializing a new coroutineScope to avoid ConcurrentModificationException in ObserverRegistry
             // This will be removed when ObserverRegistry is deprecated by browser-state.

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt

@@ -11,7 +11,6 @@ import mozilla.components.concept.engine.EngineView
 import mozilla.components.support.ktx.kotlin.isUrl
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
-import org.mozilla.fenix.browser.BrowserAnimator
 import org.mozilla.fenix.browser.BrowserAnimator.Companion.getToolbarNavOptions
 import org.mozilla.fenix.browser.BrowserFragmentDirections
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
@@ -22,6 +21,7 @@ import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.sessionsOfType
 import org.mozilla.fenix.ext.settings
+import org.mozilla.fenix.home.HomeScreenViewModel
 
 /**
  * An interface that handles the view manipulation of the BrowserToolbar, triggered by the Interactor
@@ -43,7 +43,7 @@ class DefaultBrowserToolbarController(
     private val readerModeController: ReaderModeController,
     private val sessionManager: SessionManager,
     private val engineView: EngineView,
-    private val browserAnimator: BrowserAnimator,
+    private val homeViewModel: HomeScreenViewModel,
     private val customTabSession: Session?,
     private val onTabCounterClicked: () -> Unit,
     private val onCloseTab: (Session) -> Unit
@@ -110,10 +110,9 @@ class DefaultBrowserToolbarController(
                     if (sessionManager.sessionsOfType(it.private).count() == 1) {
                         // The tab tray always returns to normal mode so do that here too
                         activity.browsingModeManager.mode = BrowsingMode.Normal
+                        homeViewModel.sessionToDelete = it.id
                         navController.navigate(
-                            BrowserFragmentDirections.actionGlobalHome(
+                            BrowserFragmentDirections.actionGlobalHome()
-                                sessionToDelete = it.id
-                            )
                         )
                     } else {
                         onCloseTab.invoke(it)

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -33,7 +33,6 @@ import androidx.core.view.isVisible
 import androidx.core.view.updateLayoutParams
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
-import androidx.fragment.app.viewModels
 import androidx.lifecycle.Observer
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.lifecycleScope
@@ -118,8 +117,8 @@ class HomeFragment : Fragment() {
     private val args by navArgs<HomeFragmentArgs>()
     private lateinit var bundleArgs: Bundle
 
-    private val homeViewModel: HomeScreenViewModel by viewModels {
+    private val homeViewModel: HomeScreenViewModel by activityViewModels {
-        ViewModelProvider.AndroidViewModelFactory(requireActivity().application)
+        ViewModelProvider.NewInstanceFactory() // this is a workaround for #4652
     }
 
     private val snackbarAnchorView: View?
@@ -452,7 +451,7 @@ class HomeFragment : Fragment() {
             updateTabCounter(it)
         }
 
-        bundleArgs.getString(SESSION_TO_DELETE)?.also {
+        homeViewModel.sessionToDelete?.also {
             if (it == ALL_NORMAL_TABS || it == ALL_PRIVATE_TABS) {
                 removeAllTabsAndShowSnackbar(it)
             } else {
@@ -460,6 +459,8 @@ class HomeFragment : Fragment() {
             }
         }
 
+        homeViewModel.sessionToDelete = null
+
         updateTabCounter(requireComponents.core.store.state)
 
         if (bundleArgs.getBoolean(FOCUS_ON_ADDRESS_BAR)) {
@@ -989,7 +990,6 @@ class HomeFragment : Fragment() {
         const val ALL_PRIVATE_TABS = "all_private"
 
         private const val FOCUS_ON_ADDRESS_BAR = "focusOnAddressBar"
-        private const val SESSION_TO_DELETE = "session_to_delete"
         private const val ANIMATION_DELAY = 100L
 
         private const val NON_TAB_ITEM_NUM = 3

app/src/main/java/org/mozilla/fenix/home/HomeScreenViewModel.kt

@@ -8,6 +8,11 @@ import android.os.Parcelable
 import androidx.lifecycle.ViewModel
 
 class HomeScreenViewModel : ViewModel() {
+    /**
+     * Used to delete a specific session once the home screen is resumed
+     */
+    var sessionToDelete: String? = null
+
     var layoutManagerState: Parcelable? = null
 
     /**

app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt

@@ -16,6 +16,8 @@ import androidx.appcompat.app.AlertDialog
 import androidx.appcompat.app.AppCompatDialogFragment
 import androidx.core.view.isVisible
 import androidx.core.view.updatePadding
+import androidx.fragment.app.activityViewModels
+import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
 import androidx.navigation.fragment.navArgs
@@ -41,8 +43,8 @@ import mozilla.components.support.base.feature.UserInteractionHandler
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.ktx.android.view.showKeyboard
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.NavGraphDirections
 import org.mozilla.fenix.R
-import org.mozilla.fenix.browser.BrowserFragmentDirections
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.components.StoreProvider
 import org.mozilla.fenix.components.TabCollectionStorage
@@ -53,6 +55,7 @@ import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.normalSessionSize
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
+import org.mozilla.fenix.home.HomeScreenViewModel
 import org.mozilla.fenix.tabtray.TabTrayDialogFragmentState.Mode
 import org.mozilla.fenix.utils.allowUndo
 
@@ -263,7 +266,8 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
         val session = sessionManager.findSessionById(sessionId) ?: return
 
         // Check if this is the last tab of this session type
-        val isLastOpenTab = store.state.tabs.filter { it.content.private == tab.content.private }.size == 1
+        val isLastOpenTab =
+            store.state.tabs.filter { it.content.private == tab.content.private }.size == 1
         if (isLastOpenTab) {
             dismissTabTrayAndNavigateHome(sessionId)
             return
@@ -282,7 +286,11 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
             snackbarMessage,
             getString(R.string.snackbar_deleted_undo),
             {
-                sessionManager.add(session, isSelected, engineSessionState = tab.engineState.engineSessionState)
+                sessionManager.add(
+                    session,
+                    isSelected,
+                    engineSessionState = tab.engineState.engineSessionState
+                )
                 _tabTrayView?.scrollToTab(session.id)
             },
             operation = { },
@@ -291,8 +299,13 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
         )
     }
 
+    private val homeViewModel: HomeScreenViewModel by activityViewModels {
+        ViewModelProvider.NewInstanceFactory() // this is a workaround for #4652
+    }
+
     private fun dismissTabTrayAndNavigateHome(sessionId: String) {
-        val directions = BrowserFragmentDirections.actionGlobalHome(sessionToDelete = sessionId)
+        homeViewModel.sessionToDelete = sessionId
+        val directions = NavGraphDirections.actionGlobalHome()
         findNavController().navigate(directions)
         dismissAllowingStateLoss()
     }

app/src/main/res/navigation/nav_graph.xml

@@ -132,11 +132,6 @@
             android:name="focusOnAddressBar"
             android:defaultValue="false"
             app:argType="boolean" />
-        <argument
-            android:name="session_to_delete"
-            android:defaultValue="@null"
-            app:argType="string"
-            app:nullable="true" />
     </fragment>
 
     <dialog

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt

@@ -38,23 +38,39 @@ import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+import org.mozilla.fenix.home.HomeScreenViewModel
 
 @RunWith(FenixRobolectricTestRunner::class)
 class DefaultBrowserToolbarControllerTest {
 
-    @RelaxedMockK private lateinit var activity: HomeActivity
+    @RelaxedMockK
-    @MockK(relaxUnitFun = true) private lateinit var navController: NavController
+    private lateinit var activity: HomeActivity
-    @RelaxedMockK private lateinit var onTabCounterClicked: () -> Unit
+    @MockK(relaxUnitFun = true)
-    @RelaxedMockK private lateinit var onCloseTab: (Session) -> Unit
+    private lateinit var navController: NavController
-    @RelaxedMockK private lateinit var sessionManager: SessionManager
+    @RelaxedMockK
-    @MockK(relaxUnitFun = true) private lateinit var engineView: EngineView
+    private lateinit var onTabCounterClicked: () -> Unit
-    @MockK private lateinit var currentSession: Session
+    @RelaxedMockK
-    @RelaxedMockK private lateinit var metrics: MetricController
+    private lateinit var onCloseTab: (Session) -> Unit
-    @RelaxedMockK private lateinit var searchUseCases: SearchUseCases
+    @RelaxedMockK
-    @RelaxedMockK private lateinit var sessionUseCases: SessionUseCases
+    private lateinit var sessionManager: SessionManager
-    @RelaxedMockK private lateinit var browserAnimator: BrowserAnimator
+    @MockK(relaxUnitFun = true)
-    @RelaxedMockK private lateinit var topSitesUseCase: TopSitesUseCases
+    private lateinit var engineView: EngineView
-    @RelaxedMockK private lateinit var readerModeController: ReaderModeController
+    @MockK
+    private lateinit var currentSession: Session
+    @RelaxedMockK
+    private lateinit var metrics: MetricController
+    @RelaxedMockK
+    private lateinit var searchUseCases: SearchUseCases
+    @RelaxedMockK
+    private lateinit var sessionUseCases: SessionUseCases
+    @RelaxedMockK
+    private lateinit var browserAnimator: BrowserAnimator
+    @RelaxedMockK
+    private lateinit var topSitesUseCase: TopSitesUseCases
+    @RelaxedMockK
+    private lateinit var readerModeController: ReaderModeController
+    @RelaxedMockK
+    private lateinit var homeViewModel: HomeScreenViewModel
 
     @Before
     fun setUp() {
@@ -193,7 +209,10 @@ class DefaultBrowserToolbarControllerTest {
 
         val controller = createController()
         controller.handleTabCounterItemInteraction(item)
-        verify { navController.navigate(BrowserFragmentDirections.actionGlobalHome(sessionToDelete = "1")) }
+        verify {
+            homeViewModel.sessionToDelete = "1"
+            navController.navigate(BrowserFragmentDirections.actionGlobalHome())
+        }
         assertEquals(BrowsingMode.Normal, browsingModeManager.mode)
     }
 
@@ -262,7 +281,7 @@ class DefaultBrowserToolbarControllerTest {
         navController = navController,
         metrics = metrics,
         engineView = engineView,
-        browserAnimator = browserAnimator,
+        homeViewModel = homeViewModel,
         customTabSession = customTabSession,
         readerModeController = readerModeController,
         sessionManager = sessionManager,