Issue #6884: SessionManager.onTrimMemory(): Do not remove engine session state from sessions. (e6abb76c) · Commits · The Tor Project / Applications / android-components

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

+13 −5

Original line number	Diff line number	Diff line
		@@ -80,6 +80,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))
		@@ -408,18 +415,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
		@@ -1079,7 +1079,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)

		@@ -1221,7 +1221,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)