Bug 889554 - Save session between app upgrades. r=mfinkle

mobile/android/base/GeckoApp.java

@@ -40,6 +40,7 @@ import android.content.Context;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.content.SharedPreferences;
+import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.res.Configuration;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
@@ -154,9 +155,10 @@ abstract public class GeckoApp
     public static final String PREFS_OOM_EXCEPTION = "OOMException";
     public static final String PREFS_WAS_STOPPED   = "wasStopped";
     public static final String PREFS_CRASHED       = "crashed";
+    public static final String PREFS_VERSION_CODE  = "versionCode";
 
     static public final int RESTORE_NONE = 0;
-    static public final int RESTORE_OOM = 1;
+    static public final int RESTORE_NORMAL = 1;
     static public final int RESTORE_CRASH = 2;
 
     static private final String LOCATION_URL = "https://location.services.mozilla.com/v1/submit";
@@ -1258,7 +1260,7 @@ abstract public class GeckoApp
         // we were in the background, or a more harsh kill while we were
         // active.
         mRestoreMode = getSessionRestoreState(savedInstanceState);
-        if (mRestoreMode == RESTORE_OOM) {
+        if (mRestoreMode == RESTORE_NORMAL && savedInstanceState != null) {
             boolean wasInBackground =
                 savedInstanceState.getBoolean(SAVED_STATE_IN_BACKGROUND, false);
 
@@ -1416,7 +1418,7 @@ abstract public class GeckoApp
             loadStartupTab(isExternalURL ? passedUri : null);
         }
 
-        if (mRestoreMode == RESTORE_OOM) {
+        if (mRestoreMode == RESTORE_NORMAL) {
             // If we successfully did an OOM restore, we now have tab stubs
             // from the last session. Any future tabs should be animated.
             Tabs.getInstance().notifyListeners(null, Tabs.TabEvents.RESTORED);
@@ -1597,7 +1599,7 @@ abstract public class GeckoApp
             // If we are doing an OOM restore, parse the session data and
             // stub the restored tabs immediately. This allows the UI to be
             // updated before Gecko has restored.
-            if (mRestoreMode == RESTORE_OOM) {
+            if (mRestoreMode == RESTORE_NORMAL) {
                 final JSONArray tabs = new JSONArray();
                 SessionParser parser = new SessionParser() {
                     @Override
@@ -1635,7 +1637,7 @@ abstract public class GeckoApp
             }
 
             JSONObject restoreData = new JSONObject();
-            restoreData.put("restoringOOM", mRestoreMode == RESTORE_OOM);
+            restoreData.put("normalRestore", mRestoreMode == RESTORE_NORMAL);
             restoreData.put("sessionString", sessionString);
             return restoreData.toString();
 
@@ -1653,11 +1655,26 @@ abstract public class GeckoApp
     }
 
     protected int getSessionRestoreState(Bundle savedInstanceState) {
-        if (savedInstanceState != null) {
-            return RESTORE_OOM;
-        }
-
         final SharedPreferences prefs = GeckoApp.getAppSharedPreferences();
+        int restoreMode = RESTORE_NONE;
+
+        // If the version has changed, the user has done an upgrade, so restore
+        // previous tabs.
+        final int versionCode = getVersionCode();
+        if (prefs.getInt(PREFS_VERSION_CODE, 0) != versionCode) {
+            ThreadUtils.postToBackgroundThread(new Runnable() {
+                @Override
+                public void run() {
+                    prefs.edit()
+                         .putInt(PREFS_VERSION_CODE, versionCode)
+                         .commit();
+                }
+            });
+
+            restoreMode = RESTORE_NORMAL;
+        } else if (savedInstanceState != null) {
+            restoreMode = RESTORE_NORMAL;
+        }
 
         // We record crashes in the crash reporter. If sessionstore.js
         // exists, but we didn't flag a crash in the crash reporter, we
@@ -1668,16 +1685,15 @@ abstract public class GeckoApp
                 @Override
                 public void run() {
                     prefs.edit()
-                         .putBoolean(GeckoApp.PREFS_CRASHED, false)
+                         .putBoolean(PREFS_CRASHED, false)
                          .commit();
                 }
             });
 
-            if (getProfile().shouldRestoreSession()) {
+            restoreMode = RESTORE_CRASH;
-                return RESTORE_CRASH;
-            }
         }
-        return RESTORE_NONE;
+
+        return restoreMode;
     }
 
     /**
@@ -2656,4 +2672,14 @@ abstract public class GeckoApp
         // during downloads.
         return new AppNotificationClient(getApplicationContext());
     }
+
+    private int getVersionCode() {
+        int versionCode = 0;
+        try {
+            versionCode = getPackageManager().getPackageInfo(getPackageName(), 0).versionCode;
+        } catch (NameNotFoundException e) {
+            Log.wtf(LOGTAG, getPackageName() + " not found", e);
+        }
+        return versionCode;
+    }
 }

mobile/android/base/GeckoProfile.java

@@ -166,23 +166,6 @@ public final class GeckoProfile {
         return mContext.getFilesDir();
     }
 
-    /**
-     * Determines whether the tabs from the previous session should be
-     * automatically restored.
-     *
-     * sessionstore.js is moved to sessionstore.bak on a clean quit, so if we
-     * still have sessionstore.js at startup, that means we were killed
-     * uncleanly. This is caused by either 1) a crash, or 2) being killed by
-     * android because of memory constraints. Either way, the existence of this
-     * file indicates that we'll want to restore the previous session.
-     *
-     * @return whether the previous session should be restored
-     */
-    public boolean shouldRestoreSession() {
-        File sessionFile = getFile("sessionstore.js");
-        return sessionFile != null && sessionFile.exists();
-    }
-
     /**
      * Moves the session file to the backup session file.
      *

mobile/android/components/SessionStore.idl

@@ -77,9 +77,10 @@ interface nsISessionStore : nsISupports
 
   /**
    * Restores the previous browser session using a fast, lightweight strategy
-   * @param aRestoringOOM  Whether this is an OOM restore from Android
+   * @param aNormalRestore True for a normal session restore; false for a crash
+   *                       restore.
    * @param aSessionString The session string to restore from. If null, the
    *                       backup session file is read from.
    */
-  void restoreLastSession(in boolean aRestoringOOM, in AString aSessionString);
+  void restoreLastSession(in boolean aNormalRestore, in AString aSessionString);
 };

