Skip to content

Commit

Permalink
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
Browse files Browse the repository at this point in the history 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
  • Loading branch information
@pocmo
pocmo committed Sep 2, 2020
1 parent cef0bfc commit 35fe396
Show file tree
Hide file tree
Showing 29 changed files with 222 additions and 909 deletions.
42 changes: 3 additions & 39 deletions ...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ class GeckoEngineSession(
internal var scrollY: Int = 0

internal var job: Job = Job()
private var lastSessionState: GeckoSession.SessionState? = null
private var stateBeforeCrash: GeckoSession.SessionState? = null
private var canGoBack: Boolean = false

/**
Expand Down Expand Up @@ -189,13 +187,6 @@ class GeckoEngineSession(
geckoSession.gotoHistoryIndex(index)
}

/**
* See [EngineSession.saveState]
*/
override fun saveState(): EngineSessionState {
return GeckoEngineSessionState(lastSessionState)
}

/**
* See [EngineSession.restoreState]
*/
Expand Down Expand Up @@ -371,22 +362,6 @@ class GeckoEngineSession(
geckoSession.exitFullScreen()
}

/**
* See [EngineSession.recoverFromCrash]
*/
@Synchronized
override fun recoverFromCrash(): Boolean {
val state = stateBeforeCrash

return if (state != null) {
geckoSession.restoreState(state)
stateBeforeCrash = null
true
} else {
false
}
}

/**
* See [EngineSession.markActiveForWebExtensions].
*/
Expand Down Expand Up @@ -601,7 +576,9 @@ class GeckoEngineSession(
}

override fun onSessionStateChange(session: GeckoSession, sessionState: GeckoSession.SessionState) {
lastSessionState = sessionState
notifyObservers {
onState(GeckoEngineSessionState(sessionState))
}
}
}

Expand Down Expand Up @@ -726,23 +703,10 @@ class GeckoEngineSession(
}

override fun onCrash(session: GeckoSession) {
stateBeforeCrash = lastSessionState

recoverGeckoSession()

notifyObservers { onCrash() }
}

override fun onKill(session: GeckoSession) {
// The content process of this session got killed (resources reclaimed by Android).
// Let's recover and restore the last known state.

val state = lastSessionState

recoverGeckoSession()

state?.let { geckoSession.restoreState(it) }

notifyObservers { onProcessKilled() }
}

