Commit 4e8403b4 authored by MozLando's avatar MozLando
Browse files

Merge #6885 



6885: Issue #6884: SessionManager.onTrimMemory(): Do not remove engine session state from sessions. r=csadilek a=pocmo

When trimming memory we want to save the state of an engine session, attach it to the holder and close the engine session. Right now we try to do this for all sessions and then "unlink" them. This causes the engine session state to be cleared from sessions that either did not have an engine session (e.g. restored sessions) or that have been trimmed before. We should only perform this action for sessions that actually have an engine session. All other sessions can be left as-is.


Co-authored-by: default avatarSebastian Kaspari <s.kaspari@gmail.com>
parents bd12ebd0 e6abb76c

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

+13 −5
Original line number Diff line number Diff line
@@ -88,6 +88,13 @@ class SessionManager( 


            session.engineSessionHolder.engineSession = null

            session.engineSessionHolder.engineObserver = null



            // Note: We could consider not clearing the engine session state here. Instead we could

            // try to actively set it here by calling save() on the engine session (if we have one).

            // That way adding the same Session again could restore the previous state automatically

            // (although we would need to make sure we do not override it with null in link()).

            // Clearing the engine session state would be left to the garbage collector whenever the

            // session itself gets collected.

            session.engineSessionHolder.engineSessionState = null



            store?.syncDispatch(UnlinkEngineSessionAction(session.id))
@@ -419,18 +426,19 @@ class SessionManager( 
                }



                if (closeEngineSessions) {

                    val state = it.engineSessionHolder.engineSession?.saveState()

                    val engineSession = it.engineSessionHolder.engineSession

                    if (engineSession != null) {

                        val state = engineSession.saveState()

                        linker.unlink(it)



                    if (state != null) {

                        it.engineSessionHolder.engineSessionState = state

                        states[it.id] = state

                    }



                        unlinkedEngineSessions++

                    }

                }

            }

        }



        logger.debug("Cleared $clearedThumbnails thumbnail(s) and unlinked $unlinkedEngineSessions engine sessions(s)")

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

+134 −2
Original line number Diff line number Diff line
@@ -1084,7 +1084,7 @@ class SessionManagerMigrationTest { 
    }



    @Test

    fun `Trimming memory - RUNNING_CRITICAL - Removes thumbnails`() {

    fun `Trimming memory - RUNNING_CRITICAL - Removes thumbnails and closes engine sessions`() {

        val store = BrowserStore()

        val manager = SessionManager(engine = mock(), store = store)
@@ -1226,7 +1226,139 @@ class SessionManagerMigrationTest { 
    }



    @Test

    fun `Trimming memory - RUNNING_ODERATE - Does not affect SessionManager`() {

    fun `Trimming memory - RUNNING_CRITICAL - Does not remove existing state from session`() {

        val store = BrowserStore()

        val manager = SessionManager(engine = mock(), store = store)



        val engineSession1 = createMockEngineSessionWithState()

        val engineSession2 = createMockEngineSessionWithState()

        val engineSessionState = mock<EngineSessionState>()



        manager.add(Session("https://www.mozilla.org").apply {

            thumbnail = mock()

        }, engineSession = engineSession1)



        manager.add(Session("https://www.firefox.com").apply {

            thumbnail = mock()

        }, engineSession = engineSession2)



        manager.add(Session("https://getpocket.com").apply {

            thumbnail = mock()

        }, engineSessionState = engineSessionState)



        // SessionManager:



        manager.sessions[0].apply {

            assertNotNull(engineSessionHolder.engineSession)

            assertNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[1].apply {

            assertNotNull(engineSessionHolder.engineSession)

            assertNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[2].apply {

            assertNull(engineSessionHolder.engineSession)

            assertNotNull(engineSessionHolder.engineSessionState)

        }



        // BrowserStore:



        store.state.tabs[0].apply {

            assertNotNull(engineState.engineSession)

            assertNull(engineState.engineSessionState)

        }



        store.state.tabs[1].apply {

            assertNotNull(engineState.engineSession)

            assertNull(engineState.engineSessionState)

        }



        store.state.tabs[2].apply {

            assertNull(engineState.engineSession)

            assertNotNull(engineState.engineSessionState)

        }



        // Now trim memory



        manager.onTrimMemory(ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL)



        // SessionManager:



        manager.sessions[0].apply {

            assertNotNull(engineSessionHolder.engineSession)

            assertNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[1].apply {

            assertNull(engineSessionHolder.engineSession)

            assertNotNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[2].apply {

            assertNull(engineSessionHolder.engineSession)

            assertNotNull(engineSessionHolder.engineSessionState)

        }



        // BrowserStore:



        store.state.tabs[0].apply {

            assertNotNull(engineState.engineSession)

            assertNull(engineState.engineSessionState)

        }



        store.state.tabs[1].apply {

            assertNull(engineState.engineSession)

            assertNotNull(engineState.engineSessionState)

        }



        store.state.tabs[2].apply {

            assertNull(engineState.engineSession)

            assertNotNull(engineState.engineSessionState)

        }



        // Now trim memory AGAIN



        manager.onTrimMemory(ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL)



        // SessionManager:



        manager.sessions[0].apply {

            assertNotNull(engineSessionHolder.engineSession)

            assertNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[1].apply {

            assertNull(engineSessionHolder.engineSession)

            assertNotNull(engineSessionHolder.engineSessionState)

        }



        manager.sessions[2].apply {

            assertNull(engineSessionHolder.engineSession)

            assertNotNull(engineSessionHolder.engineSessionState)

        }



        // BrowserStore:



        store.state.tabs[0].apply {

            assertNotNull(engineState.engineSession)

            assertNull(engineState.engineSessionState)

        }



        store.state.tabs[1].apply {

            assertNull(engineState.engineSession)

            assertNotNull(engineState.engineSessionState)

        }



        store.state.tabs[2].apply {

            assertNull(engineState.engineSession)

            assertNotNull(engineState.engineSessionState)

        }

    }



    @Test

    fun `Trimming memory - RUNNING_MODERATE - Does not affect SessionManager`() {

        val store = BrowserStore()

        val manager = SessionManager(engine = mock(), store = store)