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

Commit

Permalink
Merge #8320
Browse files Browse the repository at this point in the history
8320: Issue #8255: GeckoEngine: Drop speculative EngineSession if it crashed or process was killed r=pocmo a=csadilek



Co-authored-by: Sebastian Kaspari <[email protected]>
  • Loading branch information
MozLando and pocmo committed Sep 2, 2020
2 parents e64abb4 + 68559df commit 3f00d96
Show file tree
Hide file tree
Showing 12 changed files with 965 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import mozilla.components.browser.engine.gecko.integration.LocaleSettingUpdater
import mozilla.components.browser.engine.gecko.mediaquery.from
import mozilla.components.browser.engine.gecko.mediaquery.toGeckoValue
import mozilla.components.browser.engine.gecko.profiler.Profiler
import mozilla.components.browser.engine.gecko.util.SpeculativeSessionFactory
import mozilla.components.browser.engine.gecko.webextension.GeckoWebExtension
import mozilla.components.browser.engine.gecko.webnotifications.GeckoWebNotificationDelegate
import mozilla.components.browser.engine.gecko.webpush.GeckoWebPushDelegate
Expand Down Expand Up @@ -69,9 +70,8 @@ class GeckoEngine(
) : Engine, WebExtensionRuntime {
private val executor by lazy { executorProvider.invoke() }
private val localeUpdater = LocaleSettingUpdater(context, runtime)
@VisibleForTesting internal var speculativeSession: GeckoEngineSession? = null
private val sharedPref = context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)

