Bug 1394767 - Log an error and return in unexpected situations rather than calling NS_ASSERT and letting the subsequent code fail. r?mikedeboer draft
authorDão Gottwald <dao@mozilla.com>
Thu, 07 Sep 2017 13:45:08 +0200
changeset 660746 4613163345e93b5c0f81140d860efba2f0fb4f69
parent 660208 93dd2e456c0ecca00fb4d28744e88078a77deaf7
child 730336 43daa9b05e0bbbd7006dcc956977ad4ef43fd8da
push id78506
push userdgottwald@mozilla.com
push dateThu, 07 Sep 2017 11:45:36 +0000
reviewersmikedeboer
bugs1394767
milestone57.0a1
Bug 1394767 - Log an error and return in unexpected situations rather than calling NS_ASSERT and letting the subsequent code fail. r?mikedeboer Returning in onBrowserCrashed if the browser isn't remote breaks various tests, so I simply removed this seemingly bogus check. MozReview-Commit-ID: IoHhzdc2p7Y
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -153,17 +153,16 @@ const RESTORE_TAB_CONTENT_REASON = {
 };
 
 Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm", this);
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm", this);
 Cu.import("resource://gre/modules/TelemetryTimestamps.jsm", this);
 Cu.import("resource://gre/modules/Timer.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
-Cu.import("resource://gre/modules/debug.js", this);
 Cu.import("resource://gre/modules/osfile.jsm", this);
 
 XPCOMUtils.defineLazyServiceGetters(this, {
   gSessionStartup: ["@mozilla.org/browser/sessionstartup;1", "nsISessionStartup"],
   gScreenManager: ["@mozilla.org/gfx/screenmanager;1", "nsIScreenManager"],
   Telemetry: ["@mozilla.org/base/telemetry;1", "nsITelemetry"],
 });
 
@@ -2153,19 +2152,16 @@ var SessionStoreInternal = {
    * Handler for the event that is fired when a <xul:browser> crashes.
    *
    * @param aWindow
    *        The window that the crashed browser belongs to.
    * @param aBrowser
    *        The <xul:browser> that is now in the crashed state.
    */
   onBrowserCrashed(aBrowser) {
-    NS_ASSERT(aBrowser.isRemoteBrowser,
-              "Only remote browsers should be able to crash");
-
     this.enterCrashedState(aBrowser);
     // The browser crashed so we might never receive flush responses.
     // Resolve all pending flush requests for the crashed browser.
     TabStateFlusher.resolveAll(aBrowser);
   },
 
   /**
    * Called when a browser is showing or is about to show the tab
@@ -2891,17 +2887,22 @@ var SessionStoreInternal = {
    *
    * This method might be called multiple times before it has finished
    * flushing the browser tab. If that occurs, the loadArguments from
    * the most recent call to navigateAndRestore will be used once the
    * flush has finished.
    */
   navigateAndRestore(tab, loadArguments, historyIndex) {
     let window = tab.ownerGlobal;
-    NS_ASSERT(window.__SSi, "tab's window must be tracked");
+
+    if (!window.__SSi) {
+      Cu.reportError("Tab's window must be tracked.");
+      return;
+    }
+
     let browser = tab.linkedBrowser;
 
     // Were we already waiting for a flush from a previous call to
     // navigateAndRestore on this tab?
     let alreadyRestoring =
       this._remotenessChangingBrowsers.has(browser.permanentKey);
 
     // Stash the most recent loadArguments in this WeakMap so that
@@ -3639,21 +3640,24 @@ var SessionStoreInternal = {
       if (t != selectedIndex) {
         this.restoreTab(aTabs[t], aTabData[t]);
       }
     }
   },
 
   // Restores the given tab state for a given tab.
   restoreTab(tab, tabData, options = {}) {
-    NS_ASSERT(!tab.linkedBrowser.__SS_restoreState,
-              "must reset tab before calling restoreTab()");
+    let browser = tab.linkedBrowser;
+
+    if (browser.__SS_restoreState) {
+      Cu.reportError("Must reset tab before calling restoreTab.");
+      return;
+    }
 
     let loadArguments = options.loadArguments;
-    let browser = tab.linkedBrowser;
     let window = tab.ownerGlobal;
     let tabbrowser = window.gBrowser;
     let forceOnDemand = options.forceOnDemand;
 
     let willRestoreImmediately = options.restoreImmediately ||
                                  tabbrowser.selectedBrowser == browser;
 
     let isBrowserInserted = browser.isConnected;
@@ -4677,24 +4681,26 @@ var SessionStoreInternal = {
   /**
    * Reset the restoring state for a particular tab. This will be called when
    * removing a tab or when a tab needs to be reset (it's being overwritten).
    *
    * @param aTab
    *        The tab that will be "reset"
    */
   _resetLocalTabRestoringState(aTab) {
-    NS_ASSERT(aTab.linkedBrowser.__SS_restoreState,
-              "given tab is not restoring");
-
     let browser = aTab.linkedBrowser;
 
     // Keep the tab's previous state for later in this method
     let previousState = browser.__SS_restoreState;
 
+    if (!previousState) {
+      Cu.reportError("Given tab is not restoring.");
+      return;
+    }
+
     // The browser is no longer in any sort of restoring state.
     delete browser.__SS_restoreState;
 
     aTab.removeAttribute("pending");
 
     if (previousState == TAB_STATE_RESTORING) {
       if (this._tabsRestoringCount)
         this._tabsRestoringCount--;
@@ -4702,20 +4708,23 @@ var SessionStoreInternal = {
       // Make sure that the tab is removed from the list of tabs to restore.
       // Again, this is normally done in restoreTabContent, but that isn't being called
       // for this tab.
       TabRestoreQueue.remove(aTab);
     }
   },
 
   _resetTabRestoringState(tab) {
-    NS_ASSERT(tab.linkedBrowser.__SS_restoreState,
-              "given tab is not restoring");
-
     let browser = tab.linkedBrowser;
+
+    if (!browser.__SS_restoreState) {
+      Cu.reportError("Given tab is not restoring.");
+      return;
+    }
+
     browser.messageManager.sendAsyncMessage("SessionStore:resetRestore", {});
     this._resetLocalTabRestoringState(tab);
   },
 
   /**
    * Each fresh tab starts out with epoch=0. This function can be used to
    * start a next epoch by incrementing the current value. It will enables us
    * to ignore stale messages sent from previous epochs. The function returns