Bug 1424090 - Use dedicated observer notification instead of domwindowclosed to catch onClosed in testcases for SessionStore.jsm. r=billm
authorTooru Fujisawa <arai_a@mac.com>
Sat, 09 Dec 2017 09:52:33 -0600
changeset 395871 8f36094d67c5faf0d91a258b6dcb51d7fc8fdd94
parent 395870 2a01f65ec8af02827f0621f86e501ad236431415
child 395872 af13a0c0ef0b31bc298358837411f122df7b8399
push id98206
push userarai_a@mac.com
push dateSat, 09 Dec 2017 15:55:39 +0000
treeherdermozilla-inbound@8f36094d67c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1424090
milestone59.0a1
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
Bug 1424090 - Use dedicated observer notification instead of domwindowclosed to catch onClosed in testcases for SessionStore.jsm. r=billm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser_async_window_flushing.js
browser/components/sessionstore/test/browser_forget_async_closings.js
browser/components/sessionstore/test/head.js
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -24,16 +24,17 @@ const NOTIFY_SINGLE_WINDOW_RESTORED = "s
 const NOTIFY_WINDOWS_RESTORED = "sessionstore-windows-restored";
 const NOTIFY_BROWSER_STATE_RESTORED = "sessionstore-browser-state-restored";
 const NOTIFY_LAST_SESSION_CLEARED = "sessionstore-last-session-cleared";
 const NOTIFY_RESTORING_ON_STARTUP = "sessionstore-restoring-on-startup";
 const NOTIFY_INITIATING_MANUAL_RESTORE = "sessionstore-initiating-manual-restore";
 const NOTIFY_CLOSED_OBJECTS_CHANGED = "sessionstore-closed-objects-changed";
 
 const NOTIFY_TAB_RESTORED = "sessionstore-debug-tab-restored"; // WARNING: debug-only
+const NOTIFY_DOMWINDOWCLOSED_HANDLED = "sessionstore-debug-domwindowclosed-handled"; // WARNING: debug-only
 
 // Maximum number of tabs to restore simultaneously. Previously controlled by
 // the browser.sessionstore.max_concurrent_tabs pref.
 const MAX_CONCURRENT_TAB_RESTORES = 3;
 
 // Amount (in CSS px) by which we allow window edges to be off-screen
 // when restoring a window, before we override the saved position to
 // pull the window back within the available screen area.
