Bug 1309702 - Update getClosedTabData and getClosedWindowData in SessionStore.jsm to not return JSON, r?mikedeboer draft
authorBob Silverberg <bsilverberg@mozilla.com>
Fri, 14 Oct 2016 07:12:18 -0400
changeset 425242 7f4e73e73631ff577b9c6f46b47b4d3746a2926f
parent 423604 7ae377917236b7e6111146aa9fb4c073c0efc7f4
child 425284 4eaa89881e2d5e64df4049105ac9ec0b6b6b7da9
child 425285 9e19ed49a2a23411262ee8fc37faf055540115dc
child 425286 f2b0fed159b5f7a5fe46d3c326c3dfa1d16963ce
push id32377
push userbmo:bob.silverberg@gmail.com
push dateFri, 14 Oct 2016 11:13:22 +0000
reviewersmikedeboer
bugs1309702
milestone52.0a1
Bug 1309702 - Update getClosedTabData and getClosedWindowData in SessionStore.jsm to not return JSON, r?mikedeboer MozReview-Commit-ID: 9SevLkTp0G7
browser/components/sessionstore/RecentlyClosedTabsAndWindowsMenuUtils.jsm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser_394759_basic.js
browser/components/sessionstore/test/browser_461634.js
--- a/browser/components/sessionstore/RecentlyClosedTabsAndWindowsMenuUtils.jsm
+++ b/browser/components/sessionstore/RecentlyClosedTabsAndWindowsMenuUtils.jsm
@@ -12,19 +12,20 @@ var Cr = Components.results;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
                                   "resource://gre/modules/PluralForm.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "SessionStore",
+                                  "resource:///modules/sessionstore/SessionStore.jsm");
 
 var navigatorBundle = Services.strings.createBundle("chrome://browser/locale/browser.properties");
