Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Issue #7867: Move EngineSession from SessionManager to BrowserState.
Browse files Browse the repository at this point in the history 
Co-authored-by: Christian Sadilek <[email protected]>
  • Loading branch information
@pocmo @csadilek
pocmo and csadilek committed Aug 14, 2020
1 parent dc7d61d commit 2178f70
Show file tree
Hide file tree
Showing 66 changed files with 3,660 additions and 1,985 deletions.
7 changes: 7 additions & 0 deletions components/browser/session/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand All @@ -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'
Expand Down
127 changes: 1 addition & 126 deletions .../browser/session/src/main/java/mozilla/components/browser/session/LegacySessionManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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.
*
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -241,8 +174,6 @@ class LegacySessionManager(
snapshot.sessions.asReversed().forEach {
addInternal(
it.session,
engineSession = it.engineSession,
engineSessionState = it.engineSessionState,
parent = null,
viaRestore = true
)
Expand All @@ -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.
*/
Expand All @@ -324,8 +209,6 @@ class LegacySessionManager(

values.removeAt(indexToRemove)

unlink(session)

recalculateSelectionIndex(
indexToRemove,
selectParentIfExists,
Expand All @@ -339,7 +222,6 @@ class LegacySessionManager(
notifyObservers { onSessionRemoved(session) }

if (selectedBeforeRemove != selectedSession && selectedIndex != NO_SELECTION) {
getEngineSession(selectedSessionOrThrow)?.markActiveForWebExtensions(true)
notifyObservers { onSessionSelected(selectedSessionOrThrow) }
}
}
Expand Down Expand Up @@ -472,7 +354,6 @@ class LegacySessionManager(
*/
fun removeSessions() = synchronized(values) {
sessions.forEach {
unlink(it)
values.remove(it)
}

Expand All @@ -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
Expand All @@ -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) }
}

Expand Down
47 changes: 4 additions & 43 deletions components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
)
}
}

Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*/
Expand Down
Loading

3 comments on commit 2178f70

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to get your artifact.

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to get your artifact.

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to get your artifact.

Please sign in to comment.