@@ -767,16 +768,19 @@ var SessionStoreInternal = {
     switch (aTopic) {
       case "browser-window-before-show": // catch new windows
         this.onBeforeBrowserWindowShown(aSubject);
         break;
       case "domwindowclosed": // catch closed windows
         this.onClose(aSubject).then(() => {
           this._notifyOfClosedObjectsChange();
         });
+        if (gDebuggingEnabled) {
+          Services.obs.notifyObservers(null, NOTIFY_DOMWINDOWCLOSED_HANDLED);
+        }
         break;
       case "quit-application-granted":
         let syncShutdown = aData == "syncShutdown";
         this.onQuitApplicationGranted(syncShutdown);
         break;
       case "browser-lastwindow-close-granted":
         this.onLastWindowCloseGranted();
         break;
--- a/browser/components/sessionstore/test/browser_async_window_flushing.js
+++ b/browser/components/sessionstore/test/browser_async_window_flushing.js
@@ -37,38 +37,40 @@ add_task(async function test_add_interes
   });
 
   await promiseContentMessage(browser, "ss-test:OnHistoryReplaceEntry");
 
   // Clear out the userTypedValue so that the new window looks like
   // it's really not worth restoring.
   browser.userTypedValue = null;
 
-  // Once the domWindowClosed Promise resolves, the window should
-  // have closed, and SessionStore's onClose handler should have just
-  // run.
-  let domWindowClosed = BrowserTestUtils.domWindowClosed(newWin);
-
   // Once this windowClosed Promise resolves, we should have finished
   // the flush and revisited our decision to put this window into
   // the closed windows array.
   let windowClosed = BrowserTestUtils.windowClosed(newWin);
 
+  let handled = false;
+  whenDomWindowClosedHandled(() => {
+    // SessionStore's onClose handler should have just run.
+    let currentClosedWindows = ss.getClosedWindowCount();
+    is(currentClosedWindows, initialClosedWindows,
+       "We should not have added the window to the closed windows array");
+
+    handled = true;
+  });
+
   // Ok, let's close the window.
   newWin.close();
 
-  await domWindowClosed;
-  // OnClose has just finished running.
+  await windowClosed;
+
+  ok(handled, "domwindowclosed should already be handled here");
+
+  // The window flush has finished
   let currentClosedWindows = ss.getClosedWindowCount();
-  is(currentClosedWindows, initialClosedWindows,
-     "We should not have added the window to the closed windows array");
-
-  await windowClosed;
-  // The window flush has finished
-  currentClosedWindows = ss.getClosedWindowCount();
   is(currentClosedWindows,
      initialClosedWindows + 1,
      "We should have added the window to the closed windows array");
 });
 
 /**
  * Tests that if we initially store a closed window as interesting
  * to save in the closed windows array, that we revisit that decision
@@ -105,38 +107,40 @@ add_task(async function test_remove_unin
   await ContentTask.spawn(browser, null, async function() {
     // Epic hackery to make this browser seem suddenly boring.
     docShell.setCurrentURI(Services.io.newURI("about:blank"));
 
     let {sessionHistory} = docShell.QueryInterface(Ci.nsIWebNavigation);
     sessionHistory.PurgeHistory(sessionHistory.count);
   });
 
-  // Once the domWindowClosed Promise resolves, the window should
-  // have closed, and SessionStore's onClose handler should have just
-  // run.
-  let domWindowClosed = BrowserTestUtils.domWindowClosed(newWin);
-
   // Once this windowClosed Promise resolves, we should have finished
   // the flush and revisited our decision to put this window into
   // the closed windows array.
   let windowClosed = BrowserTestUtils.windowClosed(newWin);
 
+  let handled = false;
+  whenDomWindowClosedHandled(() => {
+    // SessionStore's onClose handler should have just run.
+    let currentClosedWindows = ss.getClosedWindowCount();
+    is(currentClosedWindows, initialClosedWindows + 1,
+       "We should have added the window to the closed windows array");
+
+    handled = true;
+  });
+
   // Ok, let's close the window.
   newWin.close();
 
-  await domWindowClosed;
-  // OnClose has just finished running.
+  await windowClosed;
+
+  ok(handled, "domwindowclosed should already be handled here");
+
+  // The window flush has finished
   let currentClosedWindows = ss.getClosedWindowCount();
-  is(currentClosedWindows, initialClosedWindows + 1,
-     "We should have added the window to the closed windows array");
-
-  await windowClosed;
-  // The window flush has finished
-  currentClosedWindows = ss.getClosedWindowCount();
   is(currentClosedWindows,
      initialClosedWindows,
      "We should have removed the window from the closed windows array");
 });
 
 /**
  * Tests that when we close a window, it is immediately removed from the
  * _windows array.
--- a/browser/components/sessionstore/test/browser_forget_async_closings.js
+++ b/browser/components/sessionstore/test/browser_forget_async_closings.js
@@ -79,31 +79,36 @@ let forgetWinHelper = async function(for
   let tab = newWin.gBrowser.selectedTab;
   let browser = tab.linkedBrowser;
   browser.loadURI(PAGE);
   await BrowserTestUtils.browserLoaded(browser, false, PAGE);
   await TabStateFlusher.flush(browser);
 
   // Now close the window and immediately choose to forget it.
   let windowClosed = BrowserTestUtils.windowClosed(newWin);
-  let domWindowClosed = BrowserTestUtils.domWindowClosed(newWin);
+
+  let handled = false;
+  whenDomWindowClosedHandled(() => {
+    // At this point, the window will have closed and the onClose handler
+    // has run, but the final update  to SessionStore hasn't come up yet.
+    // Now do the oepration that should cause us to forget the window.
+    forgetFn();
+
+    is(ss.getClosedWindowCount(), 0, "Should have forgotten the closed window");
+
+    handled = true;
+  });
 
   newWin.close();
-  await domWindowClosed;
-
-  // At this point, the window will have closed and the onClose handler
-  // has run, but the final update  to SessionStore hasn't come up yet.
-  // Now do the oepration that should cause us to forget the window.
-  forgetFn();
-
-  is(ss.getClosedWindowCount(), 0, "Should have forgotten the closed window");
 
   // Now wait for the final update to come up.
   await windowClosed;
 
+  ok(handled, "domwindowclosed should already be handled here");
+
   is(ss.getClosedWindowCount(), 0, "Should not have stored the closed window");
 };
 
 /**
  * Tests that if we choose to forget a tab while waiting for its
  * final flush to complete, we don't accidentally store it.
  */
 add_task(async function test_forget_closed_tab() {
--- a/browser/components/sessionstore/test/head.js
+++ b/browser/components/sessionstore/test/head.js
@@ -551,8 +551,15 @@ function popPrefs() {
 
 async function checkScroll(tab, expected, msg) {
   let browser = tab.linkedBrowser;
   await TabStateFlusher.flush(browser);
 
   let scroll = JSON.parse(ss.getTabState(tab)).scroll || null;
   is(JSON.stringify(scroll), JSON.stringify(expected), msg);
 }
+
+function whenDomWindowClosedHandled(aCallback) {
+  Services.obs.addObserver(function observer(aSubject, aTopic) {
+    Services.obs.removeObserver(observer, aTopic);
+    aCallback();
+  }, "sessionstore-debug-domwindowclosed-handled");
+}