Issue #8535, #7304: Prevent dispatching individual actions to restore…

… tabs Co-authored-by: Jonathan Almeida <[email protected]>
mozilla-mobile · Oct 2, 2020 · 0a57b87 · 0a57b87
1 parent 537e45e
commit 0a57b87
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 36 deletions.
diff --git a/...onents/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt b/...onents/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt
@@ -10,11 +10,11 @@ import mozilla.components.browser.session.ext.toTabSessionState
 import mozilla.components.browser.state.action.CustomTabListAction
 import mozilla.components.browser.state.action.EngineAction.LinkEngineSessionAction
 import mozilla.components.browser.state.action.EngineAction.UpdateEngineSessionStateAction
-import mozilla.components.browser.state.action.ReaderAction
 import mozilla.components.browser.state.action.TabListAction
-import mozilla.components.browser.state.action.LastAccessAction
 import mozilla.components.browser.state.selector.findTab
+import mozilla.components.browser.state.state.EngineState
 import mozilla.components.browser.state.state.ReaderState
+import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.state.recover.RecoverableTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.Engine
@@ -223,7 +223,7 @@ class SessionManager(
             }
 
         val tabs = items.map { item ->
-            item.session.toTabSessionState()
+            item.session.toRestoredTabSessionState(item)
         }
 
         val selectedTabId = if (updateSelection && snapshot.selectedSessionIndex != NO_SELECTION) {
@@ -238,23 +238,6 @@ class SessionManager(
         }
 
         store?.syncDispatch(TabListAction.RestoreAction(tabs, selectedTabId))
-
-        items.forEach { item ->
-            item.engineSessionState?.let {
-                store?.syncDispatch(UpdateEngineSessionStateAction(item.session.id, it))
-            }
-
-            item.readerState?.let {
-                store?.syncDispatch(ReaderAction.UpdateReaderActiveAction(item.session.id, it.active))
-                it.activeUrl?.let { activeUrl ->
-                    store?.syncDispatch(ReaderAction.UpdateReaderActiveUrlAction(item.session.id, activeUrl))
-                }
-            }
-
-            if (item.lastAccess != 0L) {
-                store?.syncDispatch(LastAccessAction.UpdateLastAccessAction(item.session.id, item.lastAccess))
-            }
-        }
     }
 
     /**
@@ -447,3 +430,17 @@ fun SessionManager.runWithSessionIdOrSelected(
 
     return false
 }
+
+private fun Session.toRestoredTabSessionState(snapshot: SessionManager.Snapshot.Item): TabSessionState {
+    val engineState = if (snapshot.engineSessionState != null) {
+        EngineState(engineSessionState = snapshot.engineSessionState)
+    } else {
+        EngineState()
+    }
+
+    return toTabSessionState().copy(
+        engineState = engineState,
+        readerState = snapshot.readerState ?: ReaderState(),
+        lastAccess = snapshot.lastAccess
+    )
+}
diff --git a/...ts/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerTest.kt b/...ts/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerTest.kt
@@ -4,27 +4,22 @@
 
 package mozilla.components.browser.session
 
-import mozilla.components.browser.state.action.BrowserAction
-import mozilla.components.browser.state.action.TabListAction
-import mozilla.components.browser.state.action.LastAccessAction
+import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.state.CustomTabConfig
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.state.recover.toRecoverableTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.EngineSessionState
 import mozilla.components.support.test.any
-import mozilla.components.support.test.argumentCaptor
 import mozilla.components.support.test.mock
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.mockito.ArgumentMatchers.anyString
-
 import org.mockito.Mockito.`when`
-import org.mockito.Mockito.calls
-import org.mockito.Mockito.inOrder
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.never
 import org.mockito.Mockito.reset
@@ -1144,11 +1139,9 @@ class SessionManagerTest {
 
     @Test
     fun `WHEN restoring a session THEN dispatch updates to store`() {
-        val store = spy(BrowserStore())
-        val inOrder = inOrder(store)
+        val store = BrowserStore()
         val manager = SessionManager(mock(), store)
-        val session = Session("http://www.mozilla.org")
-        val captor = argumentCaptor<BrowserAction>()
+        val session = Session(id = "test123", initialUrl = "http://www.mozilla.org")
 
         manager.restore(SessionManager.Snapshot(listOf(
             SessionManager.Snapshot.Item(
@@ -1157,10 +1150,8 @@ class SessionManagerTest {
             )
         ), 0))
 
-        inOrder.verify(store, calls(2)).dispatch(captor.capture())
-
-        assertTrue(captor.allValues[0] is TabListAction.RestoreAction)
-        assertTrue(captor.allValues[1] is LastAccessAction.UpdateLastAccessAction)
-        assertEquals(123, (captor.allValues[1] as LastAccessAction.UpdateLastAccessAction).lastAccess)
+        val restoredTab = store.state.findTab(session.id)
+        assertNotNull(restoredTab!!)
+        assertEquals(123, restoredTab.lastAccess)
     }
 }