@VisibleForTesting internal val speculativeConnectionFactory = SpeculativeSessionFactory()
private var webExtensionDelegate: WebExtensionDelegate? = null
private val webExtensionActionHandler = object : ActionHandler {
override fun onBrowserAction(extension: WebExtension, session: EngineSession?, action: Action) {
Expand Down Expand Up @@ -150,17 +150,8 @@ class GeckoEngine(
*/
override fun createSession(private: Boolean, contextId: String?): EngineSession {
ThreadUtils.assertOnUiThread()
val speculativeSession = this.speculativeSession?.let { speculativeSession ->
if (speculativeSession.geckoSession.settings.usePrivateMode == private &&
speculativeSession.geckoSession.settings.contextId == contextId) {
speculativeSession
} else {
speculativeSession.close()
null
}.also {
this.speculativeSession = null
}
}

val speculativeSession = speculativeConnectionFactory.get(private, contextId)
return speculativeSession ?: GeckoEngineSession(runtime, private, defaultSettings, contextId)
}

Expand All @@ -175,20 +166,14 @@ class GeckoEngine(
* See [Engine.speculativeCreateSession].
*/
override fun speculativeCreateSession(private: Boolean, contextId: String?) {
ThreadUtils.assertOnUiThread()
if (this.speculativeSession?.geckoSession?.settings?.usePrivateMode != private ||
this.speculativeSession?.geckoSession?.settings?.contextId != contextId) {
this.speculativeSession?.geckoSession?.close()
this.speculativeSession = GeckoEngineSession(runtime, private, defaultSettings, contextId)
}
speculativeConnectionFactory.create(runtime, private, contextId, defaultSettings)
}

/**
* See [Engine.clearSpeculativeSession].
*/
override fun clearSpeculativeSession() {
this.speculativeSession?.geckoSession?.close()
this.speculativeSession = null
speculativeConnectionFactory.clear()
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package mozilla.components.browser.engine.gecko.util

import androidx.annotation.VisibleForTesting
import mozilla.components.browser.engine.gecko.GeckoEngineSession
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.Settings
import mozilla.components.support.utils.ThreadUtils
import org.mozilla.geckoview.GeckoRuntime

/**
* Helper factory for creating and maintaining a speculative [EngineSession].
*/
internal class SpeculativeSessionFactory {
@VisibleForTesting internal var speculativeEngineSession: SpeculativeEngineSession? = null

/**
* Creates a speculative [EngineSession] using the provided [contextId] and [defaultSettings].
* Creates a private session if [private] is set to true.
*
* The speculative [EngineSession] is kept internally until explicitly needed and access via [get].
*/
@Synchronized
fun create(
runtime: GeckoRuntime,
private: Boolean,
contextId: String?,
defaultSettings: Settings?
) {
ThreadUtils.assertOnUiThread()

if (speculativeEngineSession?.matches(private, contextId) == true) {
// We already have a speculative engine session for this configuration. Nothing to do here.
return
}

// Clear any potentially non-matching engine session
clear()

speculativeEngineSession = SpeculativeEngineSession.create(
this,
runtime,
private,
contextId,
defaultSettings
)
}

/**
* Clears the internal speculative [EngineSession].
*/
@Synchronized
fun clear() {
speculativeEngineSession?.cleanUp()
speculativeEngineSession = null
}

/**
* Returns a previously created ([private] speculative [EngineSession] if it uses the same
* [contextId]. Returns `null` if no speculative [EngineSession] for that configuration is
* available.
*/
@Synchronized
fun get(
private: Boolean,
contextId: String?
): GeckoEngineSession? {
val speculativeEngineSession = speculativeEngineSession ?: return null

return if (speculativeEngineSession.matches(private, contextId)) {
this.speculativeEngineSession = null
speculativeEngineSession.unwrap()
} else {
clear()
null
}
}

@VisibleForTesting
internal fun hasSpeculativeSession(): Boolean {
return speculativeEngineSession != null
}
}

/**
* Internal wrapper for [GeckoEngineSession] that takes care of registering and unregistering an
* observer for handling content process crashes/kills.
*/
internal class SpeculativeEngineSession constructor(
@VisibleForTesting internal val engineSession: GeckoEngineSession,
@VisibleForTesting internal val observer: SpeculativeSessionObserver
) {
/**
* Checks whether the [SpeculativeEngineSession] matches the given configuration.
*/
fun matches(private: Boolean, contextId: String?): Boolean {
return engineSession.geckoSession.settings.usePrivateMode == private &&
engineSession.geckoSession.settings.contextId == contextId
}

/**
* Unwraps the internal [GeckoEngineSession].
*
* After calling [unwrap] the wrapper will no longer observe the [GeckoEngineSession] and fruther
* crash handling is left to the application.
*/
fun unwrap(): GeckoEngineSession {
engineSession.unregister(observer)
return engineSession
}

/**
* Cleans up the internal state of this [SpeculativeEngineSession]. After calling this method
* his [SpeculativeEngineSession] cannot be used anynmore.
*/
fun cleanUp() {
engineSession.unregister(observer)
engineSession.close()
}

companion object {
fun create(
factory: SpeculativeSessionFactory,
runtime: GeckoRuntime,
private: Boolean,
contextId: String?,
defaultSettings: Settings?
): SpeculativeEngineSession {
val engineSession = GeckoEngineSession(runtime, private, defaultSettings, contextId)
val observer = SpeculativeSessionObserver(factory)
engineSession.register(observer)

return SpeculativeEngineSession(engineSession, observer)
}
}
}

/**
* [EngineSession.Observer] implementation that will notify the [SpeculativeSessionFactory] if an
* [GeckoEngineSession] can no longer be used after a crash.
*/
internal class SpeculativeSessionObserver(
private val factory: SpeculativeSessionFactory

) : EngineSession.Observer {
override fun onCrash() {
factory.clear()
}

override fun onProcessKilled() {
factory.clear()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import android.content.Context
import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.browser.engine.gecko.ext.getAntiTrackingPolicy
import mozilla.components.browser.engine.gecko.mediaquery.toGeckoValue
import mozilla.components.browser.engine.gecko.util.SpeculativeEngineSession
import mozilla.components.browser.engine.gecko.util.SpeculativeSessionObserver
import mozilla.components.concept.engine.DefaultSettings
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.engine.EngineSession.TrackingProtectionPolicy
Expand Down Expand Up @@ -94,60 +96,62 @@ class GeckoEngineTest {

// Create a private speculative session and consume it
engine.speculativeCreateSession(private = true)
assertNotNull(engine.speculativeSession)
var privateSpeculativeSession = engine.speculativeSession!!
assertTrue(engine.speculativeConnectionFactory.hasSpeculativeSession())
var privateSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!.engineSession
assertSame(privateSpeculativeSession, engine.createSession(private = true))
assertNull(engine.speculativeSession)
assertFalse(engine.speculativeConnectionFactory.hasSpeculativeSession())

// Create a regular speculative session and make sure it is not returned
// if a private session is requested instead.
engine.speculativeCreateSession(private = false)
assertNotNull(engine.speculativeSession)
privateSpeculativeSession = engine.speculativeSession!!
assertTrue(engine.speculativeConnectionFactory.hasSpeculativeSession())
privateSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!.engineSession
assertNotSame(privateSpeculativeSession, engine.createSession(private = true))
// Make sure previous (never used) speculative session is now closed
assertFalse(privateSpeculativeSession.geckoSession.isOpen)
assertNull(engine.speculativeSession)
assertFalse(engine.speculativeConnectionFactory.hasSpeculativeSession())
}

@Test
fun speculativeCreateSession() {
val engine = GeckoEngine(context, runtime = runtime)
assertNull(engine.speculativeSession)
assertNull(engine.speculativeConnectionFactory.speculativeEngineSession)

// Create a private speculative session
engine.speculativeCreateSession(private = true)
assertNotNull(engine.speculativeSession)
val privateSpeculativeSession = engine.speculativeSession!!
assertTrue(privateSpeculativeSession.geckoSession.settings.usePrivateMode)
assertNotNull(engine.speculativeConnectionFactory.speculativeEngineSession)
val privateSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!
assertTrue(privateSpeculativeSession.engineSession.geckoSession.settings.usePrivateMode)

// Creating another private speculative session should have no effect as
// session hasn't been "consumed".
engine.speculativeCreateSession(private = true)
assertSame(privateSpeculativeSession, engine.speculativeSession)
assertTrue(privateSpeculativeSession.geckoSession.settings.usePrivateMode)
assertSame(privateSpeculativeSession, engine.speculativeConnectionFactory.speculativeEngineSession)
assertTrue(privateSpeculativeSession.engineSession.geckoSession.settings.usePrivateMode)

// Creating a non-private speculative session should affect prepared session
engine.speculativeCreateSession(private = false)
assertNotSame(privateSpeculativeSession, engine.speculativeSession)
assertNotSame(privateSpeculativeSession, engine.speculativeConnectionFactory.speculativeEngineSession)
// Make sure previous (never used) speculative session is now closed
assertFalse(privateSpeculativeSession.geckoSession.isOpen)
val regularSpeculativeSession = engine.speculativeSession!!
assertFalse(regularSpeculativeSession.geckoSession.settings.usePrivateMode)
assertFalse(privateSpeculativeSession.engineSession.geckoSession.isOpen)
val regularSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!
assertFalse(regularSpeculativeSession.engineSession.geckoSession.settings.usePrivateMode)
}

@Test
fun clearSpeculativeSession() {
val engine = GeckoEngine(context, runtime = runtime)
assertNull(engine.speculativeSession)
assertNull(engine.speculativeConnectionFactory.speculativeEngineSession)

val mockGeckoSession: GeckoSession = mock()
val mockEngineSession: GeckoEngineSession = mock()
whenever(mockEngineSession.geckoSession).thenReturn(mockGeckoSession)
engine.speculativeSession = mockEngineSession
val mockEngineSessionObserver: SpeculativeSessionObserver = mock()
engine.speculativeConnectionFactory.speculativeEngineSession =
SpeculativeEngineSession(mockEngineSession, mockEngineSessionObserver)
engine.clearSpeculativeSession()
verify(mockGeckoSession).close()
assertNull(engine.speculativeSession)

verify(mockEngineSession).unregister(mockEngineSessionObserver)
verify(mockEngineSession).close()
assertNull(engine.speculativeConnectionFactory.speculativeEngineSession)
}

@Test
Expand All @@ -156,20 +160,20 @@ class GeckoEngineTest {

// Create a speculative session with a context id and consume it
engine.speculativeCreateSession(private = false, contextId = "1")
assertNotNull(engine.speculativeSession)
var newSpeculativeSession = engine.speculativeSession!!
assertNotNull(engine.speculativeConnectionFactory.speculativeEngineSession)
var newSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!.engineSession
assertSame(newSpeculativeSession, engine.createSession(private = false, contextId = "1"))
assertNull(engine.speculativeSession)
assertNull(engine.speculativeConnectionFactory.speculativeEngineSession)

// Create a regular speculative session and make sure it is not returned
// if a session with a context id is requested instead.
engine.speculativeCreateSession(private = false)
assertNotNull(engine.speculativeSession)
newSpeculativeSession = engine.speculativeSession!!
assertNotNull(engine.speculativeConnectionFactory.speculativeEngineSession)
newSpeculativeSession = engine.speculativeConnectionFactory.speculativeEngineSession!!.engineSession
assertNotSame(newSpeculativeSession, engine.createSession(private = false, contextId = "1"))
// Make sure previous (never used) speculative session is now closed
assertFalse(newSpeculativeSession.geckoSession.isOpen)
assertNull(engine.speculativeSession)
assertNull(engine.speculativeConnectionFactory.speculativeEngineSession)
}

@Test
Expand Down
Loading

15 comments on commit 3f00d96

@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.

@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.

@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.

@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.

@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.