Closes #3566: Remove "default session" behavior from SessionManager. (d4678eea) · Commits · The Tor Project / Applications / android-components

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

+0 −19

Original line number	Diff line number	Diff line
		@@ -20,7 +20,6 @@ import kotlin.math.min
		@Suppress("TooManyFunctions", "LargeClass")
		class LegacySessionManager(
		val engine: Engine,
		val defaultSession: (() -> Session)? = null,
		delegate: Observable<SessionManager.Observer> = ObserverRegistry()
		) : Observable<SessionManager.Observer> by delegate {
		private val values = mutableListOf<Session>()
		@@ -307,13 +306,6 @@ class LegacySessionManager(

		notifyObservers { onSessionRemoved(session) }

		// NB: we're not explicitly calling notifyObservers here, since adding a session when none
		// are present will notify observers with onSessionSelected.
		if (selectedIndex == NO_SELECTION && defaultSession != null) {
		add(defaultSession.invoke())
		return
		}

		if (selectionUpdated && selectedIndex != NO_SELECTION) {
		notifyObservers { onSessionSelected(selectedSessionOrThrow) }
		}
		@@ -463,18 +455,12 @@ class LegacySessionManager(
		// NB: This callback indicates to observers that either removeSessions or removeAll were
		// invoked, not that the manager is now empty.
		notifyObservers { onAllSessionsRemoved() }

		if (defaultSession != null) {
		add(defaultSession.invoke())
		}
		}

		/**
		* Removes all sessions including CustomTab sessions.
		*/
		fun removeAll() = synchronized(values) {
		val onlyCustomTabSessionsPresent = values.isNotEmpty() && sessions.isEmpty()

		values.forEach { unlink(it) }
		values.clear()

		@@ -483,11 +469,6 @@ class LegacySessionManager(
		// NB: This callback indicates to observers that either removeSessions or removeAll were
		// invoked, not that the manager is now empty.
		notifyObservers { onAllSessionsRemoved() }

		// Don't create a default session if we only had CustomTab sessions to begin with.
		if (!onlyCustomTabSessionsPresent && defaultSession != null) {
		add(defaultSession.invoke())
		}
		}

		/**

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

+1 −3

Original line number	Diff line number	Diff line
		@@ -4,7 +4,6 @@

		package mozilla.components.browser.session

		import androidx.lifecycle.Transformations.map
		import mozilla.components.browser.session.ext.syncDispatch
		import mozilla.components.browser.session.ext.toCustomTabSessionState
		import mozilla.components.browser.session.ext.toTabSessionState
		@@ -22,9 +21,8 @@ import mozilla.components.support.base.observer.Observable
		@Suppress("TooManyFunctions")
		class SessionManager(
		val engine: Engine,
		val defaultSession: (() -> Session)? = null,
		private val store: BrowserStore? = null,
		private val delegate: LegacySessionManager = LegacySessionManager(engine, defaultSession)
		private val delegate: LegacySessionManager = LegacySessionManager(engine)
		) : Observable<SessionManager.Observer> by delegate {
		/**
		* Returns the number of session including CustomTab sessions.

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

+0 −133

Original line number	Diff line number	Diff line
		@@ -29,37 +29,6 @@ import org.mockito.Mockito.verify
		import org.mockito.Mockito.verifyNoMoreInteractions

		class SessionManagerTest {
		@Test
		fun `default session can be specified`() {
		val manager = SessionManager(mock(), defaultSession = { Session("http://www.mozilla.org") })
		assertEquals(0, manager.size)
		}

		@Test
		fun `default session is used when manager becomes empty`() {
		val session0 = Session("about:blank")
		val session1 = Session("http://www.firefox.com")

		val manager = SessionManager(mock(), defaultSession = { session0 })

		manager.add(session1)
		assertEquals(1, manager.size)
		assertEquals("http://www.firefox.com", manager.selectedSessionOrThrow.url)

		manager.remove(session1)
		assertEquals(1, manager.size)
		assertEquals("about:blank", manager.selectedSessionOrThrow.url)

		manager.add(session1)
		manager.removeAll()
		assertEquals(1, manager.size)
		assertEquals("about:blank", manager.selectedSessionOrThrow.url)

		manager.removeSessions()
		assertEquals(1, manager.size)
		assertEquals("about:blank", manager.selectedSessionOrThrow.url)
		}

		@Test
		fun `session can be added`() {
		val manager = SessionManager(mock())
		@@ -177,74 +146,6 @@ class SessionManagerTest {
		verifyNoMoreInteractions(observer)
		}

		@Test
		fun `observer is called when all sessions removed and default session present`() {
		val session0 = Session("https://www.mozilla.org")
		val manager = SessionManager(mock(), defaultSession = { session0 })
		val observer: SessionManager.Observer = mock()

		manager.register(observer)

		manager.removeAll()

		assertEquals(1, manager.size)
		verify(observer).onSessionAdded(session0)
		verify(observer).onSessionSelected(session0)
		verify(observer).onAllSessionsRemoved()

		val observer2: SessionManager.Observer = mock()

		manager.register(observer2)

		manager.removeSessions()

		assertEquals(1, manager.size)
		verify(observer2).onSessionAdded(session0)
		verify(observer2).onSessionSelected(session0)
		verify(observer2).onAllSessionsRemoved()
		}

		@Test
		fun `default session not used when all sessions were removed and they were all CustomTab`() {
		val session1 = Session("https://www.mozilla.org")
		session1.customTabConfig = Mockito.mock(CustomTabConfig::class.java)

		val manager = SessionManager(mock(), defaultSession = { Session("about:blank") })
		val observer: SessionManager.Observer = mock()

		manager.add(session1)
		manager.register(observer)

		manager.removeAll()

		assertEquals(0, manager.size)
		verify(observer).onAllSessionsRemoved()
		}

		@Test
		fun `default session is used when all sessions were removed and they were mixed CustomTab and regular`() {
		val session1 = Session("https://www.mozilla.org")
		session1.customTabConfig = Mockito.mock(CustomTabConfig::class.java)
		val session2 = Session("https://www.firefox.com")

		val session0 = Session("about:blank")
		val manager = SessionManager(mock(), defaultSession = { session0 })
		val observer: SessionManager.Observer = mock()

		manager.register(observer)

		manager.add(session1)
		manager.add(session2)
		manager.removeAll()

		assertEquals(1, manager.size)
		assertEquals("about:blank", manager.selectedSessionOrThrow.url)

		verify(observer).onAllSessionsRemoved()
		verify(observer).onSessionSelected(session0)
		verify(observer).onSessionAdded(session0)
		}

		@Test
		fun `observer is called when session is removed`() {
		val manager = SessionManager(mock())
		@@ -1102,40 +1003,6 @@ class SessionManagerTest {
		assertNull(manager.selectedSession)
		}

		@Test
		fun `SessionManager will select default if regular session is removed an no other regular session is left`() {
		val defaultSession = Session("http://www.mozilla.com")
		val manager = SessionManager(mock(), defaultSession = { defaultSession })
		assertNull(manager.selectedSession)

		val regular1 = Session("https://www.getpocket.com", private = false)
		manager.add(regular1)

		val private1 = Session("https://example.org/private1", private = true)
		manager.add(private1)

		val private2 = Session("https://example.org/private2", private = true)
		manager.add(private2)

		val regular2 = Session("https://www.firefox.com", private = false)
		manager.add(regular2)

		val regular3 = Session("https://www.firefox.org", private = false)
		manager.add(regular3, selected = true)

		manager.remove(regular3)
		assertEquals(regular2, manager.selectedSession)

		manager.remove(regular2)
		assertEquals(regular1, manager.selectedSession)

		// Removing the last regular session should NOT cause a private session to be selected,
		// but a default session was provided, so it should be selected now.
		manager.remove(regular1)
		assertEquals(manager.defaultSession?.invoke(), manager.selectedSession)
		assertEquals("http://www.mozilla.com", manager.selectedSession?.url)
		}

		@Test
		fun `SessionManager#runWithSession executes the block when session found`() {
		val sessionManager = spy(SessionManager(mock()))

components/browser/session/src/test/java/mozilla/components/browser/session/storage/SessionStorageTest.kt

+1 −1

Original line number	Diff line number	Diff line
		@@ -261,7 +261,7 @@ class SessionStorageTest {
		@Test
		fun `AutoSave - when all sessions get removed`() {
		runBlocking {
		val sessionManager = SessionManager(mock(), { Session("https://getpocket.com") })
		val sessionManager = SessionManager(mock())
		sessionManager.add(Session("https://www.firefox.com"))
		sessionManager.add(Session("https://www.mozilla.org"))

docs/changelog.md

+3 −0

Original line number	Diff line number	Diff line
		@@ -21,6 +21,9 @@ permalink: /changelog/
		* concept-engine, browser-engine-gecko(-beta/nightly), browser-engine-system
		* Added `EngineView.release()` to manually release an `EngineSession` that is currently being rendered by the `EngineView`. Usually an app does not need to call `release()` manually since `EngineView` takes care of releasing the `EngineSession` on specific lifecycle events. However sometimes the app wants to release an `EngineSession` to immediately render it on another `EngineView`; e.g. when transforming a Custom Tab into a regular browser tab.

		* browser-session
		* ⚠️ This is a breaking change: Removed "default session" behavior from `SessionManager`. This feature was never used by any app except the sample browser.

		# 2.0.0

		* [Commits](https://github.com/mozilla-mobile/android-components/compare/v1.0.0...v2.0.0)