Issue #7867: Move EngineSession from SessionManager to BrowserState.

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

components/browser/session/build.gradle

@@ -21,6 +21,12 @@ android {
     }
 }
 
+tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach() {
+    kotlinOptions.freeCompilerArgs += [
+            "-Xuse-experimental=kotlinx.coroutines.ExperimentalCoroutinesApi"
+    ]
+}
+
 dependencies {
     api project(':browser-state')
 
@@ -47,6 +53,7 @@ dependencies {
     testImplementation Dependencies.androidx_test_junit
     testImplementation Dependencies.testing_robolectric
     testImplementation Dependencies.testing_mockito
+    testImplementation Dependencies.testing_coroutines
 }
 
 apply from: '../../../publish.gradle'

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

@@ -6,8 +6,6 @@ package mozilla.components.browser.session
 
 import androidx.annotation.GuardedBy
 import mozilla.components.concept.engine.Engine
-import mozilla.components.concept.engine.EngineSession
-import mozilla.components.concept.engine.EngineSessionState
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
 import kotlin.math.max
@@ -19,7 +17,6 @@ import kotlin.math.min
 @Suppress("TooManyFunctions", "LargeClass")
 class LegacySessionManager(
     val engine: Engine,
-    private val engineSessionLinker: SessionManager.EngineSessionLinker,
     delegate: Observable<SessionManager.Observer> = ObserverRegistry()
 ) : Observable<SessionManager.Observer> by delegate {
     // It's important that any access to `values` is synchronized;
@@ -35,60 +32,6 @@ class LegacySessionManager(
     val size: Int
         get() = synchronized(values) { values.size }
 
-    /**
-     * Produces a snapshot of this manager's state, suitable for restoring via [SessionManager.restore].
-     * Only regular sessions are included in the snapshot. Private and Custom Tab sessions are omitted.
-     *
-     * @return [SessionManager.Snapshot] of the current session state.
-     */
-    fun createSnapshot(): SessionManager.Snapshot = synchronized(values) {
-        if (values.isEmpty()) {
-            return SessionManager.Snapshot.empty()
-        }
-
-        // Filter out CustomTab and private sessions.
-        // We're using 'values' directly instead of 'sessions' to get benefits of a sequence.
-        val sessionStateTuples = values.asSequence()
-                .filter { !it.isCustomTabSession() }
-                .filter { !it.private }
-                .map { session -> createSessionSnapshot(session) }
-                .toList()
-
-        // We might have some sessions (private, custom tab) but none we'd include in the snapshot.
-        if (sessionStateTuples.isEmpty()) {
-            return SessionManager.Snapshot.empty()
-        }
-
-        // We need to find out the index of our selected session in the filtered list. If we have a
-        // mix of private, custom tab and regular sessions, global selectedIndex isn't good enough.
-        // We must have a selectedSession if there is at least one "regular" (non-CustomTabs) session
-        // present. Selected session might be private, in which case we reset our selection index to 0.
-        var selectedIndexAfterFiltering = 0
-        selectedSession?.takeIf { !it.private }?.let { selected ->
-            sessionStateTuples.find { it.session.id == selected.id }?.let { selectedTuple ->
-                selectedIndexAfterFiltering = sessionStateTuples.indexOf(selectedTuple)
-            }
-        }
-
-        // Sanity check to guard against producing invalid snapshots.
-        checkNotNull(sessionStateTuples.getOrNull(selectedIndexAfterFiltering)) {
-            "Selection index after filtering session must be valid"
-        }
-
-        SessionManager.Snapshot(
-            sessions = sessionStateTuples,
-            selectedSessionIndex = selectedIndexAfterFiltering
-        )
-    }
-
-    fun createSessionSnapshot(session: Session): SessionManager.Snapshot.Item {
-        return SessionManager.Snapshot.Item(
-            session,
-            session.engineSessionHolder.engineSession,
-            session.engineSessionHolder.engineSessionState
-        )
-    }
-
     /**
      * Gets the currently selected session if there is one.
      *
@@ -136,19 +79,15 @@ class LegacySessionManager(
     fun add(
         session: Session,
         selected: Boolean = false,
-        engineSession: EngineSession? = null,
-        engineSessionState: EngineSessionState? = null,
         parent: Session? = null
     ) = synchronized(values) {
-        addInternal(session, selected, engineSession, engineSessionState, parent = parent, viaRestore = false)
+        addInternal(session, selected, parent = parent, viaRestore = false)
     }
 
     @Suppress("LongParameterList", "ComplexMethod")
     private fun addInternal(
         session: Session,
         selected: Boolean = false,
-        engineSession: EngineSession? = null,
-        engineSessionState: EngineSessionState? = null,
         parent: Session? = null,
         viaRestore: Boolean = false,
         notifyObservers: Boolean = true
@@ -173,12 +112,6 @@ class LegacySessionManager(
             }
         }
 
-        if (engineSession != null) {
-            link(session, engineSession)
-        } else if (engineSessionState != null) {
-            session.engineSessionHolder.engineSessionState = engineSessionState
-        }
-
         // If session is being added via restore, skip notification and auto-selection.
         // Restore will handle these actions as appropriate.
         if (viaRestore || !notifyObservers) {
@@ -241,8 +174,6 @@ class LegacySessionManager(
         snapshot.sessions.asReversed().forEach {
             addInternal(
                 it.session,
-                engineSession = it.engineSession,
-                engineSessionState = it.engineSessionState,
                 parent = null,
                 viaRestore = true
             )
@@ -262,52 +193,6 @@ class LegacySessionManager(
         notifyObservers { onSessionsRestored() }
     }
 
-    /**
-     * Gets the linked engine session for the provided session (if it exists).
-     */
-    fun getEngineSession(session: Session = selectedSessionOrThrow) = session.engineSessionHolder.engineSession
-
-    /**
-     * Gets the linked engine session for the provided session and creates it if needed.
-     */
-    fun getOrCreateEngineSession(
-        session: Session = selectedSessionOrThrow,
-        skipLoading: Boolean = false
-    ): EngineSession {
-        getEngineSession(session)?.let { return it }
-
-        return engine.createSession(session.private, session.contextId).apply {
-            var restored = false
-            session.engineSessionHolder.engineSessionState?.let { state ->
-                restored = restoreState(state)
-                session.engineSessionHolder.engineSessionState = null
-            }
-
-            link(session, this, restored, skipLoading)
-        }
-    }
-
-    private fun link(
-        session: Session,
-        engineSession: EngineSession,
-        restored: Boolean = false,
-        skipLoading: Boolean = false
-    ) {
-        val parent = values.find { it.id == session.parentId }?.let {
-            this.getEngineSession(it)
-        }
-        engineSessionLinker.link(session, engineSession, parent, restored, skipLoading)
-
-        if (session == selectedSession) {
-            engineSession.markActiveForWebExtensions(true)
-        }
-    }
-
-    private fun unlink(session: Session) {
-        getEngineSession(session)?.markActiveForWebExtensions(false)
-        engineSessionLinker.unlink(session)
-    }
-
     /**
      * Removes the provided session. If no session is provided then the selected session is removed.
      */
@@ -324,8 +209,6 @@ class LegacySessionManager(
 
         values.removeAt(indexToRemove)
 
-        unlink(session)
-
         recalculateSelectionIndex(
             indexToRemove,
             selectParentIfExists,
@@ -339,7 +222,6 @@ class LegacySessionManager(
         notifyObservers { onSessionRemoved(session) }
 
         if (selectedBeforeRemove != selectedSession && selectedIndex != NO_SELECTION) {
-            getEngineSession(selectedSessionOrThrow)?.markActiveForWebExtensions(true)
             notifyObservers { onSessionSelected(selectedSessionOrThrow) }
         }
     }
@@ -472,7 +354,6 @@ class LegacySessionManager(
      */
     fun removeSessions() = synchronized(values) {
         sessions.forEach {
-            unlink(it)
             values.remove(it)
         }
 
@@ -487,7 +368,6 @@ class LegacySessionManager(
      * Removes all sessions including CustomTab sessions.
      */
     fun removeAll() = synchronized(values) {
-        values.forEach { unlink(it) }
         values.clear()
 
         selectedIndex = NO_SELECTION
@@ -507,13 +387,8 @@ class LegacySessionManager(
             "Value to select is not in list"
         }
 
-        selectedSession?.let {
-            getEngineSession(it)?.markActiveForWebExtensions(false)
-        }
-
         selectedIndex = index
 
-        getEngineSession(session)?.markActiveForWebExtensions(true)
         notifyObservers { onSessionSelected(session) }
     }
 

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

@@ -6,28 +6,22 @@ package mozilla.components.browser.session
 
 import android.content.Intent
 import android.graphics.Bitmap
-import mozilla.components.browser.session.engine.EngineSessionHolder
 import mozilla.components.browser.session.engine.request.LaunchIntentMetadata
 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.RemoveThumbnailAction
 import mozilla.components.browser.state.action.ContentAction.RemoveWebAppManifestAction
 import mozilla.components.browser.state.action.ContentAction.UpdateBackNavigationStateAction
 import mozilla.components.browser.state.action.ContentAction.UpdateForwardNavigationStateAction
 import mozilla.components.browser.state.action.ContentAction.UpdateLoadingStateAction
 import mozilla.components.browser.state.action.ContentAction.UpdateProgressAction
 import mozilla.components.browser.state.action.ContentAction.UpdateSearchTermsAction
 import mozilla.components.browser.state.action.ContentAction.UpdateSecurityInfoAction
-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.ContentAction.UpdateWebAppManifestAction
-import mozilla.components.browser.state.action.CustomTabListAction.RemoveCustomTabAction
-import mozilla.components.browser.state.action.EngineAction
-import mozilla.components.browser.state.action.TabListAction.AddTabAction
+import mozilla.components.browser.state.action.CustomTabListAction
 import mozilla.components.browser.state.action.TrackingProtectionAction
 import mozilla.components.browser.state.selector.findTabOrCustomTab
 import mozilla.components.browser.state.state.CustomTabConfig
@@ -55,12 +49,6 @@ class Session(
     val contextId: String? = null,
     delegate: Observable<Observer> = ObserverRegistry()
 ) : Observable<Session.Observer> by delegate {
-    /**
-     * Holder for keeping a reference to an engine session and its observer to update this session
-     * object.
-     */
-    internal val engineSessionHolder = EngineSessionHolder()
-
     // For migration purposes every `Session` has a reference to the `BrowserStore` (if used) in order to dispatch
     // actions to it when the `Session` changes.
     internal var store: BrowserStore? = null
@@ -94,10 +82,8 @@ class Session(
         fun onTrackerBlocked(session: Session, tracker: Tracker, all: List<Tracker>) = Unit
         fun onTrackerLoaded(session: Session, tracker: Tracker, all: List<Tracker>) = Unit
         fun onDesktopModeChanged(session: Session, enabled: Boolean) = Unit
-        fun onThumbnailChanged(session: Session, bitmap: Bitmap?) = Unit
         fun onContentPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
         fun onAppPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
-        fun onCrashStateChanged(session: Session, crashed: Boolean) = Unit
         fun onRecordingDevicesChanged(session: Session, devices: List<RecordingDevice>) = Unit
         fun onLaunchIntentRequest(session: Session, url: String, appIntent: Intent?) = Unit
     }
@@ -218,11 +204,9 @@ class Session(
         // 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()))
-            engineSessionHolder.engineSession?.let { engineSession ->
-                store?.syncDispatch(EngineAction.LinkEngineSessionAction(id, engineSession))
-            }
+            store?.syncDispatch(
+                CustomTabListAction.TurnCustomTabIntoNormalTabAction(id)
+            )
         }
     }
 
@@ -293,16 +277,6 @@ class Session(
         }
     }
 
-    /**
-     * The target of the latest thumbnail.
-     */
-    var thumbnail: Bitmap? by Delegates.observable<Bitmap?>(null) { _, _, new ->
-        notifyObservers { onThumbnailChanged(this@Session, new) }
-
-        val action = if (new != null) UpdateThumbnailAction(id, new) else RemoveThumbnailAction(id)
-        store?.syncDispatch(action)
-    }
-
     /**
      * Desktop Mode state, true if the desktop mode is requested, otherwise false.
      */
@@ -341,19 +315,6 @@ class Session(
             !request.consumeBy(consumers)
     }
 
-    /**
-     * Whether this [Session] has crashed.
-     *
-     * In conjunction with a `concept-engine` implementation that uses a multi-process architecture, single sessions
-     * can crash without crashing the whole app.
-     *
-     * A crashed session may still be operational (since the underlying engine implementation has recovered its content
-     * process), but further action may be needed to restore the last state before the session has crashed (if desired).
-     */
-    var crashed: Boolean by Delegates.observable(false) { _, old, new ->
-        notifyObservers(old, new) { onCrashStateChanged(this@Session, new) }
-    }
-
     /**
      * List of recording devices (e.g. camera or microphone) currently in use by web content.
      */