Bug 896170 - Part 2: Use intent extra to trigger restore on restart. r=mfinkle

--HG--
extra : rebase_source : fd14708cfc970dce433c02c1a08863e38680e5b3

mobile/android/base/BrowserApp.java

@@ -108,9 +108,6 @@ abstract public class BrowserApp extends GeckoApp
     private View mHomePagerContainer;
     protected Telemetry.Timer mAboutHomeStartupTimer = null;
 
-    // Set the default session restore value
-    private int mSessionRestore = -1;
-
     private static final int GECKO_TOOLS_MENU = -1;
     private static final int ADDON_MENU_OFFSET = 1000;
     private class MenuItemInfo {
@@ -385,15 +382,6 @@ abstract public class BrowserApp extends GeckoApp
         });
     }
 
-    @Override
-    protected int getSessionRestoreState(Bundle savedInstanceState) {
-        if (mSessionRestore > -1) {
-            return mSessionRestore;
-        }
-
-        return super.getSessionRestoreState(savedInstanceState);
-    }
-
     @Override
     public void onCreate(Bundle savedInstanceState) {
         mAboutHomeStartupTimer = new Telemetry.Timer("FENNEC_STARTUP_TIME_ABOUTHOME");
@@ -401,8 +389,8 @@ abstract public class BrowserApp extends GeckoApp
         String args = getIntent().getStringExtra("args");
         if (args != null && args.contains(GUEST_BROWSING_ARG)) {
             mProfile = GeckoProfile.createGuestProfile(this);
-        } else if (GeckoProfile.maybeCleanupGuestProfile(this)) {
-            mSessionRestore = RESTORE_NORMAL;
+        } else {
+            GeckoProfile.maybeCleanupGuestProfile(this);
         }
 
         super.onCreate(savedInstanceState);
@@ -826,7 +814,7 @@ abstract public class BrowserApp extends GeckoApp
     @Override
     protected void loadStartupTab(String url) {
         // We aren't showing about:home, so cancel the telemetry timer
-        if (url != null || mRestoreMode != RESTORE_NONE) {
+        if (url != null || mShouldRestore) {
             mAboutHomeStartupTimer.cancel();
         }
 

mobile/android/base/CrashReporter.java

@@ -132,7 +132,6 @@ public class CrashReporter extends Activity
             getSharedPreferences(GeckoApp.PREFS_NAME, 0);
         SharedPreferences.Editor editor = prefs.edit();
         editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
-        editor.putBoolean(GeckoApp.PREFS_CRASHED, true);
         editor.commit();
 
         final CheckBox allowContactCheckBox = (CheckBox) findViewById(R.id.allow_contact);
@@ -413,6 +412,7 @@ public class CrashReporter extends Activity
             Intent intent = new Intent(action);
             intent.setClassName(AppConstants.ANDROID_PACKAGE_NAME,
                                 AppConstants.BROWSER_INTENT_CLASS);
+            intent.putExtra("didRestart", true);
             Log.i(LOGTAG, intent.toString());
             startActivity(intent);
         } catch (Exception e) {

mobile/android/base/GeckoApp.java

@@ -149,13 +149,8 @@ abstract public class GeckoApp
     public static final String PREFS_NAME          = "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_NORMAL = 1;
-    static public final int RESTORE_CRASH = 2;
-
     static private final String LOCATION_URL = "https://location.services.mozilla.com/v1/submit";
 
     // Delay before running one-time "cleanup" tasks that may be needed
@@ -197,7 +192,7 @@ abstract public class GeckoApp
 
     private HashMap<String, PowerManager.WakeLock> mWakeLocks = new HashMap<String, PowerManager.WakeLock>();
 
-    protected int mRestoreMode = RESTORE_NONE;
+    protected boolean mShouldRestore;
     protected boolean mInitialized = false;
     private Telemetry.Timer mJavaUiStartupTimer;
     private Telemetry.Timer mGeckoReadyStartupTimer;
@@ -1243,11 +1238,9 @@ abstract public class GeckoApp
         mNotificationHelper = new NotificationHelper(this);
         mToast = new ButtonToast(findViewById(R.id.toast));
 
-        // Check if the last run was exited due to a normal kill while
-        // we were in the background, or a more harsh kill while we were
-        // active.
-        mRestoreMode = getSessionRestoreState(savedInstanceState);
-        if (mRestoreMode == RESTORE_NORMAL && savedInstanceState != null) {
+        // Determine whether we should restore tabs.
+        mShouldRestore = getSessionRestoreState(savedInstanceState);
+        if (mShouldRestore && savedInstanceState != null) {
             boolean wasInBackground =
                 savedInstanceState.getBoolean(SAVED_STATE_IN_BACKGROUND, false);
 
@@ -1334,7 +1327,7 @@ abstract public class GeckoApp
      */
     protected void loadStartupTab(String url) {
         if (url == null) {
-            if (mRestoreMode == RESTORE_NONE) {
+            if (!mShouldRestore) {
                 // Show about:home if we aren't restoring previous session and
                 // there's no external URL
                 Tab tab = Tabs.getInstance().loadUrl("about:home", Tabs.LOADURL_NEW_TAB);
@@ -1389,7 +1382,7 @@ abstract public class GeckoApp
         // If we are doing a restore, read the session data and send it to Gecko
         if (!mIsRestoringActivity) {
             String restoreMessage = null;
-            if (mRestoreMode != RESTORE_NONE) {
+            if (mShouldRestore) {
                 try {
                     // restoreSessionTabs() will create simple tab stubs with the
                     // URL and title for each page, but we also need to restore
@@ -1401,7 +1394,7 @@ abstract public class GeckoApp
                 } catch (SessionRestoreException e) {
                     // If restore failed, do a normal startup
                     Log.e(LOGTAG, "An error occurred during restore", e);
-                    mRestoreMode = RESTORE_NONE;
+                    mShouldRestore = false;
                 }
             }
 
@@ -1416,17 +1409,14 @@ abstract public class GeckoApp
             loadStartupTab(null);
         }
 
-        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);
-        } else {
-            // Move the session file if it exists
-            getProfile().moveSessionFile();
-        }
+        // We now have tab stubs from the last session. Any future tabs should
+        // be animated.
+        Tabs.getInstance().notifyListeners(null, Tabs.TabEvents.RESTORED);
 
-        if (mRestoreMode == RESTORE_NONE) {
-            Tabs.getInstance().notifyListeners(null, Tabs.TabEvents.RESTORED);
+        // If we're not restoring, move the session file so it can be read for
+        // the last tabs section.
+        if (!mShouldRestore) {
+            getProfile().moveSessionFile();
         }
 
         Telemetry.HistogramAdd("FENNEC_STARTUP_GECKOAPP_ACTION", startupAction.ordinal());
@@ -1597,7 +1587,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_NORMAL) {
+            if (mShouldRestore) {
                 final JSONArray tabs = new JSONArray();
                 SessionParser parser = new SessionParser() {
                     @Override
@@ -1635,7 +1625,6 @@ abstract public class GeckoApp
             }
 
             JSONObject restoreData = new JSONObject();
-            restoreData.put("normalRestore", mRestoreMode == RESTORE_NORMAL);
             restoreData.put("sessionString", sessionString);
             return restoreData.toString();
 
@@ -1652,9 +1641,15 @@ abstract public class GeckoApp
         return mProfile;
     }
 
-    protected int getSessionRestoreState(Bundle savedInstanceState) {
+    /**
+     * Determine whether the session should be restored.
+     *
+     * @param savedInstanceState Saved instance state given to the activity
+     * @return                   Whether to restore
+     */
+    protected boolean getSessionRestoreState(Bundle savedInstanceState) {
         final SharedPreferences prefs = GeckoApp.getAppSharedPreferences();
-        int restoreMode = RESTORE_NONE;
+        boolean shouldRestore = false;
 
         final int versionCode = getVersionCode();
         if (prefs.getInt(PREFS_VERSION_CODE, 0) != versionCode) {
@@ -1669,33 +1664,23 @@ abstract public class GeckoApp
                 }
             });
 
-            restoreMode = RESTORE_NORMAL;
-        } else if (savedInstanceState != null || PreferenceManager.getDefaultSharedPreferences(this)
-                                                                  .getString(GeckoPreferences.PREFS_RESTORE_SESSION, "quit")
-                                                                  .equals("always")) {
-            // We're coming back from a background kill by the OS or the user
-            // has chosen to always restore, so restore.
-            restoreMode = RESTORE_NORMAL;
+            shouldRestore = true;
+        } else if (savedInstanceState != null || getSessionRestorePreference().equals("always") || getRestartFromIntent()) {
+            // We're coming back from a background kill by the OS, the user
+            // has chosen to always restore, or we just restarted.
+            shouldRestore = true;
         }
 
-        // We record crashes in the crash reporter. If sessionstore.js
-        // exists, but we didn't flag a crash in the crash reporter, we
-        // were probably just force killed by the user, so we shouldn't do
-        // a restore.
-        if (prefs.getBoolean(PREFS_CRASHED, false)) {
-            ThreadUtils.postToBackgroundThread(new Runnable() {
-                @Override
-                public void run() {
-                    prefs.edit()
-                         .putBoolean(PREFS_CRASHED, false)
-                         .commit();
-                }
-            });
+        return shouldRestore;
+    }
 
-            restoreMode = RESTORE_CRASH;
-        }
+    private String getSessionRestorePreference() {
+        return PreferenceManager.getDefaultSharedPreferences(this)
+                                .getString(GeckoPreferences.PREFS_RESTORE_SESSION, "quit");
+    }
 
-        return restoreMode;
+    private boolean getRestartFromIntent() {
+        return getIntent().getBooleanExtra("didRestart", false);
     }
 
     /**
@@ -2157,12 +2142,14 @@ abstract public class GeckoApp
                             Intent.FLAG_ACTIVITY_MULTIPLE_TASK);
             if (args != null)
                 intent.putExtra("args", args);
+            intent.putExtra("didRestart", true);
             Log.d(LOGTAG, "Restart intent: " + intent.toString());
             GeckoAppShell.killAnyZombies();
             startActivity(intent);
         } catch (Exception e) {
             Log.e(LOGTAG, "Error effecting restart.", e);
         }
+
         finish();
         // Give the restart process time to start before we die
         GeckoAppShell.waitForAnotherGeckoProc();

mobile/android/base/WebAppImpl.java

@@ -152,9 +152,9 @@ public class WebAppImpl extends GeckoApp {
     }
 
     @Override
-    protected int getSessionRestoreState(Bundle savedInstanceState) {
+    protected boolean getSessionRestoreState(Bundle savedInstanceState) {
         // for now webapps never restore your session
-        return RESTORE_NONE;
+        return false;
     }
 
     @Override

mobile/android/components/SessionStore.idl

@@ -70,17 +70,10 @@ interface nsISessionStore : nsISupports
    */
   void deleteTabValue(in jsval aTab, in AString aKey);
 
-  /**
-   * @returns A boolean indicating we should restore previous browser session
-   */
-  boolean shouldRestore();
-
   /**
    * Restores the previous browser session using a fast, lightweight strategy
-   * @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 aNormalRestore, in AString aSessionString);
+  void restoreLastSession(in AString aSessionString);
 };

mobile/android/components/SessionStore.js

@@ -44,7 +44,6 @@ SessionStore.prototype = {
   _lastSaveTime: 0,
   _interval: 10000,
   _maxTabsUndo: 1,
-  _shouldRestore: false,
 
   init: function ss_init() {
     // Get file references
@@ -57,12 +56,6 @@ SessionStore.prototype = {
 
     this._interval = Services.prefs.getIntPref("browser.sessionstore.interval");
     this._maxTabsUndo = Services.prefs.getIntPref("browser.sessionstore.max_tabs_undo");
-
-    // Do we need to restore session just this once, in case of a restart?
-    if (this._sessionFileBackup.exists() && Services.prefs.getBoolPref("browser.sessionstore.resume_session_once")) {
-      Services.prefs.setBoolPref("browser.sessionstore.resume_session_once", false);
-      this._shouldRestore = true;
-    }
   },
 
   _clearDisk: function ss_clearDisk() {
@@ -132,26 +125,9 @@ SessionStore.prototype = {
         this._loadState = STATE_QUITTING;
         break;
       case "quit-application":
-        // If we are restarting, lets restore the tabs
-        if (aData == "restart") {
-          Services.prefs.setBoolPref("browser.sessionstore.resume_session_once", true);
-
-          // Ignore purges when restarting. The notification is fired after "quit-application".
-          Services.obs.removeObserver(this, "browser:purge-session-history");
-        }
-
         // Freeze the data at what we've got (ignoring closing windows)
         this._loadState = STATE_QUITTING;
 
-        // Move this session to sessionstore.bak so that:
-        //   1) we can get "tabs from last time" from sessionstore.bak
-        //   2) if sessionstore.js exists on next start, we know we crashed
-        OS.File.move(this._sessionFile.path, this._sessionFileBackup.path).then(null, function onError(reason) {
-          if (!(reason instanceof OS.File.Error && reason.becauseNoSuchFile)) {
-            Cu.reportError("Error moving sessionstore files: " + reason);
-          }
-        });
-
         observerService.removeObserver(this, "domwindowopened");
         observerService.removeObserver(this, "domwindowclosed");
         observerService.removeObserver(this, "browser-lastwindow-close-granted");
@@ -217,13 +193,7 @@ SessionStore.prototype = {
 
           // Do a restore, triggered by Java
           let data = JSON.parse(aData);
-          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,
-          // our Java front-end doesn't know we're doing a restore, so it has
-          // already opened an about:home tab.
-          this.restoreLastSession(false, null);
+          this.restoreLastSession(data.sessionString);
         } else {
           // Not doing a restore; just send restore message
           Services.obs.notifyObservers(null, "sessionstore-windows-restored", "");
@@ -939,11 +909,7 @@ SessionStore.prototype = {
       throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
   },
 
-  shouldRestore: function ss_shouldRestore() {
-    return this._shouldRestore;
-  },
-
-  restoreLastSession: function ss_restoreLastSession(aNormalRestore, aSessionString) {
+  restoreLastSession: function ss_restoreLastSession(aSessionString) {
     let self = this;
 
     function restoreWindow(data) {
@@ -959,31 +925,6 @@ SessionStore.prototype = {
     }
 
     try {
-      if (!aNormalRestore && !this._shouldRestore) {
-        // If we're here, it means we're restoring from a crash. Check prefs
-        // and other conditions to make sure we want to continue with the
-        // 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")) {
-          throw "Restore is disabled via prefs";
-        }
-
-        // Check to see if we've exceeded the maximum number of crashes to
-        // avoid a crash loop
-        let maxCrashes = Services.prefs.getIntPref("browser.sessionstore.max_resumed_crashes");
-        let recentCrashes = Services.prefs.getIntPref("browser.sessionstore.recent_crashes") + 1;
-        Services.prefs.setIntPref("browser.sessionstore.recent_crashes", recentCrashes);
-        Services.prefs.savePrefFile(null);
-
-        if (recentCrashes > maxCrashes) {
-          throw "Exceeded maximum number of allowed restores";
-        }
-      }
-
       // Normally, we'll receive the session string from Java, but there are
       // cases where we may want to restore that Java cannot detect (e.g., if
       // browser.sessionstore.resume_session_once is true). In these cases, the