Closes mozilla-mobile#3310: browser-engine-gecko-nightly: Automatical…

…ly recover and restore if content process is killed.

components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt

@@ -498,7 +498,7 @@ class GeckoEngineSession(
             geckoSession.close()
             createGeckoSession()
 
-            notifyObservers { onCrashStateChange(crashed = true) }
+            notifyObservers { onCrash() }
         }
 
         override fun onFullScreen(session: GeckoSession, fullScreen: Boolean) {

components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt

@@ -41,12 +41,8 @@ class GeckoEngineView @JvmOverloads constructor(
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal val observer = object : EngineSession.Observer {
-        override fun onCrashStateChange(crashed: Boolean) {
-            if (crashed) {
-                // When crashing the previous GeckoSession is no longer usable. Internally GeckoEngineSession will
-                // create a new instance. This means we will need to tell GeckoView about this new GeckoSession:
-                currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
-            }
+        override fun onCrash() {
+            rebind()
         }
         override fun onAppPermissionRequest(permissionRequest: PermissionRequest) = Unit
         override fun onContentPermissionRequest(permissionRequest: PermissionRequest) = Unit
@@ -85,6 +81,15 @@ 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() {
+        currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
+    }
+
     @Synchronized
     override fun release() {
         currentSession?.apply { unregister(observer) }

components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt

@@ -1595,11 +1595,11 @@ class GeckoEngineSessionTest {
 
         captureDelegates()
 
-        var crashedState: Boolean? = null
+        var crashedState = false
 
         engineSession.register(object : EngineSession.Observer {
-            override fun onCrashStateChange(crashed: Boolean) {
-                crashedState = crashed
+            override fun onCrash() {
+                crashedState = true
             }
         })
 

components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt

@@ -15,6 +15,7 @@ import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito
 import org.mockito.Mockito.never
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
@@ -104,4 +105,24 @@ class GeckoEngineViewTest {
         verify(geckoView).releaseSession()
         verify(engineSession).unregister(any())
     }
+
+    @Test
+    fun `View will rebind session if session crashed`() {
+        val engineView = GeckoEngineView(context)
+        val engineSession = mock<GeckoEngineSession>()
+        val geckoSession = mock<GeckoSession>()
+        val geckoView = mock<NestedGeckoView>()
+
+        whenever(engineSession.geckoSession).thenReturn(geckoSession)
+        engineView.currentGeckoView = geckoView
+
+        engineView.render(engineSession)
+
+        Mockito.reset(geckoView)
+        verify(geckoView, never()).setSession(geckoSession)
+
+        engineView.observer.onCrash()
+
+        verify(geckoView).setSession(geckoSession)
+    }
 }

components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt

@@ -500,10 +500,33 @@ 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() }
+        }
+
+        private fun recoverGeckoSession() {
+            // Recover the GeckoSession after the process getting killed or crashing. We create a
+            // new underlying GeckoSession.
+            // Eventually we may be able to re-use the same GeckoSession by re-opening it. However
+            // that seems to have caused issues:
+            // https://github.com/mozilla-mobile/android-components/issues/3640
+
             geckoSession.close()
             createGeckoSession()
-
-            notifyObservers { onCrashStateChange(crashed = true) }
         }
 
         override fun onFullScreen(session: GeckoSession, fullScreen: Boolean) {

components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt

@@ -41,13 +41,14 @@ class GeckoEngineView @JvmOverloads constructor(
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal val observer = object : EngineSession.Observer {
-        override fun onCrashStateChange(crashed: Boolean) {
-            if (crashed) {
-                // When crashing the previous GeckoSession is no longer usable. Internally GeckoEngineSession will
-                // create a new instance. This means we will need to tell GeckoView about this new GeckoSession:
-                currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
-            }
+        override fun onCrash() {
+            rebind()
+        }
+
+        override fun onProcessKilled() {
+            rebind()
         }
+
         override fun onAppPermissionRequest(permissionRequest: PermissionRequest) = Unit
         override fun onContentPermissionRequest(permissionRequest: PermissionRequest) = Unit
     }
@@ -85,6 +86,15 @@ 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() {
+        currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
+    }
+
     @Synchronized
     override fun release() {
         currentSession?.apply { unregister(observer) }

components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt

@@ -1592,11 +1592,11 @@ class GeckoEngineSessionTest {
 
         captureDelegates()
 
-        var crashedState: Boolean? = null
+        var crashedState = false
 
         engineSession.register(object : EngineSession.Observer {
-            override fun onCrashStateChange(crashed: Boolean) {
-                crashedState = crashed
+            override fun onCrash() {
+                crashedState = true
             }
         })
 
@@ -1792,6 +1792,32 @@ class GeckoEngineSessionTest {
         assertEquals(LoadUrlFlags.BYPASS_CLASSIFIER, GeckoSession.LOAD_FLAGS_BYPASS_CLASSIFIER)
     }
 
+    @Test
+    fun `onKill will recover, restore state and notify observers`() {
+        val engineSession = GeckoEngineSession(mock(),
+            geckoSessionProvider = geckoSessionProvider)
+
+        captureDelegates()
+
+        var observerNotified = false
+
+        engineSession.register(object : EngineSession.Observer {
+            override fun onProcessKilled() {
+                observerNotified = true
+            }
+        })
+
+        val mockedState: GeckoSession.SessionState = mock()
+        progressDelegate.value.onSessionStateChange(geckoSession, mockedState)
+
+        verify(geckoSession, never()).restoreState(mockedState)
+
+        contentDelegate.value.onKill(geckoSession)
+
+        verify(geckoSession).restoreState(mockedState)
+        assertTrue(observerNotified)
+    }
+
     private fun mockGeckoSession(): GeckoSession {
         val session = mock<GeckoSession>()
         whenever(session.settings).thenReturn(

components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt

@@ -16,6 +16,7 @@ import org.junit.Assert.assertNull
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.never
+import org.mockito.Mockito.reset
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.mozilla.geckoview.GeckoResult
@@ -104,4 +105,44 @@ class GeckoEngineViewTest {
         verify(geckoView).releaseSession()
         verify(engineSession).unregister(any())
     }
+
+    @Test
+    fun `View will rebind session if process gets killed`() {
+        val engineView = GeckoEngineView(context)
+        val engineSession = mock<GeckoEngineSession>()
+        val geckoSession = mock<GeckoSession>()
+        val geckoView = mock<NestedGeckoView>()
+
+        whenever(engineSession.geckoSession).thenReturn(geckoSession)
+        engineView.currentGeckoView = geckoView
+
+        engineView.render(engineSession)
+
+        reset(geckoView)
+        verify(geckoView, never()).setSession(geckoSession)
+
+        engineView.observer.onProcessKilled()
+
+        verify(geckoView).setSession(geckoSession)
+    }
+
+    @Test
+    fun `View will rebind session if session crashed`() {
+        val engineView = GeckoEngineView(context)
+        val engineSession = mock<GeckoEngineSession>()
+        val geckoSession = mock<GeckoSession>()
+        val geckoView = mock<NestedGeckoView>()
+
+        whenever(engineSession.geckoSession).thenReturn(geckoSession)
+        engineView.currentGeckoView = geckoView
+
+        engineView.render(engineSession)
+
+        reset(geckoView)
+        verify(geckoView, never()).setSession(geckoSession)
+
+        engineView.observer.onCrash()
+
+        verify(geckoView).setSession(geckoSession)
+    }
 }

components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt

@@ -499,7 +499,7 @@ class GeckoEngineSession(
             geckoSession.close()
             createGeckoSession()
 
-            notifyObservers { onCrashStateChange(crashed = true) }
+            notifyObservers { onCrash() }
         }
 
         override fun onFullScreen(session: GeckoSession, fullScreen: Boolean) {

components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt

@@ -41,12 +41,8 @@ class GeckoEngineView @JvmOverloads constructor(
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal val observer = object : EngineSession.Observer {
-        override fun onCrashStateChange(crashed: Boolean) {
-            if (crashed) {
-                // When crashing the previous GeckoSession is no longer usable. Internally GeckoEngineSession will
-                // create a new instance. This means we will need to tell GeckoView about this new GeckoSession:
-                currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
-            }
+        override fun onCrash() {
+            rebind()
         }
         override fun onAppPermissionRequest(permissionRequest: PermissionRequest) = Unit
         override fun onContentPermissionRequest(permissionRequest: PermissionRequest) = Unit
@@ -85,6 +81,15 @@ 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() {
+        currentSession?.let { currentGeckoView.setSession(it.geckoSession) }
+    }
+
     @Synchronized
     override fun release() {
         currentSession?.apply { unregister(observer) }

components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt

@@ -1568,11 +1568,11 @@ class GeckoEngineSessionTest {
 
         captureDelegates()
 
-        var crashedState: Boolean? = null
+        var crashedState: Boolean = false
 
         engineSession.register(object : EngineSession.Observer {
-            override fun onCrashStateChange(crashed: Boolean) {
-                crashedState = crashed
+            override fun onCrash() {
+                crashedState = true
             }
         })
 

components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt

@@ -104,4 +104,24 @@ class GeckoEngineViewTest {
         verify(geckoView).releaseSession()
         verify(engineSession).unregister(any())
     }
+
+    @Test
+    fun `View will rebind session if session crashed`() {
+        val engineView = GeckoEngineView(context)
+        val engineSession = mock<GeckoEngineSession>()
+        val geckoSession = mock<GeckoSession>()
+        val geckoView = mock<NestedGeckoView>()
+
+        whenever(engineSession.geckoSession).thenReturn(geckoSession)
+        engineView.currentGeckoView = geckoView
+
+        engineView.render(engineSession)
+
+        Mockito.reset(geckoView)
+        verify(geckoView, Mockito.never()).setSession(geckoSession)
+
+        engineView.observer.onCrash()
+
+        verify(geckoView).setSession(geckoSession)
+    }
 }

components/browser/session/src/main/java/mozilla/components/browser/session/engine/EngineObserver.kt

@@ -174,8 +174,8 @@ internal class EngineObserver(
         session.webAppManifest = manifest
     }
 
-    override fun onCrashStateChange(crashed: Boolean) {
-        session.crashed = crashed
+    override fun onCrash() {
+        session.crashed = true
     }
 
     override fun onRecordingStateChanged(devices: List<RecordingDevice>) {

components/browser/session/src/test/java/mozilla/components/browser/session/engine/EngineObserverTest.kt

@@ -411,13 +411,12 @@ class EngineObserverTest {
 
         val observer = EngineObserver(session)
 
-        observer.onCrashStateChange(true)
+        observer.onCrash()
         assertTrue(session.crashed)
 
-        observer.onCrashStateChange(false)
-        assertFalse(session.crashed)
+        session.crashed = false
 
-        observer.onCrashStateChange(true)
+        observer.onCrash()
         assertTrue(session.crashed)
     }
 

components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt

@@ -54,7 +54,8 @@ abstract class EngineSession(
         fun onMediaAdded(media: Media) = Unit
         fun onMediaRemoved(media: Media) = Unit
         fun onWebAppManifestLoaded(manifest: WebAppManifest) = Unit
-        fun onCrashStateChange(crashed: Boolean) = Unit
+        fun onCrash() = Unit
+        fun onProcessKilled() = Unit
         fun onRecordingStateChanged(devices: List<RecordingDevice>) = Unit
 
         /**