Bug 1484001 - Avoid ending up with a null mRuntime after restoring state in GeckoView. r=snorp
authorJan Henning <jh+bugzilla@buttercookie.de>
Thu, 30 Aug 2018 20:09:02 +0000
changeset 482653 34d7cf7fa7eb1c51c6e7f2b4cbf3e5da20ba36d8
parent 482652 c476aa79a8ca991dc8ea1e5c2eab98e442dad406
child 482654 8d89bfeb0b2f480a638e83a7d0178039033438f1
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewerssnorp
bugs1484001
milestone63.0a1
Bug 1484001 - Avoid ending up with a null mRuntime after restoring state in GeckoView. r=snorp If we don't have a saved session, we do nothing. If somebody called setSession beforehand, we continue using that session, otherwise we create a fallback session in onAttachedToWindow(). If we have a saved session and nobody called setSession, we use the saved session. If the saved session was closed and doesn't have a runtime, we use the default runtime as a fallback. If we have both a saved session and somebody already called setSession, we transfer what can be transferred from the saved session, unless the saved session is closed and the session from setSession is open. If the saved session was open, we use its runtime as well going forward (since transferring the state from an open session transfers the window and the runtime as well), otherwise we keep the old mRuntime. Differential Revision: https://phabricator.services.mozilla.com/D4711
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
@@ -358,22 +358,17 @@ public class GeckoView extends FrameLayo
     public void onAttachedToWindow() {
         if (mSession == null) {
             setSession(new GeckoSession(), GeckoRuntime.getDefault(getContext()));
         }
 
         if (!mSession.isOpen()) {
             mSession.open(mRuntime);
         }
-        // Temporary solution until we find out why mRuntime can end up as null here. It means we
-        // might miss an orientation change if we were background OOM-killed, but it's better than
-        // crashing. See bug 1484001.
-        if (mRuntime != null) {
-            mRuntime.orientationChanged();
-        }
+        mRuntime.orientationChanged();
 
         super.onAttachedToWindow();
     }
 
     @Override
     public void onDetachedFromWindow() {
         super.onDetachedFromWindow();
 
@@ -423,21 +418,46 @@ public class GeckoView extends FrameLayo
         if (!(state instanceof SavedState)) {
             super.onRestoreInstanceState(state);
             return;
         }
 
         final SavedState ss = (SavedState) state;
         super.onRestoreInstanceState(ss.getSuperState());
 
-        if (mSession == null && ss.session != null) {
-            setSession(ss.session, ss.session.getRuntime());
-        } else if (ss.session != null) {
-            mSession.transferFrom(ss.session);
-            mRuntime = mSession.getRuntime();
+        restoreSession(ss.session);
+    }
+
+    private void restoreSession(final @Nullable GeckoSession savedSession) {
+        if (savedSession == null) {
+            return;
+        }
+
+        GeckoRuntime runtimeToRestore = savedSession.getRuntime();
+        // Note: setSession sets either both mSession and mRuntime, or none of them. So if we don't
+        // have an mRuntime here, we won't have an mSession, either.
+        if (mRuntime == null) {
+            if (runtimeToRestore == null) {
+                // If the saved session is closed, we fall back to using the default runtime, same
+                // as we do when we don't even have an mSession in onAttachedToWindow().
+                runtimeToRestore = GeckoRuntime.getDefault(getContext());
+            }
+            setSession(savedSession, runtimeToRestore);
+        // We already have a session. We only want to transfer the saved session if its close/open
+        // state is the same or better as our current session.
+        } else if (savedSession.isOpen() || !mSession.isOpen()) {
+            if (mSession.isOpen()) {
+                mSession.close();
+            }
+            mSession.transferFrom(savedSession);
+            if (runtimeToRestore != null) {
+                // If the saved session was open, we transfer its runtime as well. Otherwise we just
+                // keep the runtime we already had in mRuntime.
+                mRuntime = runtimeToRestore;
+            }
         }
     }
 
     @Override
     public void onWindowFocusChanged(boolean hasWindowFocus) {
         super.onWindowFocusChanged(hasWindowFocus);
 
         if (mSession != null) {