Bug 1041537 - Prevent ContentSearch from leaking the browser if it's still processing a message while the test suite shuts down r=adw
authorTim Taubert <ttaubert@mozilla.com>
Thu, 21 Aug 2014 11:18:54 +0200
changeset 200963 885c0c395a10845aaee95a9ac84356132f826c6e
parent 200962 966c24f8e64dae7e0e8423f09cc1e74b7a0eaffa
child 200964 b1c83316ffd61d5dd6787feb6a00829552ef77be
push id8332
push userttaubert@mozilla.com
push dateFri, 22 Aug 2014 14:36:45 +0000
treeherderfx-team@885c0c395a10 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1041537
milestone34.0a1
Bug 1041537 - Prevent ContentSearch from leaking the browser if it's still processing a message while the test suite shuts down r=adw
browser/modules/ContentSearch.jsm
testing/mochitest/browser-test.js
--- a/browser/modules/ContentSearch.jsm
+++ b/browser/modules/ContentSearch.jsm
@@ -76,29 +76,39 @@ const OUTBOUND_MESSAGE = INBOUND_MESSAGE
  */
 
 this.ContentSearch = {
 
   // Inbound events are queued and processed in FIFO order instead of handling
   // them immediately, which would result in non-FIFO responses due to the
   // asynchrononicity added by converting image data URIs to ArrayBuffers.
   _eventQueue: [],
-  _currentEvent: null,
+  _currentEventPromise: null,
 
   // This is used to handle search suggestions.  It maps xul:browsers to objects
   // { controller, previousFormHistoryResult }.  See _onMessageGetSuggestions.
   _suggestionMap: new WeakMap(),
 
   init: function () {
     Cc["@mozilla.org/globalmessagemanager;1"].
       getService(Ci.nsIMessageListenerManager).
       addMessageListener(INBOUND_MESSAGE, this);
     Services.obs.addObserver(this, "browser-search-engine-modified", false);
   },
 
+  destroy: function () {
+    Cc["@mozilla.org/globalmessagemanager;1"].
+      getService(Ci.nsIMessageListenerManager).
+      removeMessageListener(INBOUND_MESSAGE, this);
+    Services.obs.removeObserver(this, "browser-search-engine-modified");
+
+    this._eventQueue.length = 0;
+    return Promise.resolve(this._currentEventPromise);
+  },
+
   /**
    * Focuses the search input in the page with the given message manager.
    * @param  messageManager
    *         The MessageManager object of the selected browser.
    */
   focusInput: function (messageManager) {
     messageManager.sendAsyncMessage(OUTBOUND_MESSAGE, {
       type: "FocusInput"
@@ -136,32 +146,34 @@ this.ContentSearch = {
         type: "Observe",
         data: data,
       });
       this._processEventQueue();
       break;
     }
   },
 
-  _processEventQueue: Task.async(function* () {
-    if (this._currentEvent || !this._eventQueue.length) {
+  _processEventQueue: function () {
+    if (this._currentEventPromise || !this._eventQueue.length) {
       return;
     }
-    this._currentEvent = this._eventQueue.shift();
-    try {
-      yield this["_on" + this._currentEvent.type](this._currentEvent.data);
-    }
-    catch (err) {
-      Cu.reportError(err);
-    }
-    finally {
-      this._currentEvent = null;
-      this._processEventQueue();
-    }
-  }),
+
+    let event = this._eventQueue.shift();
+
+    return this._currentEventPromise = Task.spawn(function* () {
+      try {
+        yield this["_on" + event.type](event.data);
+      } catch (err) {
+        Cu.reportError(err);
+      } finally {
+        this._currentEventPromise = null;
+        this._processEventQueue();
+      }
+    }.bind(this));
+  },
 
   _onMessage: Task.async(function* (msg) {
     let methodName = "_onMessage" + msg.data.type;
     if (methodName in this) {
       yield this._initService();
       yield this[methodName](msg, msg.data.data);
       msg.target.removeEventListener("SwapDocShells", msg, true);
     }
--- a/testing/mochitest/browser-test.js
+++ b/testing/mochitest/browser-test.js
@@ -11,20 +11,23 @@ if (Cc === undefined) {
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
   "resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserNewTabPreloader",
-  "resource:///modules/BrowserNewTabPreloader.jsm", "BrowserNewTabPreloader");
+  "resource:///modules/BrowserNewTabPreloader.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "CustomizationTabPreloader",
-  "resource:///modules/CustomizationTabPreloader.jsm", "CustomizationTabPreloader");
+  "resource:///modules/CustomizationTabPreloader.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "ContentSearch",
+  "resource:///modules/ContentSearch.jsm");
 
 const SIMPLETEST_OVERRIDES =
   ["ok", "is", "isnot", "ise", "todo", "todo_is", "todo_isnot", "info", "expectAssertions"];
 
 window.addEventListener("load", function testOnLoad() {
   window.removeEventListener("load", testOnLoad);
   window.addEventListener("MozAfterPaint", function testOnMozAfterPaint() {
     window.removeEventListener("MozAfterPaint", testOnMozAfterPaint);
@@ -450,16 +453,18 @@ Tester.prototype = {
       this.currentTest.scope = null;
     }
 
     // Check the window state for the current test before moving to the next one.
     // This also causes us to check before starting any tests, since nextTest()
     // is invoked to start the tests.
     this.waitForWindowsState((function () {
       if (this.done) {
+        let promise = Promise.resolve();
+
         // Uninitialize a few things explicitly so that they can clean up
         // frames and browser intentionally kept alive until shutdown to
         // eliminate false positives.
         if (gConfig.testRoot == "browser") {
           // Replace the document currently loaded in the browser's sidebar.
           // This will prevent false positives for tests that were the last
           // to touch the sidebar. They will thus not be blamed for leaking
           // a document.
@@ -479,16 +484,19 @@ Tester.prototype = {
             Cu.import("resource://gre/modules/BackgroundPageThumbs.jsm", {});
           BackgroundPageThumbs._destroy();
 
           BrowserNewTabPreloader.uninit();
           CustomizationTabPreloader.uninit();
           SocialFlyout.unload();
           SocialShare.uninit();
           TabView.uninit();
+
+          // Destroying ContentSearch is asynchronous.
+          promise = ContentSearch.destroy();
         }
 
         // Schedule GC and CC runs before finishing in order to detect
         // DOM windows leaked by our tests or the tested code. Note that we
         // use a shrinking GC so that the JS engine will discard JIT code and
         // JIT caches more aggressively.
 
         let checkForLeakedGlobalWindows = aCallback => {
@@ -510,30 +518,32 @@ Tester.prototype = {
           for (let result of aResults) {
             let test = this.openedWindows[result.url];
             let msg = "leaked until shutdown [" + result.name +
                       " " + (this.openedURLs[result.url] || "NULL") + "]";
             test.addResult(new testResult(false, msg, "", false));
           }
         };
 
-        checkForLeakedGlobalWindows(aResults => {
-          if (aResults.length == 0) {
-            this.finish();
-            return;
-          }
-          // After the first check, if there are reported leaked windows, sleep
-          // for a while, to allow off-main-thread work to complete and free up
-          // main-thread objects.  Then check again.
-          setTimeout(() => {
-            checkForLeakedGlobalWindows(aResults => {
-              reportLeaks(aResults);
+        promise.then(() => {
+          checkForLeakedGlobalWindows(aResults => {
+            if (aResults.length == 0) {
               this.finish();
-            });
-          }, 1000);
+              return;
+            }
+            // After the first check, if there are reported leaked windows, sleep
+            // for a while, to allow off-main-thread work to complete and free up
+            // main-thread objects.  Then check again.
+            setTimeout(() => {
+              checkForLeakedGlobalWindows(aResults => {
+                reportLeaks(aResults);
+                this.finish();
+              });
+            }, 1000);
+          });
         });
 
         return;
       }
 
       this.currentTestIndex++;
       this.execTest();
     }).bind(this));