-var ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
 
 this.RecentlyClosedTabsAndWindowsMenuUtils = {
 
   /**
   * Builds up a document fragment of UI items for the recently closed tabs.
   * @param   aWindow
   *          The window that the tabs were closed in.
   * @param   aTagName
@@ -35,18 +36,18 @@ this.RecentlyClosedTabsAndWindowsMenuUti
   * @param   aRestoreAllLabel (defaults to "menuRestoreAllTabs.label")
   *          Which localizable string to use for the 'restore all tabs' item.
   * @returns A document fragment with UI items for each recently closed tab.
   */
   getTabsFragment: function(aWindow, aTagName, aPrefixRestoreAll=false,
                             aRestoreAllLabel="menuRestoreAllTabs.label") {
     let doc = aWindow.document;
     let fragment = doc.createDocumentFragment();
-    if (ss.getClosedTabCount(aWindow) != 0) {
-      let closedTabs = JSON.parse(ss.getClosedTabData(aWindow));
+    if (SessionStore.getClosedTabCount(aWindow) != 0) {
+      let closedTabs = SessionStore.getClosedTabData(aWindow, false);
       for (let i = 0; i < closedTabs.length; i++) {
         let element = doc.createElementNS(kNSXUL, aTagName);
         element.setAttribute("label", closedTabs[i].title);
         if (closedTabs[i].image) {
           setImage(aWindow, closedTabs[i], element);
         }
         element.setAttribute("value", i);
         if (aTagName == "menuitem") {
@@ -94,17 +95,17 @@ this.RecentlyClosedTabsAndWindowsMenuUti
   *          Whether the 'restore all windows' item is suffixed or prefixed to the list.
   *          If suffixed (the default) a separator will be inserted before it.
   * @param   aRestoreAllLabel (defaults to "menuRestoreAllWindows.label")
   *          Which localizable string to use for the 'restore all windows' item.
   * @returns A document fragment with UI items for each recently closed window.
   */
   getWindowsFragment: function(aWindow, aTagName, aPrefixRestoreAll=false,
                             aRestoreAllLabel="menuRestoreAllWindows.label") {
-    let closedWindowData = JSON.parse(ss.getClosedWindowData());
+    let closedWindowData = SessionStore.getClosedWindowData(false);
     let fragment = aWindow.document.createDocumentFragment();
     if (closedWindowData.length != 0) {
       let menuLabelString = navigatorBundle.GetStringFromName("menuUndoCloseWindowLabel");
       let menuLabelStringSingleTab =
         navigatorBundle.GetStringFromName("menuUndoCloseWindowSingleTabLabel");
 
       let doc = aWindow.document;
       for (let i = 0; i < closedWindowData.length; i++) {
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -241,34 +241,34 @@ this.SessionStore = {
   duplicateTab: function ss_duplicateTab(aWindow, aTab, aDelta = 0) {
     return SessionStoreInternal.duplicateTab(aWindow, aTab, aDelta);
   },
 
   getClosedTabCount: function ss_getClosedTabCount(aWindow) {
     return SessionStoreInternal.getClosedTabCount(aWindow);
   },
 
-  getClosedTabData: function ss_getClosedTabDataAt(aWindow) {
-    return SessionStoreInternal.getClosedTabData(aWindow);
+  getClosedTabData: function ss_getClosedTabData(aWindow, aAsString = true) {
+    return SessionStoreInternal.getClosedTabData(aWindow, aAsString);
   },
 
   undoCloseTab: function ss_undoCloseTab(aWindow, aIndex) {
     return SessionStoreInternal.undoCloseTab(aWindow, aIndex);
   },
 
   forgetClosedTab: function ss_forgetClosedTab(aWindow, aIndex) {
     return SessionStoreInternal.forgetClosedTab(aWindow, aIndex);
   },
 
   getClosedWindowCount: function ss_getClosedWindowCount() {
     return SessionStoreInternal.getClosedWindowCount();
   },
 
-  getClosedWindowData: function ss_getClosedWindowData() {
-    return SessionStoreInternal.getClosedWindowData();
+  getClosedWindowData: function ss_getClosedWindowData(aAsString = true) {
+    return SessionStoreInternal.getClosedWindowData(aAsString);
   },
 
   undoCloseWindow: function ss_undoCloseWindow(aIndex) {
     return SessionStoreInternal.undoCloseWindow(aIndex);
   },
 
   forgetClosedWindow: function ss_forgetClosedWindow(aIndex) {
     return SessionStoreInternal.forgetClosedWindow(aIndex);
@@ -2179,27 +2179,29 @@ var SessionStoreInternal = {
 
     if (!DyingWindowCache.has(aWindow)) {
       throw Components.Exception("Window is not tracked", Cr.NS_ERROR_INVALID_ARG);
     }
 
     return DyingWindowCache.get(aWindow)._closedTabs.length;
   },
 
-  getClosedTabData: function ssi_getClosedTabDataAt(aWindow) {
+  getClosedTabData: function ssi_getClosedTabData(aWindow, aAsString = true) {
     if ("__SSi" in aWindow) {
-      return JSON.stringify(this._windows[aWindow.__SSi]._closedTabs);
+      return aAsString ?
+        JSON.stringify(this._windows[aWindow.__SSi]._closedTabs) :
+        Cu.cloneInto(this._windows[aWindow.__SSi]._closedTabs, {});
     }
 
     if (!DyingWindowCache.has(aWindow)) {
       throw Components.Exception("Window is not tracked", Cr.NS_ERROR_INVALID_ARG);
     }
 
     let data = DyingWindowCache.get(aWindow);
-    return JSON.stringify(data._closedTabs);
+    return aAsString ? JSON.stringify(data._closedTabs) : Cu.cloneInto(data._closedTabs, {});
   },
 
   undoCloseTab: function ssi_undoCloseTab(aWindow, aIndex) {
     if (!aWindow.__SSi) {
       throw Components.Exception("Window is not tracked", Cr.NS_ERROR_INVALID_ARG);
     }
 
     var closedTabs = this._windows[aWindow.__SSi]._closedTabs;
@@ -2245,18 +2247,18 @@ var SessionStoreInternal = {
     // remove closed tab from the array
     this.removeClosedTabData(closedTabs, aIndex);
   },
 
   getClosedWindowCount: function ssi_getClosedWindowCount() {
     return this._closedWindows.length;
   },
 
-  getClosedWindowData: function ssi_getClosedWindowData() {
-    return JSON.stringify(this._closedWindows);
+  getClosedWindowData: function ssi_getClosedWindowData(aAsString = true) {
+    return aAsString ? JSON.stringify(this._closedWindows) : Cu.cloneInto(this._closedWindows, {});
   },
 
   undoCloseWindow: function ssi_undoCloseWindow(aIndex) {
     if (!(aIndex in this._closedWindows)) {
       throw Components.Exception("Invalid index: not in the closed windows", Cr.NS_ERROR_INVALID_ARG);
     }
 
     // reopen the window
--- a/browser/components/sessionstore/test/browser_394759_basic.js
+++ b/browser/components/sessionstore/test/browser_394759_basic.js
@@ -2,16 +2,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const TEST_URL = "data:text/html;charset=utf-8,<input%20id=txt>" +
                  "<input%20type=checkbox%20id=chk>";
 
+Cu.import("resource:///modules/sessionstore/SessionStore.jsm");
+
 /**
  * This test ensures that closing a window is a reversible action. We will
  * close the the window, restore it and check that all data has been restored.
  * This includes window-specific data as well as form data for tabs.
  */
 function test() {
   waitForExplicitFinish();
 
@@ -20,63 +22,69 @@ function test() {
   let uniqueText = "pi != " + Math.random();
 
   // Clear the list of closed windows.
   forgetClosedWindows();
 
   provideWindow(function onTestURLLoaded(newWin) {
     newWin.gBrowser.addTab().linkedBrowser.stop();
 
-    // mark the window with some unique data to be restored later on
+    // Mark the window with some unique data to be restored later on.
     ss.setWindowValue(newWin, uniqueKey, uniqueValue);
     let [txt, chk] = newWin.content.document.querySelectorAll("#txt, #chk");
     txt.value = uniqueText;
 
     let browser = newWin.gBrowser.selectedBrowser;
     setInputChecked(browser, {id: "chk", checked: true}).then(() => {
       BrowserTestUtils.closeWindow(newWin).then(() => {
         is(ss.getClosedWindowCount(), 1,
            "The closed window was added to Recently Closed Windows");
-        let data = JSON.parse(ss.getClosedWindowData())[0];
-        ok(data.title == TEST_URL && JSON.stringify(data).indexOf(uniqueText) > -1,
+
+        let data = SessionStore.getClosedWindowData(false);
+
+        // Verify that non JSON serialized data is the same as JSON serialized data.
+        is(JSON.stringify(data), ss.getClosedWindowData(),
+           "Non-serialized data is the same as serialized data")
+
+        ok(data[0].title == TEST_URL && JSON.stringify(data[0]).indexOf(uniqueText) > -1,
            "The closed window data was stored correctly");
 
-        // reopen the closed window and ensure its integrity
+        // Reopen the closed window and ensure its integrity.
         let newWin2 = ss.undoCloseWindow(0);
 
         ok(newWin2 instanceof ChromeWindow,
            "undoCloseWindow actually returned a window");
         is(ss.getClosedWindowCount(), 0,
            "The reopened window was removed from Recently Closed Windows");
 
-        // SSTabRestored will fire more than once, so we need to make sure we count them
+        // SSTabRestored will fire more than once, so we need to make sure we count them.
         let restoredTabs = 0;
-        let expectedTabs = data.tabs.length;
+        let expectedTabs = data[0].tabs.length;
         newWin2.addEventListener("SSTabRestored", function sstabrestoredListener(aEvent) {
           ++restoredTabs;
           info("Restored tab " + restoredTabs + "/" + expectedTabs);
           if (restoredTabs < expectedTabs) {
             return;
           }
 
-          is(restoredTabs, expectedTabs, "correct number of tabs restored");
+          is(restoredTabs, expectedTabs, "Correct number of tabs restored");
           newWin2.removeEventListener("SSTabRestored", sstabrestoredListener, true);
 
           is(newWin2.gBrowser.tabs.length, 2,
              "The window correctly restored 2 tabs");
           is(newWin2.gBrowser.currentURI.spec, TEST_URL,
              "The window correctly restored the URL");
 
           let [txt, chk] = newWin2.content.document.querySelectorAll("#txt, #chk");
           ok(txt.value == uniqueText && chk.checked,
              "The window correctly restored the form");
           is(ss.getWindowValue(newWin2, uniqueKey), uniqueValue,
              "The window correctly restored the data associated with it");
 
-          // clean up
+          // Clean up.
           BrowserTestUtils.closeWindow(newWin2).then(finish);
         }, true);
       });
     });
   }, TEST_URL);
 }
 
 function setInputChecked(browser, data) {
--- a/browser/components/sessionstore/test/browser_461634.js
+++ b/browser/components/sessionstore/test/browser_461634.js
@@ -1,12 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+Cu.import("resource:///modules/sessionstore/SessionStore.jsm");
+
 function test() {
   /** Test for Bug 461634 **/
 
   waitForExplicitFinish();
 
   const REMEMBER = Date.now(), FORGET = Math.random();
   let test_state = { windows: [{ "tabs": [{ "entries": [] }], _closedTabs: [
     { state: { entries: [{ url: "http://www.example.net/" }] }, title: FORGET },
@@ -25,49 +27,59 @@ function test() {
       aFunction();
       return false;
     }
     catch (ex) {
       return ex.name == "NS_ERROR_ILLEGAL_VALUE";
     }
   }
 
-  // open a window and add the above closed tab list
+  // Open a window and add the above closed tab list.
   let newWin = openDialog(location, "", "chrome,all,dialog=no");
   promiseWindowLoaded(newWin).then(() => {
     gPrefService.setIntPref("browser.sessionstore.max_tabs_undo",
                             test_state.windows[0]._closedTabs.length);
     ss.setWindowState(newWin, JSON.stringify(test_state), true);
 
-    let closedTabs = JSON.parse(ss.getClosedTabData(newWin));
+    let closedTabs = SessionStore.getClosedTabData(newWin, false);
+
+    // Verify that non JSON serialized data is the same as JSON serialized data.
+    is(JSON.stringify(closedTabs), SessionStore.getClosedTabData(newWin),
+       "Non-serialized data is the same as serialized data")
+
     is(closedTabs.length, test_state.windows[0]._closedTabs.length,
        "Closed tab list has the expected length");
     is(countByTitle(closedTabs, FORGET),
        test_state.windows[0]._closedTabs.length - remember_count,
        "The correct amout of tabs are to be forgotten");
     is(countByTitle(closedTabs, REMEMBER), remember_count,
-       "Everything is set up.");
+       "Everything is set up");
 
-    // all of the following calls with illegal arguments should throw NS_ERROR_ILLEGAL_VALUE
+    // All of the following calls with illegal arguments should throw NS_ERROR_ILLEGAL_VALUE.
     ok(testForError(() => ss.forgetClosedTab({}, 0)),
        "Invalid window for forgetClosedTab throws");
     ok(testForError(() => ss.forgetClosedTab(newWin, -1)),
        "Invalid tab for forgetClosedTab throws");
     ok(testForError(() => ss.forgetClosedTab(newWin, test_state.windows[0]._closedTabs.length + 1)),
        "Invalid tab for forgetClosedTab throws");
 
-    // Remove third tab, then first tab
+    // Remove third tab, then first tab.
     ss.forgetClosedTab(newWin, 2);
     ss.forgetClosedTab(newWin, null);
 
-    closedTabs = JSON.parse(ss.getClosedTabData(newWin));
+    closedTabs = SessionStore.getClosedTabData(newWin, false);
+
+    // Verify that non JSON serialized data is the same as JSON serialized data.
+    is(JSON.stringify(closedTabs), SessionStore.getClosedTabData(newWin),
+       "Non-serialized data is the same as serialized data")
+
     is(closedTabs.length, remember_count,
        "The correct amout of tabs was removed");
     is(countByTitle(closedTabs, FORGET), 0,
        "All tabs specifically forgotten were indeed removed");
     is(countByTitle(closedTabs, REMEMBER), remember_count,
-       "... and tabs not specifically forgetten weren't.");
+       "... and tabs not specifically forgetten weren't");
 
-    // clean up
+    // Clean up.
     gPrefService.clearUserPref("browser.sessionstore.max_tabs_undo");
     BrowserTestUtils.closeWindow(newWin).then(finish);
   });
 }