Bug 1145911 - fix leak and other test failures for reading list, r=gijs
authorFlorian Quèze <florian@queze.net>
Sat, 21 Mar 2015 08:56:40 -0700
changeset 263754 9ec91e9617f387dc8e593afc42a27eab2e34f2a8
parent 263753 75134b207f0dc7fe6a35cee9fec2b267c9ef68ee
child 263755 b5bfda73ba542bab25adb5febb3c9180c07fb66c
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs
bugs1145911
milestone39.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 1145911 - fix leak and other test failures for reading list, r=gijs
browser/base/content/browser-readinglist.js
browser/base/content/test/general/browser_readerMode.js
--- a/browser/base/content/browser-readinglist.js
+++ b/browser/base/content/browser-readinglist.js
@@ -53,16 +53,21 @@ let ReadingListUI = {
    */
   uninit() {
     Preferences.ignore("browser.readinglist.enabled", this.updateUI, this);
 
     const mm = window.messageManager;
     for (let msg of this.MESSAGES) {
       mm.removeMessageListener(msg, this);
     }
+
+    if (this.listenerRegistered) {
+      ReadingList.removeListener(this);
+      this.listenerRegistered = false;
+    }
   },
 
   /**
    * Whether the ReadingList feature is enabled or not.
    * @type {boolean}
    */
   get enabled() {
     return Preferences.get("browser.readinglist.enabled", false);
@@ -86,17 +91,17 @@ let ReadingListUI = {
       // This is a no-op if we're already registered.
       ReadingList.addListener(this);
       this.listenerRegistered = true;
     } else {
       if (this.listenerRegistered) {
         // This is safe to call if we're not currently registered, but we don't
         // want to forcibly load the normally lazy-loaded module on startup.
         ReadingList.removeListener(this);
-        this.listenerRegistered = true;
+        this.listenerRegistered = false;
       }
 
       this.hideSidebar();
     }
 
     document.getElementById(READINGLIST_COMMAND_ID).setAttribute("hidden", !enabled);
   },
 
@@ -246,16 +251,22 @@ let ReadingListUI = {
     if (!uri) {
       this.toolbarButton.setAttribute("hidden", true);
       if (this.isSidebarOpen)
         document.getElementById("sidebar").contentWindow.postMessage(msg, "*");
       return;
     }
 
     let isInList = yield ReadingList.hasItemForURL(uri);
+
+    if (window.closed) {
+      // Skip updating the UI if the window was closed since our hasItemForURL call.
+      return;
+    }
+
     if (this.isSidebarOpen) {
       if (isInList)
         msg.url = typeof uri == "string" ? uri : uri.spec;
       document.getElementById("sidebar").contentWindow.postMessage(msg, "*");
     }
     this.setToolbarButtonState(isInList);
   }),
 
--- a/browser/base/content/test/general/browser_readerMode.js
+++ b/browser/base/content/test/general/browser_readerMode.js
@@ -60,16 +60,21 @@ add_task(function* () {
   // After we click ReadingList button, status should be "open".
   listButton.click();
   yield promiseWaitForCondition(() => listButton.classList.contains("on"));
   ok(listButton.classList.contains("on"),
     "List button should now indicate SideBar-ReadingList open.");
   ok(ReadingListUI.isSidebarOpen,
     "The ReadingListUI should now indicate SideBar-ReadingList open.");
 
+  // Now close the sidebar.
+  listButton.click();
+  yield promiseWaitForCondition(() => !listButton.classList.contains("on"));
+  ok(!ReadingListUI.isSidebarOpen, "The sidebar should be closed.");
+
   readerButton.click();
   yield promiseTabLoadEvent(tab);
   is(gBrowser.selectedBrowser.currentURI.spec, url, "Original page loaded after clicking active reader mode button");
 
   // Load a new tab that is NOT reader-able.
   let newTab = gBrowser.selectedTab = gBrowser.addTab();
   yield promiseTabLoadEvent(newTab, "about:robots");
   yield promiseWaitForCondition(() => readerButton.hidden);