Expand Down
90 changes: 18 additions & 72 deletions ...ngine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ package mozilla.components.browser.engine.gecko
import android.content.Context
import android.graphics.Bitmap
import android.util.AttributeSet
import android.view.View
import android.widget.FrameLayout
import androidx.annotation.VisibleForTesting
import androidx.core.view.ViewCompat
import mozilla.components.browser.engine.gecko.selection.GeckoSelectionActionDelegate
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineView
import mozilla.components.concept.engine.permission.PermissionRequest
import mozilla.components.concept.engine.selection.SelectionActionDelegate
import org.mozilla.geckoview.BasicSelectionActionDelegate
import org.mozilla.geckoview.GeckoResult
Expand All @@ -30,7 +28,7 @@ class GeckoEngineView @JvmOverloads constructor(
defStyleAttr: Int = 0
) : FrameLayout(context, attrs, defStyleAttr), EngineView {
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal var currentGeckoView = object : NestedGeckoView(context) {
internal var geckoView = object : NestedGeckoView(context) {

override fun onAttachedToWindow() {
try {
Expand All @@ -40,12 +38,12 @@ class GeckoEngineView @JvmOverloads constructor(
val otherActivityClassName =
this.session?.accessibility?.view?.context?.javaClass?.simpleName
val otherActivityClassHashcode =
this.session?.accessibility?.view?.context?.hashCode()
this.session?.accessibility?.view?.context?.hashCode()
val activityClassName = context.javaClass.simpleName
val activityClassHashCode = context.hashCode()
val msg = "ATTACH VIEW: Current activity: $activityClassName hashcode " +
"$activityClassHashCode Other activity: $otherActivityClassName " +
"hashcode $otherActivityClassHashcode"
"$activityClassHashCode Other activity: $otherActivityClassName " +
"hashcode $otherActivityClassHashcode"
throw IllegalStateException(msg, e)
}
}
Expand All @@ -63,24 +61,6 @@ class GeckoEngineView @JvmOverloads constructor(
ViewCompat.setImportantForAutofill(this, ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_YES)
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal val observer = object : EngineSession.Observer {
override fun onCrash() {
rebind()
}

override fun onProcessKilled() {
rebind()
}

override fun onFirstContentfulPaint() {
visibility = View.VISIBLE
}

override fun onAppPermissionRequest(permissionRequest: PermissionRequest) = Unit
override fun onContentPermissionRequest(permissionRequest: PermissionRequest) = Unit
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal var currentSession: GeckoEngineSession? = null

Expand All @@ -90,9 +70,7 @@ class GeckoEngineView @JvmOverloads constructor(
override var selectionActionDelegate: SelectionActionDelegate? = null

init {
// Currently this is just a FrameLayout with a single GeckoView instance. Eventually this
// implementation should handle at least two GeckoView so that we can switch between
addView(currentGeckoView)
addView(geckoView)
}

/**
Expand All @@ -101,23 +79,18 @@ class GeckoEngineView @JvmOverloads constructor(
@Synchronized
override fun render(session: EngineSession) {
val internalSession = session as GeckoEngineSession
currentSession = session

currentSession?.apply { unregister(observer) }

currentSession = session.apply {
register(observer)
}

if (currentGeckoView.session != internalSession.geckoSession) {
currentGeckoView.session?.let {
if (geckoView.session != internalSession.geckoSession) {
geckoView.session?.let {
// Release a previously assigned session. Otherwise GeckoView will close it
// automatically.
detachSelectionActionDelegate(it)
currentGeckoView.releaseSession()
geckoView.releaseSession()
}

try {
currentGeckoView.setSession(internalSession.geckoSession)
geckoView.setSession(internalSession.geckoSession)
attachSelectionActionDelegate(internalSession.geckoSession)
} catch (e: IllegalStateException) {
// This is to debug "display already acquired" crashes
Expand All @@ -128,8 +101,8 @@ class GeckoEngineView @JvmOverloads constructor(
val activityClassName = context.javaClass.simpleName
val activityClassHashCode = context.hashCode()
val msg = "SET SESSION: Current activity: $activityClassName hashcode " +
"$activityClassHashCode Other activity: $otherActivityClassName " +
"hashcode $otherActivityClassHashcode"
"$activityClassHashCode Other activity: $otherActivityClassName " +
"hashcode $otherActivityClassHashcode"
throw IllegalStateException(msg, e)
}
}
Expand All @@ -150,40 +123,13 @@ class GeckoEngineView @JvmOverloads constructor(
}
}

/**
* Rebinds the current session to this view. This may be required after the session crashed and
* the view needs to notified about a new underlying GeckoSession (created under the hood by
* GeckoEngineSession) - since the previous one is no longer usable.
*/
private fun rebind() {
try {
currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
} catch (e: IllegalArgumentException) {
// This is to debug "Display not attached" crashes
val otherActivityClassName =
currentSession?.geckoSession?.accessibility?.view?.context?.javaClass?.simpleName
val otherActivityClassHashcode =
currentSession?.geckoSession?.accessibility?.view?.context?.hashCode()
val activityClassName = context.javaClass.simpleName
val activityClassHashCode = context.hashCode()
val msg = "REBIND: Current activity: $activityClassName hashcode " +
"$activityClassHashCode Other activity: $otherActivityClassName " +
"hashcode $otherActivityClassHashcode"
throw IllegalArgumentException(msg, e)
}
}

@Synchronized
override fun release() {
detachSelectionActionDelegate(currentSession?.geckoSession)

currentSession?.apply {
unregister(observer)
}

currentSession = null

currentGeckoView.releaseSession()
geckoView.releaseSession()
}

override fun onDetachedFromWindow() {
Expand All @@ -201,21 +147,21 @@ class GeckoEngineView @JvmOverloads constructor(
override fun getInputResult(): EngineView.InputResult {
// Direct mapping of GeckoView's returned values.
// There should be a 1-1 relation. If not fail fast to allow for a quick fix.
return EngineView.InputResult.values().first { it.value == currentGeckoView.inputResult }
return EngineView.InputResult.values().first { it.value == geckoView.inputResult }
}

override fun setVerticalClipping(clippingHeight: Int) {
currentGeckoView.setVerticalClipping(clippingHeight)
geckoView.setVerticalClipping(clippingHeight)
}

override fun setDynamicToolbarMaxHeight(height: Int) {
currentGeckoView.setDynamicToolbarMaxHeight(height)
geckoView.setDynamicToolbarMaxHeight(height)
}

@Suppress("TooGenericExceptionCaught")
override fun captureThumbnail(onFinish: (Bitmap?) -> Unit) {
try {
val geckoResult = currentGeckoView.capturePixels()
val geckoResult = geckoView.capturePixels()
geckoResult.then({ bitmap ->
onFinish(bitmap)
GeckoResult<Void>()
Expand Down Expand Up @@ -243,7 +189,7 @@ class GeckoEngineView @JvmOverloads constructor(
// https://bugzilla.mozilla.org/show_bug.cgi?id=1630775
// We do this to prevent the content from resizing when the view is not visible:
// https://github.com/mozilla-mobile/android-components/issues/6664
currentGeckoView.visibility = visibility
geckoView.visibility = visibility
super.setVisibility(visibility)
}
}
Loading

0 comments on commit 35fe396

Please sign in to comment.