mobile/android/components/SessionStore.js

@@ -223,7 +223,7 @@ SessionStore.prototype = {
 
           // Do a restore, triggered by Java
           let data = JSON.parse(aData);
-          this.restoreLastSession(data.restoringOOM, data.sessionString);
+          this.restoreLastSession(data.normalRestore, data.sessionString);
         } else if (this._shouldRestore) {
           // Do a restore triggered by Gecko (e.g., if
           // browser.sessionstore.resume_session_once is true). In these cases,
@@ -956,7 +956,7 @@ SessionStore.prototype = {
     return this._shouldRestore;
   },
 
-  restoreLastSession: function ss_restoreLastSession(aRestoringOOM, aSessionString) {
+  restoreLastSession: function ss_restoreLastSession(aNormalRestore, aSessionString) {
     let self = this;
 
     function restoreWindow(data) {
@@ -972,10 +972,13 @@ SessionStore.prototype = {
     }
 
     try {
-      if (!aRestoringOOM && !this._shouldRestore) {
+      if (!aNormalRestore && !this._shouldRestore) {
-        // If we're here, it means we're restoring from a crash (not an OOM
+        // If we're here, it means we're restoring from a crash. Check prefs
-        // kill). Check prefs and other conditions to make sure we want to
+        // and other conditions to make sure we want to continue with the
-        // continue with the restore.
+        // restore.
+        // TODO: Since the tabs have already been created as stubs after
+        // crashing, it's too late to try to abort the restore here. This logic
+        // should be moved to Java; see bug 889722.
 
         // Disable crash recovery if it has been turned off.
         if (!Services.prefs.getBoolPref("browser.sessionstore.resume_from_crash")) {