Merge mozilla-mobile#4358

4358: Closes mozilla-mobile#4348: Empty toolbar after switching from custom tab to browser r=pocmo a=csadilek

Pretty much exactly as described in mozilla-mobile#4348 (comment). We need to update our store to move the tab from the custom tab list to the regular tab list. I decided to re-use our existing events (Remove, Add) instead of introducing a new one, as it's technically two different reducers having to handle it.

I've verified that this fixes the issues in Fenix (and r-b).

Co-authored-by: Christian Sadilek <[email protected]>

Author: MozLando

components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt

@@ -10,6 +10,7 @@ import mozilla.components.browser.session.engine.request.LoadRequestMetadata
 import mozilla.components.browser.session.engine.request.LoadRequestOption
 import mozilla.components.browser.session.ext.syncDispatch
 import mozilla.components.browser.session.ext.toSecurityInfoState
+import mozilla.components.browser.session.ext.toTabSessionState
 import mozilla.components.browser.state.action.ContentAction.ConsumeDownloadAction
 import mozilla.components.browser.state.action.ContentAction.ConsumeHitResultAction
 import mozilla.components.browser.state.action.ContentAction.RemoveIconAction
@@ -24,6 +25,8 @@ import mozilla.components.browser.state.action.ContentAction.UpdateSecurityInfoA
 import mozilla.components.browser.state.action.ContentAction.UpdateThumbnailAction
 import mozilla.components.browser.state.action.ContentAction.UpdateTitleAction
 import mozilla.components.browser.state.action.ContentAction.UpdateUrlAction
+import mozilla.components.browser.state.action.CustomTabListAction.RemoveCustomTabAction
+import mozilla.components.browser.state.action.TabListAction.AddTabAction
 import mozilla.components.browser.state.action.TrackingProtectionAction
 import mozilla.components.browser.state.state.CustomTabConfig
 import mozilla.components.browser.state.store.BrowserStore
@@ -263,8 +266,16 @@ class Session(
     /**
      * Configuration data in case this session is used for a Custom Tab.
      */
-    var customTabConfig: CustomTabConfig? by Delegates.observable<CustomTabConfig?>(null) { _, _, new ->
+    var customTabConfig: CustomTabConfig? by Delegates.observable<CustomTabConfig?>(null) { _, old, new ->
         notifyObservers { onCustomTabConfigChanged(this@Session, new) }
+
+        // The custom tab config is set to null when we're migrating custom
+        // tabs to regular tabs, so we have to dispatch the corresponding
+        // browser actions to keep the store in sync.
+        if (old != new && new == null) {
+            store?.syncDispatch(RemoveCustomTabAction(id))
+            store?.syncDispatch(AddTabAction(toTabSessionState()))
+        }
     }
 
     /**

components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt

@@ -70,6 +70,34 @@ class SessionManagerMigrationTest {
         assertEquals("https://www.mozilla.org", tab.content.url)
     }
 
+    @Test
+    fun `Migrating a custom tab session`() {
+        val store = BrowserStore()
+
+        val sessionManager = SessionManager(engine = mock(), store = store)
+
+        val customTabSession = Session("https://www.mozilla.org")
+        customTabSession.customTabConfig = mock()
+        sessionManager.add(customTabSession)
+
+        assertEquals(0, sessionManager.sessions.size)
+        assertEquals(1, sessionManager.all.size)
+        assertEquals(0, store.state.tabs.size)
+        assertEquals(1, store.state.customTabs.size)
+
+        val customTab = store.state.customTabs[0]
+        assertEquals("https://www.mozilla.org", customTab.content.url)
+
+        customTabSession.customTabConfig = null
+        assertEquals(1, sessionManager.sessions.size)
+        assertEquals(1, sessionManager.all.size)
+        assertEquals(1, store.state.tabs.size)
+        assertEquals(0, store.state.customTabs.size)
+
+        val tab = store.state.tabs[0]
+        assertEquals("https://www.mozilla.org", tab.content.url)
+    }
+
     @Test
     fun `Remove session`() {
         val store = BrowserStore()

components/browser/session/src/test/java/mozilla/components/browser/session/SessionTest.kt

@@ -13,7 +13,10 @@ import mozilla.components.browser.session.Session.Source
 import mozilla.components.browser.session.engine.request.LoadRequestMetadata
 import mozilla.components.browser.session.engine.request.LoadRequestOption
 import mozilla.components.browser.session.ext.toSecurityInfoState
+import mozilla.components.browser.session.ext.toTabSessionState
 import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.action.CustomTabListAction
+import mozilla.components.browser.state.action.TabListAction
 import mozilla.components.browser.state.state.CustomTabConfig
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.HitResult
@@ -305,6 +308,24 @@ class SessionTest {
         assertEquals("id", config!!.id)
     }
 
+    @Test
+    fun `action is dispatched when custom tab config is set`() {
+        val store: BrowserStore = mock()
+        `when`(store.dispatch(any())).thenReturn(mock())
+
+        val session = Session("https://www.mozilla.org")
+        session.store = store
+        session.customTabConfig = mock()
+
+        verify(store, never()).dispatch(CustomTabListAction.RemoveCustomTabAction(session.id))
+        verify(store, never()).dispatch(TabListAction.AddTabAction(session.toTabSessionState()))
+
+        session.customTabConfig = null
+        verify(store).dispatch(CustomTabListAction.RemoveCustomTabAction(session.id))
+        verify(store).dispatch(TabListAction.AddTabAction(session.toTabSessionState()))
+        verifyNoMoreInteractions(store)
+    }
+
     @Test
     fun `observer is notified when web app manifest is set`() {
         val manifest = WebAppManifest(