Bug 490040 - Reattaching a lone tab into another window causes an empty window to be added to Recently Closed Windows, r=zeniko

browser/components/sessionstore/src/nsSessionStore.js

@@ -511,7 +511,9 @@ SessionStoreService.prototype = {
           this.onTabAdd(aEvent.currentTarget.ownerDocument.defaultView, tabpanel);
         }
         else {
-          this.onTabClose(aEvent.currentTarget.ownerDocument.defaultView, aEvent.originalTarget);
+          // aEvent.detail determines if the tab was closed by moving to a different window
+          if (!aEvent.detail)
+            this.onTabClose(aEvent.currentTarget.ownerDocument.defaultView, aEvent.originalTarget);
           this.onTabRemove(aEvent.currentTarget.ownerDocument.defaultView, tabpanel);
         }
         break;
@@ -640,9 +642,12 @@ SessionStoreService.prototype = {
         this._updateCookies([winData]);
       }
       
-      // store closed-window data for undo
-      this._closedWindows.unshift(winData);
-      this._capClosedWindows();
+      // save the window if it has multiple tabs or a single tab with entries
+      if (winData.tabs.length > 1 ||
+          (winData.tabs.length == 1 && winData.tabs[0].entries.length > 0)) {
+        this._closedWindows.unshift(winData);
+        this._capClosedWindows();
+      }
       
       // clear this window from the list
       delete this._windows[aWindow.__SSi];

browser/components/sessionstore/test/browser/Makefile.in

@@ -86,6 +86,7 @@ _BROWSER_TEST_FILES = \
 	browser_485482.js \
 	browser_485482_sample.html \
 	browser_485563.js \
+	browser_490040.js \
 	$(NULL)
 
 libs:: $(_BROWSER_TEST_FILES)

browser/components/sessionstore/test/browser/browser_394759.js

@@ -141,6 +141,10 @@ function test() {
       let window = openDialog(location, "_blank", settings, url);
       window.addEventListener("load", function(aEvent) {
         window.gBrowser.addEventListener("load", function(aEvent) {
+          // the window _should_ have state with a tab of url, but it doesn't
+          // always happend before window.close(). addTab ensure we don't treat
+          // this window as a stateless window
+          window.gBrowser.addTab();
           window.gBrowser.removeEventListener("load", arguments.callee, true);
           executeSoon(function() {
             window.close();

browser/components/sessionstore/test/browser/browser_490040.js

@@ -0,0 +1,134 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is sessionstore test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2008
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ * Paul O’Shannessy <paul@oshannessy.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+function test() {
+  /** Test for Bug 490040 **/
+
+  let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
+  let os = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
+
+  waitForExplicitFinish();
+
+  function testWithState(aState, aCallback) {
+    // ensure we can store the window if need be
+    let curClosedWindowCount = ss.getClosedWindowCount();
+    gPrefService.setIntPref("browser.sessionstore.max_windows_undo", curClosedWindowCount + 1);
+
+    let theWin = openDialog(location, "_blank", "chrome,all,dialog=no");
+    theWin.addEventListener("load", function(aEvent) {
+      theWin.gBrowser.removeEventListener("load", arguments.callee, true);
+
+      ss.setWindowState(theWin, JSON.stringify(aState.windowState), true);
+
+      let observer = {
+        QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
+                                               Ci.nsISupportsWeakReference]),
+        observe: function(aSubject, aTopic, aData) {
+          let _this = this;
+          // use executeSoon to ensure this happens after SS observer
+          executeSoon(function() {
+            is(ss.getClosedWindowCount(), curClosedWindowCount + (aState.shouldBeAdded ? 1 : 0),
+               "That window should " + (aState.shouldBeAdded ? "" : "not ") + "be restorable");
+            os.removeObserver(_this, "domwindowclosed");
+            executeSoon(aCallback);
+          });
+        }
+      };
+      os.addObserver(observer, "domwindowclosed", true);
+
+      theWin.gBrowser.addEventListener("load", function() {
+        theWin.gBrowser.removeEventListener("load", arguments.callee, true);
+        theWin.close();
+      }, true);
+    }, true);
+  }
+
+  // Only windows with open tabs are restorable. Windows where a lone tab is
+  // detached may have _closedTabs, but is left with just an empty tab.
+  let states = [
+    {
+      shouldBeAdded: true,
+      windowState: {
+        windows: [{
+          tabs: [{ entries: [{ url: "http://example.com", title: "example.com" }] }],
+          selected: 1,
+          _closedTabs: []
+        }]
+      }
+    },
+    {
+      shouldBeAdded: false,
+      windowState: {
+        windows: [{
+          tabs: [{ entries: [] }],
+          _closedTabs: []
+        }]
+      }
+    },
+    {
+      shouldBeAdded: false,
+      windowState: {
+        windows: [{
+          tabs: [{ entries: [] }],
+          _closedTabs: [{ state: { entries: [{ url: "http://example.com", index: 1 }] } }]
+        }]
+      }
+    },
+    {
+      shouldBeAdded: false,
+      windowState: {
+        windows: [{
+          tabs: [{ entries: [] }],
+          _closedTabs: [],
+          extData: { keyname: "pi != " + Math.random() }
+        }]
+      }
+    }
+  ];
+
+  testWithState(states[0], function() {
+    testWithState(states[1], function() {
+      testWithState(states[2], function() {
+        testWithState(states[3], function() {
+          gPrefService.clearUserPref("browser.sessionstore.max_windows_undo");
+          finish();
+        });
+      });
+    });
+  });
+}
+