Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote. r?mikedeboer draft
authorMike Conley <mconley@mozilla.com>
Tue, 09 Aug 2016 13:32:21 -0400
changeset 398800 570439017aabbd92a0e5e758a223e225bf05ec07
parent 397822 e78975b53563d80c99ebfbdf8a9fbf6b829a8a48
child 527749 6e14130ec16a71b7a9f33ac8706c39a372924b5a
push id25632
push usermconley@mozilla.com
push dateTue, 09 Aug 2016 18:57:23 +0000
reviewersmikedeboer
bugs1290280
milestone51.0a1
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote. r?mikedeboer The code that checks to see whether or not we should flip the remoteness of a browser before loading the session state into it wasn't accounting for the fact that oftentimes, restoreImmediately isn't included, so it's undefined, which coerces to "false-y". This caused us to very quickly destroy a TabParent, very soon after creating it. In some cases, the IPC layer seems to not like that, and throws an OnChannelError, which causes the TabParent ActorDestroy method to be called with an abnormal shutdown reason, which causes the tab crash observer to fire, which bubbles the tab crash event. We should probably make the IPC layer more resilient to this sort of thing, but we should also probably not flip remoteness when we really don't need to. I've removed the restoreImmediately check, and just use forceOnDemand to determine whether or not we should force the browser into the non-remote state. MozReview-Commit-ID: 5AmPHvzDZlX
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -3622,40 +3622,37 @@ var SessionStoreInternal = {
    *
    *        tabbrowser (<xul:tabbrowser>):
    *          The tabbrowser that the browser belongs to.
    *
    *        tabData (object):
    *          The tabData state that we're attempting to
    *          restore for the tab.
    *
-   *        restoreImmediately (bool):
-   *          true if loading of content should occur immediately
-   *          after restoration.
-   *
    *        forceOnDemand (bool):
    *          true if the tab is being put into the restore-on-demand
    *          background state.
    */
   _maybeUpdateBrowserRemoteness({ browser, tabbrowser, tabData,
-                                  restoreImmediately, forceOnDemand }) {
+                                  forceOnDemand }) {
     // If the browser we're attempting to restore happens to be
     // remote, we need to flip it back to non-remote if it's going
     // to go into the pending background tab state. This is to make
-    // sure that the background tab can't crash if it hasn't yet
+    // sure that a background tab can't crash if it hasn't yet
     // been restored. Normally, when a window is restored, the tabs
     // that SessionStore inserts are non-remote - but the initial
     // browser is, by default, remote, so this check and flip is
     // mostly for that case.
     //
-    // Note that pinned tabs are exempt from this flip, since
-    // they'll be loaded immediately anyways.
+    // Note that selected tabs and pinned tabs are exempt from
+    // this flip, since they'll be loaded immediately anyways.
     if (browser.isRemoteBrowser &&
         !tabData.pinned &&
-        (!restoreImmediately || forceOnDemand)) {
+        tabbrowser.selectedBrowser != browser &&
+        forceOnDemand) {
       tabbrowser.updateBrowserRemoteness(browser, false);
     }
   },
 
   /**
    * Update the session start time and send a telemetry measurement
    * for the number of days elapsed since the session was started.
    *