Commit 38186676 authored by MozLando's avatar MozLando
Browse files

Merge #8121

8121: Issue #7867: Move EngineSession from SessionManager to BrowserState. r=csadilek a=pocmo

Fenix PR: https://github.com/mozilla-mobile/fenix/pull/13933
R-B PR: https://github.com/mozilla-mobile/reference-browser/pull/1292

Co-authored-by: default avatarSebastian Kaspari <s.kaspari@gmail.com>
parents e47c5f09 9e60a621
......@@ -232,7 +232,15 @@ class GeckoEngineSession(
policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
geckoSession.settings.useTrackingProtection = shouldBlockContent && enabled
notifyAtLeastOneObserver { onTrackerBlockingEnabledChange(enabled) }
notifyAtLeastOneObserver {
// We now register engine observers in a middleware using a dedicated
// store thread. Since this notification can be delayed until an observer
// is registered we switch to the main scope to make sure we're not notifying
// on the store thread.
MainScope().launch {
onTrackerBlockingEnabledChange(enabled)
}
}
}
/**
......
......@@ -232,7 +232,15 @@ class GeckoEngineSession(
policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
geckoSession.settings.useTrackingProtection = shouldBlockContent && enabled
notifyAtLeastOneObserver { onTrackerBlockingEnabledChange(enabled) }
notifyAtLeastOneObserver {
// We now register engine observers in a middleware using a dedicated
// store thread. Since this notification can be delayed until an observer
// is registered we switch to the main scope to make sure we're not notifying
// on the store thread.
MainScope().launch {
onTrackerBlockingEnabledChange(enabled)
}
}
}
/**
......
......@@ -232,7 +232,15 @@ class GeckoEngineSession(
policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
geckoSession.settings.useTrackingProtection = shouldBlockContent && enabled
notifyAtLeastOneObserver { onTrackerBlockingEnabledChange(enabled) }
notifyAtLeastOneObserver {
// We now register engine observers in a middleware using a dedicated
// store thread. Since this notification can be delayed until an observer
// is registered we switch to the main scope to make sure we're not notifying
// on the store thread.
MainScope().launch {
onTrackerBlockingEnabledChange(enabled)
}
}
}
/**
......
......@@ -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'
......
......@@ -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) }
}
......
......@@ -6,14 +6,11 @@ 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
......@@ -21,13 +18,10 @@ import mozilla.components.browser.state.action.ContentAction.UpdateLoadingStateA
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.
*/
......
......@@ -4,17 +4,13 @@
package mozilla.components.browser.session
import android.content.ComponentCallbacks2
import mozilla.components.browser.session.engine.EngineObserver
import mozilla.components.browser.session.ext.syncDispatch
import mozilla.components.browser.session.ext.toCustomTabSessionState
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.EngineAction.UnlinkEngineSessionAction
import mozilla.components.browser.state.action.ReaderAction
import mozilla.components.browser.state.action.SystemAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.ReaderState
......@@ -22,11 +18,7 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineSessionState
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.base.memory.MemoryConsumer
import mozilla.components.support.base.observer.Observable
import mozilla.components.support.ktx.kotlin.isExtensionUrl
import java.lang.IllegalArgumentException
/**
* This class provides access to a centralized registry of all active sessions.
......@@ -35,117 +27,25 @@ import java.lang.IllegalArgumentException
class SessionManager(
val engine: Engine,
private val store: BrowserStore? = null,
private val linker: EngineSessionLinker = EngineSessionLinker(store),
private val delegate: LegacySessionManager = LegacySessionManager(engine, linker)
) : Observable<SessionManager.Observer> by delegate, MemoryConsumer {
private val logger = Logger("SessionManager")
/**
* This class only exists for migrating from browser-session
* to browser-state. We need a way to dispatch the corresponding browser
* actions when an engine session is linked and unlinked.
*/
class EngineSessionLinker(private val store: BrowserStore?) {
/**
* Links the provided [Session] and [EngineSession].
*/
fun link(
session: Session,
engineSession: EngineSession,
parentEngineSession: EngineSession?,
sessionRestored: Boolean = false,
skipLoading: Boolean = false
) {
unlink(session)
session.engineSessionHolder.apply {
this.engineSession = engineSession
this.engineObserver = EngineObserver(session, store).also { observer ->
engineSession.register(observer)
if (!sessionRestored && !skipLoading) {
if (session.url.isExtensionUrl()) {
// The parent tab/session is used as a referrer which is not accurate
// for extension pages. The extension page is not loaded by the parent
// tab, but opened by an extension e.g. via browser.tabs.update.
engineSession.loadUrl(session.url)
} else {
engineSession.loadUrl(session.url, parentEngineSession)
}
}
}
}
store?.syncDispatch(LinkEngineSessionAction(session.id, engineSession))
}
fun unlink(session: Session) {
val observer = session.engineSessionHolder.engineObserver
val engineSession = session.engineSessionHolder.engineSession
if (observer != null && engineSession != null) {
engineSession.unregister(observer)
}
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))
// Now, that neither the session manager nor the store keep a reference to the engine
// session, we can close it.
engineSession?.close()
}
}
private val delegate: LegacySessionManager = LegacySessionManager(engine)
) : Observable<SessionManager.Observer> by delegate {
/**
* Returns the number of session including CustomTab sessions.
*/
val size: Int
get() = delegate.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 [Snapshot] of the current session state.
*/
fun createSnapshot(): Snapshot {
val snapshot = delegate.createSnapshot()
// The reader state is no longer part of the browser session so we copy
// it from the store if it exists. This can be removed once browser-state
// migration is complete and the session storage uses the store instead
// of the session manager.
return snapshot.copy(
sessions = snapshot.sessions.map { item ->
store?.state?.findTab(item.session.id)?.let {
item.copy(readerState = it.readerState)
} ?: item
}
)
}
/**
* Produces a [Snapshot.Item] of a single [Session], suitable for restoring via [SessionManager.restore].
*/
fun createSessionSnapshot(session: Session): Snapshot.Item {
val item = delegate.createSessionSnapshot(session)
// The reader state is no longer part of the browser session so we copy
// it from the store if it exists. This can be removed once browser-state
// migration is complete and the session storage uses the store instead
// of the session manager.
return store?.state?.findTab(item.session.id)?.let {
item.copy(readerState = it.readerState)
} ?: item
val tab = store?.state?.findTab(session.id)
return Snapshot.Item(
session,
tab?.engineState?.engineSession,
tab?.engineState?.engineSessionState,
tab?.readerState)
}
/**
......@@ -216,6 +116,16 @@ class SessionManager(
)
}
delegate.add(session, selected, parent)
if (engineSession != null) {
store?.syncDispatch(LinkEngineSessionAction(
session.id,
engineSession,
skipLoading = true
))
}
if (engineSessionState != null && engineSession == null) {
// If the caller passed us an engine session state then also notify the store. We only
// do this if there is no engine session, because this mirrors the behavior in
......@@ -225,8 +135,6 @@ class SessionManager(
engineSessionState
))