Bug 1142734: Stop using Timer.jsm to avoid replacing the browser window setTimout and clearTimeout functions. r?jsantell draft
authorDave Townsend <dtownsend@oxymoronical.com>
Mon, 08 Feb 2016 10:11:14 -0800
changeset 329600 e51de234076ffabbf07f17dc1693db9a0a2014fb
parent 329599 6f669e396b901b3a58b9b35ac20390d62d78befa
child 513992 6870f46546a5dcdfcb0c72032a56b6ca6ef4b1a7
push id10561
push userdtownsend@mozilla.com
push dateMon, 08 Feb 2016 19:03:34 +0000
reviewersjsantell
bugs1142734
milestone47.0a1
Bug 1142734: Stop using Timer.jsm to avoid replacing the browser window setTimout and clearTimeout functions. r?jsantell jetpack-addon-harness.js runs in a browser window scope so it already has the setTimeout functions available to it. Loading Timer.jsm overrides the DOM timer functions with those from Timer.jsm. Any other code that used setTimeout previously will have timer IDs from the DOM functions which don't match those in Timer.jsm. If this other code attempts to clear a timer it can then end up clearing an unrelated timer. In the intermittent failure here the browser-thumbnails code manages to clear the timer that is waiting to resolve the promise that makes tests continue. I've also added an additional timer that throws an exception and so ends tests if the add-on uninstall doesn't actually complete in a reasonable time as well as removing the add-on listener.
testing/mochitest/jetpack-addon-harness.js
--- a/testing/mochitest/jetpack-addon-harness.js
+++ b/testing/mochitest/jetpack-addon-harness.js
@@ -4,17 +4,16 @@ var gConfig;
 if (Cc === undefined) {
   var Cc = Components.classes;
   var Ci = Components.interfaces;
   var Cu = Components.utils;
 }
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
   "resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
   "resource://gre/modules/AddonManager.jsm");
 
 setTimeout(testInit, 0);
 
@@ -93,25 +92,32 @@ function installAddon(url) {
 // Uninstalls an add-on returning a promise for when it is gone
 function uninstallAddon(oldAddon) {
   return new Promise(function(resolve, reject) {
     AddonManager.addAddonListener({
       onUninstalled: function(addon) {
         if (addon.id != oldAddon.id)
           return;
 
+        AddonManager.removeAddonListener(this);
+
         dump("TEST-INFO | jetpack-addon-harness.js | Uninstalled test add-on " + addon.id + "\n");
 
         // Some add-ons do async work on uninstall, we must wait for that to
         // complete
         setTimeout(resolve, 500);
       }
     });
 
     oldAddon.uninstall();
+
+    // The uninstall should happen quickly, if not throw an exception
+    setTimeout(() => {
+      reject(new Error(`Addon ${oldAddon.id} failed to uninstall in a timely fashion.`));
+    }, 10000);
   });
 }
 
 // Waits for a test add-on to signal it has completed its tests
 function waitForResults() {
   return new Promise(function(resolve, reject) {
     Services.obs.addObserver(function(subject, topic, data) {
       Services.obs.removeObserver(arguments.callee, "sdk:test:results");