Backed out changeset b821fc0de45c (bug 1543684) for failing browser_354894_perwindowpb.js on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Mon, 06 May 2019 19:52:40 +0300
changeset 531532 335579594d1cdf26497b50ffa46dbe7ff16a6896
parent 531531 2f3479842f9101fe2466775eaf777c728c289839
child 531533 2f0db0b04a913275f91ede9fc984a40065c08266
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1543684
milestone68.0a1
backs outb821fc0de45c129f8daebbcff0ee885cd97bd45f
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset b821fc0de45c (bug 1543684) for failing browser_354894_perwindowpb.js on a CLOSED TREE
browser/components/sessionstore/SessionStartup.jsm
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStartup.jsm
+++ b/browser/components/sessionstore/SessionStartup.jsm
@@ -49,20 +49,19 @@ const STATE_RUNNING_STR = "running";
 const TYPE_NO_SESSION = 0;
 const TYPE_RECOVER_SESSION = 1;
 const TYPE_RESUME_SESSION = 2;
 const TYPE_DEFER_SESSION = 3;
 
 // 'browser.startup.page' preference value to resume the previous session.
 const BROWSER_STARTUP_RESUME_SESSION = 3;
 
-function warning(msg, exception) {
+function warning(aMsg, aException) {
   let consoleMsg = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
-  consoleMsg.init(msg, exception.fileName, null, exception.lineNumber, 0, Ci.nsIScriptError.warningFlag,
-    "component javascript");
+consoleMsg.init(aMsg, aException.fileName, null, aException.lineNumber, 0, Ci.nsIScriptError.warningFlag, "component javascript");
   Services.console.logMessage(consoleMsg);
 }
 
 var gOnceInitializedDeferred = (function() {
   let deferred = {};
 
   deferred.promise = new Promise((resolve, reject) => {
     deferred.resolve = resolve;
@@ -75,32 +74,32 @@ var gOnceInitializedDeferred = (function
 /* :::::::: The Service ::::::::::::::: */
 
 var SessionStartup = {
   NO_SESSION: TYPE_NO_SESSION,
   RECOVER_SESSION: TYPE_RECOVER_SESSION,
   RESUME_SESSION: TYPE_RESUME_SESSION,
   DEFER_SESSION: TYPE_DEFER_SESSION,
 
-  // The state to restore at startup.
+  // the state to restore at startup
   _initialState: null,
   _sessionType: TYPE_NO_SESSION,
   _initialized: false,
 
   // Stores whether the previous session crashed.
   _previousSessionCrashed: null,
 
   _resumeSessionEnabled: null,
 
 /* ........ Global Event Handlers .............. */
 
   /**
    * Initialize the component
    */
-  init() {
+  init: function sss_init() {
     Services.obs.notifyObservers(null, "sessionstore-init-started");
     StartupPerformance.init();
 
     // do not need to initialize anything in auto-started private browsing sessions
     if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
       this._initialized = true;
       gOnceInitializedDeferred.resolve();
       return;
@@ -112,32 +111,36 @@ var SessionStartup = {
         // but we aren't automatically started by the OS (or else appinfo.restartedByOS
         // would have been set). Therefore we should clear resume_session_once
         // to avoid forcing a resume for a normal startup.
         Services.prefs.setBoolPref("browser.sessionstore.resume_session_once", false);
       }
       Services.prefs.setBoolPref("browser.sessionstore.resuming_after_os_restart", false);
     }
 
+    this._resumeSessionEnabled =
+      Services.prefs.getBoolPref("browser.sessionstore.resume_session_once") ||
+      Services.prefs.getIntPref("browser.startup.page") == BROWSER_STARTUP_RESUME_SESSION;
+
     SessionFile.read().then(
       this._onSessionFileRead.bind(this),
       console.error
     );
   },
 
-  // Wrap a string as a nsISupports.
-  _createSupportsString(data) {
+  // Wrap a string as a nsISupports
+  _createSupportsString: function ssfi_createSupportsString(aData) {
     let string = Cc["@mozilla.org/supports-string;1"]
                    .createInstance(Ci.nsISupportsString);
-    string.data = data;
+    string.data = aData;
     return string;
   },
 
   /**
-   * Complete initialization once the Session File has been read.
+   * Complete initialization once the Session File has been read
    *
    * @param source The Session State string read from disk.
    * @param parsed The object obtained by parsing |source| as JSON.
    */
   _onSessionFileRead({source, parsed, noFilesFound}) {
     this._initialized = true;
 
     // Let observers modify the state before it is used
@@ -172,35 +175,35 @@ var SessionStartup = {
       let pinnedTabCount = initialState.windows.reduce((winAcc, win) => {
         return winAcc + win.tabs.reduce((tabAcc, tab) => {
           return tabAcc + (tab.pinned ? 1 : 0);
         }, 0);
       }, 0);
       Services.telemetry.scalarSet("browser.engagement.restored_pinned_tabs_count", pinnedTabCount);
     }, 60000);
 
-    // If this is a normal restore then throw away any previous session.
-    if (!this.isAutomaticRestoreEnabled() && this._initialState) {
+    // If this is a normal restore then throw away any previous session
+    if (!this._resumeSessionEnabled && this._initialState) {
       delete this._initialState.lastSessionState;
     }
 
     let resumeFromCrash = Services.prefs.getBoolPref("browser.sessionstore.resume_from_crash");
 
     CrashMonitor.previousCheckpoints.then(checkpoints => {
       if (checkpoints) {
         // If the previous session finished writing the final state, we'll
         // assume there was no crash.
         this._previousSessionCrashed = !checkpoints["sessionstore-final-state-write-complete"];
       } else if (noFilesFound) {
         // If the Crash Monitor could not load a checkpoints file it will
         // provide null. This could occur on the first run after updating to
         // a version including the Crash Monitor, or if the checkpoints file
         // was removed, or on first startup with this profile, or after Firefox Reset.
 
-        // There was no checkpoints file and no sessionstore.js or its backups,
+        // There was no checkpoints file and no sessionstore.js or its backups
         // so we will assume that this was a fresh profile.
         this._previousSessionCrashed = false;
       } else {
         // If this is the first run after an update, sessionstore.js should
         // still contain the session.state flag to indicate if the session
         // crashed. If it is not present, we will assume this was not the first
         // run after update and the checkpoints file was somehow corrupted or
         // removed by a crash.
@@ -215,54 +218,53 @@ var SessionStartup = {
           (this._initialState.session.state == STATE_RUNNING_STR);
       }
 
       // Report shutdown success via telemetry. Shortcoming here are
       // being-killed-by-OS-shutdown-logic, shutdown freezing after
       // session restore was written, etc.
       Services.telemetry.getHistogramById("SHUTDOWN_OK").add(!this._previousSessionCrashed);
 
-      // Set the startup type.
-      if (this.isAutomaticRestoreEnabled()) {
-        this._sessionType = this.RESUME_SESSION;
-      } else if (this._previousSessionCrashed && resumeFromCrash) {
+      // set the startup type
+      if (this._previousSessionCrashed && resumeFromCrash)
         this._sessionType = this.RECOVER_SESSION;
-      } else if (this._initialState) {
+      else if (!this._previousSessionCrashed && this._resumeSessionEnabled)
+        this._sessionType = this.RESUME_SESSION;
+      else if (this._initialState)
         this._sessionType = this.DEFER_SESSION;
-      } else {
-        this._initialState = null; // Reset the state.
-      }
+      else
+        this._initialState = null; // reset the state
       Services.obs.addObserver(this, "sessionstore-windows-restored", true);
 
       if (this._sessionType != this.NO_SESSION)
         Services.obs.addObserver(this, "browser:purge-session-history", true);
 
       // We're ready. Notify everyone else.
       Services.obs.notifyObservers(null, "sessionstore-state-finalized");
 
       gOnceInitializedDeferred.resolve();
     });
   },
 
   /**
    * Handle notifications
    */
-  observe(subject, topic, data) {
-    switch (topic) {
-      case "sessionstore-windows-restored":
-        Services.obs.removeObserver(this, "sessionstore-windows-restored");
-        // Free _initialState after nsSessionStore is done with it.
-        this._initialState = null;
-        this._didRestore = true;
-        break;
-      case "browser:purge-session-history":
-        Services.obs.removeObserver(this, "browser:purge-session-history");
-        // Reset all state on sanitization.
-        this._sessionType = this.NO_SESSION;
-        break;
+  observe: function sss_observe(aSubject, aTopic, aData) {
+    switch (aTopic) {
+    case "sessionstore-windows-restored":
+      Services.obs.removeObserver(this, "sessionstore-windows-restored");
+      // free _initialState after nsSessionStore is done with it
+      this._initialState = null;
+      this._didRestore = true;
+      break;
+    case "browser:purge-session-history":
+      Services.obs.removeObserver(this, "browser:purge-session-history");
+      // reset all state on sanitization
+      this._sessionType = this.NO_SESSION;
+      break;
     }
   },
 
 /* ........ Public API ................*/
 
   get onceInitialized() {
     return gOnceInitializedDeferred.promise;
   },
@@ -270,77 +272,76 @@ var SessionStartup = {
   /**
    * Get the session state as a jsval
    */
   get state() {
     return this._initialState;
   },
 
   /**
+   * Determines whether there is a pending session restore. Should only be
+   * called after initialization has completed.
+   * @returns bool
+   */
+  doRestore: function sss_doRestore() {
+    return this._willRestore();
+  },
+
+  /**
    * Determines whether automatic session restoration is enabled for this
    * launch of the browser. This does not include crash restoration. In
    * particular, if session restore is configured to restore only in case of
    * crash, this method returns false.
    * @returns bool
    */
   isAutomaticRestoreEnabled() {
-    if (this._resumeSessionEnabled === null) {
-      this._resumeSessionEnabled = !PrivateBrowsingUtils.permanentPrivateBrowsing &&
-        (Services.prefs.getBoolPref("browser.sessionstore.resume_session_once") ||
-         Services.prefs.getIntPref("browser.startup.page") == BROWSER_STARTUP_RESUME_SESSION);
+    if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
+      return false;
     }
 
-    return this._resumeSessionEnabled;
+    return Services.prefs.getBoolPref("browser.sessionstore.resume_session_once") ||
+           Services.prefs.getIntPref("browser.startup.page") == BROWSER_STARTUP_RESUME_SESSION;
   },
 
   /**
    * Determines whether there is a pending session restore.
    * @returns bool
    */
-  willRestore() {
+  _willRestore() {
     return this._sessionType == this.RECOVER_SESSION ||
            this._sessionType == this.RESUME_SESSION;
   },
 
   /**
-   * Determines whether there is a pending session restore and if that will refer
-   * back to a crash.
-   * @returns bool
-   */
-  willRestoreAsCrashed() {
-    return this._sessionType == this.RECOVER_SESSION;
-  },
-
-  /**
    * Returns a boolean or a promise that resolves to a boolean, indicating
    * whether we will restore a session that ends up replacing the homepage.
    * True guarantees that we'll restore a session; false means that we
    * /probably/ won't do so.
    * The browser uses this to avoid unnecessarily loading the homepage when
    * restoring a session.
    */
   get willOverrideHomepage() {
     // If the session file hasn't been read yet and resuming the session isn't
     // enabled via prefs, go ahead and load the homepage. We may still replace
     // it when recovering from a crash, which we'll only know after reading the
     // session file, but waiting for that would delay loading the homepage in
     // the non-crash case.
-    if (!this._initialState && !this.isAutomaticRestoreEnabled()) {
+    if (!this._initialState && !this._resumeSessionEnabled) {
       return false;
     }
     // If we've already restored the session, we won't override again.
     if (this._didRestore) {
       return false;
     }
 
     return new Promise(resolve => {
       this.onceInitialized.then(() => {
         // If there are valid windows with not only pinned tabs, signal that we
         // will override the default homepage by restoring a session.
-        resolve(this.willRestore() &&
+        resolve(this._willRestore() &&
                 this._initialState &&
                 this._initialState.windows &&
                 this._initialState.windows.some(w => w.tabs.some(t => !t.pinned)));
       });
     });
   },
 
   /**
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -638,17 +638,17 @@ var SessionStoreInternal = {
   /**
    * Initialize the session using the state provided by SessionStartup
    */
   initSession() {
     TelemetryStopwatch.start("FX_SESSION_RESTORE_STARTUP_INIT_SESSION_MS");
     let state;
     let ss = SessionStartup;
 
-    if (ss.willRestore() ||
+    if (ss.doRestore() ||
         ss.sessionType == ss.DEFER_SESSION) {
       state = ss.state;
     }
 
     if (state) {
       try {
         // If we're doing a DEFERRED session, then we want to pull pinned tabs
         // out so they can be restored.
@@ -669,17 +669,17 @@ var SessionStoreInternal = {
           if (remainingState.windows.length) {
             LastSession.setState(remainingState);
           }
         } else {
           // Get the last deferred session in case the user still wants to
           // restore it
           LastSession.setState(state.lastSessionState);
 
-          if (ss.willRestoreAsCrashed()) {
+          if (ss.previousSessionCrashed) {
             this._recentCrashes = (state.session &&
                                    state.session.recentCrashes || 0) + 1;
 
             if (this._needsRestorePage(state, this._recentCrashes)) {
               // replace the crashed session with a restore-page-only session
               let url = "about:sessionrestore";
               let formdata = {id: {sessionData: state}, url};
               let entry = {url, triggeringPrincipal_base64: E10SUtils.SERIALIZED_SYSTEMPRINCIPAL };
@@ -1219,17 +1219,17 @@ var SessionStoreInternal = {
           closedWindowState = this._closedWindows[i];
           closedWindowIndex = i;
           break;
         }
       }
 
       if (closedWindowState) {
         let newWindowState;
-        if (AppConstants.platform == "macosx" || !SessionStartup.willRestore()) {
+        if (AppConstants.platform == "macosx" || !this._doResumeSession()) {
           // We want to split the window up into pinned tabs and unpinned tabs.
           // Pinned tabs should be restored. If there are any remaining tabs,
           // they should be added back to _closedWindows.
           // We'll cheat a little bit and reuse _prepDataForDeferredRestore
           // even though it wasn't built exactly for this.
           let [appTabsState, normalTabsState] =
             this._prepDataForDeferredRestore({ windows: [closedWindowState] });
 
@@ -4665,16 +4665,25 @@ var SessionStoreInternal = {
 
     this._updateWindowRestoreState(window, aState);
     WINDOW_SHOWING_PROMISES.set(window, PromiseUtils.defer());
 
     return window;
   },
 
   /**
+   * Whether or not to resume session, if not recovering from a crash.
+   * @returns bool
+   */
+  _doResumeSession: function ssi_doResumeSession() {
+    return this._prefBranch.getIntPref("startup.page") == 3 ||
+           this._prefBranch.getBoolPref("sessionstore.resume_session_once");
+  },
+
+  /**
    * whether the user wants to load any other page at startup
    * (except the homepage) - needed for determining whether to overwrite the current tabs
    * C.f.: nsBrowserContentHandler's defaultArgs implementation.
    * @returns bool
    */
   _isCmdLineEmpty: function ssi_isCmdLineEmpty(aWindow, aState) {
     var pinnedOnly = aState.windows &&
                      aState.windows.every(win =>