Bug 608820 - Set callbacks before invoking actions to avoid async races and convert test to Task. r=Mossop, a=test-only
authorIrving Reid <irving@mozilla.com>
Tue, 29 Apr 2014 14:06:29 -0400
changeset 200136 003162dc4e12ebd5c1b567404b558e0b5d4b729a
parent 200135 22d65607166fa67f5ccb1c4e27cc8e51763a5bcd
child 200137 b92c30f6d6397a7cff935d64fa54550dee1affba
push id486
push userasasaki@mozilla.com
push dateMon, 14 Jul 2014 18:39:42 +0000
treeherdermozilla-release@d33428174ff1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMossop, test-only
bugs608820
milestone31.0a2
Bug 608820 - Set callbacks before invoking actions to avoid async races and convert test to Task. r=Mossop, a=test-only
toolkit/mozapps/extensions/test/browser/browser-common.ini
toolkit/mozapps/extensions/test/browser/browser_bug567127.js
toolkit/mozapps/extensions/test/browser/head.js
--- a/toolkit/mozapps/extensions/test/browser/browser-common.ini
+++ b/toolkit/mozapps/extensions/test/browser/browser-common.ini
@@ -7,17 +7,16 @@ support-files =
 [browser_bug557943.js]
 [browser_bug562797.js]
 skip-if = e10s # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly
 [browser_bug562854.js]
 [browser_bug562890.js]
 [browser_bug562899.js]
 [browser_bug562992.js]
 [browser_bug567127.js]
-skip-if = os == 'win' || os == 'mac' # Bug 608820
 [browser_bug567137.js]
 [browser_bug570760.js]
 skip-if = e10s # Bug ?????? - EventUtils.synthesizeKey not e10s friendly
 [browser_bug572561.js]
 [browser_bug577990.js]
 [browser_bug580298.js]
 [browser_bug581076.js]
 [browser_bug586574.js]
--- a/toolkit/mozapps/extensions/test/browser/browser_bug567127.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_bug567127.js
@@ -93,44 +93,41 @@ function test_confirmation(aWindow, aExp
       }
     }
     ok(found, "Should have seen " + url + " in the list");
   }
 
   aWindow.document.documentElement.cancelDialog();
 }
 
-
-function test() {
-  waitForExplicitFinish();
-  
-  open_manager("addons://list/extension", function(aWindow) {
-    gManagerWindow = aWindow;
-    run_next_test();
-  });
-}
+add_task(function* test_install_from_file() {
+  gManagerWindow = yield open_manager("addons://list/extension");
 
-function end_test() {
-  is(gSawInstallNotification, true, "Should have seen addon-install-started notification.");
-
-  MockFilePicker.cleanup();
-  close_manager(gManagerWindow, function() {
-    finish();
-  });
-}
-
-
-add_test(function() {
   var filePaths = [
                    get_addon_file_url("browser_bug567127_1.xpi"),
                    get_addon_file_url("browser_bug567127_2.xpi")
                   ];
   MockFilePicker.returnFiles = filePaths.map(function(aPath) aPath.file);
   
   Services.obs.addObserver(gInstallNotificationObserver,
                            "addon-install-started", false);
 
-  new WindowOpenListener(INSTALL_URI, function(aWindow) {
-    test_confirmation(aWindow, filePaths.map(function(aPath) aPath.spec));
-  }, run_next_test);
+  // Set handler that executes the core test after the window opens,
+  // and resolves the promise when the window closes
+  let pInstallURIClosed = new Promise((resolve, reject) => {
+    new WindowOpenListener(INSTALL_URI, function(aWindow) {
+      try {
+        test_confirmation(aWindow, filePaths.map(function(aPath) aPath.spec));
+      } catch(e) {
+        reject(e);
+      }
+    }, resolve);
+  });
 
   gManagerWindow.gViewController.doCommand("cmd_installFromFile");
+
+  yield pInstallURIClosed;
+
+  is(gSawInstallNotification, true, "Should have seen addon-install-started notification.");
+
+  MockFilePicker.cleanup();
+  yield close_manager(gManagerWindow);
 });
--- a/toolkit/mozapps/extensions/test/browser/head.js
+++ b/toolkit/mozapps/extensions/test/browser/head.js
@@ -142,16 +142,22 @@ function log_exceptions(aCallback, ...aA
     return aCallback.apply(null, aArgs);
   }
   catch (e) {
     info("Exception thrown: " + e);
     throw e;
   }
 }
 
+function log_callback(aPromise, aCallback) {
+  aPromise.then(aCallback)
+    .then(null, e => info("Exception thrown: " + e));
+  return aPromise;
+}
+
 function add_test(test) {
   gPendingTests.push(test);
 }
 
 function run_next_test() {
   if (gTestsRun > 0)
     info("Test " + gTestsRun + " took " + (Date.now() - gTestStart) + "ms");
 
@@ -278,97 +284,99 @@ function wait_for_manager_load(aManagerW
   info("Waiting for initialization");
   aManagerWindow.document.addEventListener("Initialized", function() {
     aManagerWindow.document.removeEventListener("Initialized", arguments.callee, false);
     log_exceptions(aCallback, aManagerWindow);
   }, false);
 }
 
 function open_manager(aView, aCallback, aLoadCallback, aLongerTimeout) {
-  let deferred = Promise.defer();
+  let p = new Promise((resolve, reject) => {
 
-  function setup_manager(aManagerWindow) {
-    if (aLoadCallback)
-      log_exceptions(aLoadCallback, aManagerWindow);
+    function setup_manager(aManagerWindow) {
+      if (aLoadCallback)
+        log_exceptions(aLoadCallback, aManagerWindow);
+
+      if (aView)
+        aManagerWindow.loadView(aView);
 
-    if (aView)
-      aManagerWindow.loadView(aView);
+      ok(aManagerWindow != null, "Should have an add-ons manager window");
+      is(aManagerWindow.location, MANAGER_URI, "Should be displaying the correct UI");
 
-    ok(aManagerWindow != null, "Should have an add-ons manager window");
-    is(aManagerWindow.location, MANAGER_URI, "Should be displaying the correct UI");
+      waitForFocus(function() {
+        info("window has focus, waiting for manager load");
+        wait_for_manager_load(aManagerWindow, function() {
+          info("Manager waiting for view load");
+          wait_for_view_load(aManagerWindow, function() {
+            resolve(aManagerWindow);
+          }, null, aLongerTimeout);
+        });
+      }, aManagerWindow);
+    }
 
-    waitForFocus(function() {
-      wait_for_manager_load(aManagerWindow, function() {
-        wait_for_view_load(aManagerWindow, function() {
-          // Some functions like synthesizeMouse don't like to be called during
-          // the load event so ensure that has completed
-          executeSoon(function() {
-            if (aCallback) {
-              log_exceptions(aCallback, aManagerWindow);
-            }
-            deferred.resolve(aManagerWindow);
-          });
-        }, null, aLongerTimeout);
-      });
-    }, aManagerWindow);
-  }
+    if (gUseInContentUI) {
+      info("Loading manager window in tab");
+      Services.obs.addObserver(function (aSubject, aTopic, aData) {
+        Services.obs.removeObserver(arguments.callee, aTopic);
+        if (aSubject.location.href != MANAGER_URI) {
+          info("Ignoring load event for " + aSubject.location.href);
+          return;
+        }
+        setup_manager(aSubject);
+      }, "EM-loaded", false);
 
-  if (gUseInContentUI) {
-    gBrowser.selectedTab = gBrowser.addTab();
-    switchToTabHavingURI(MANAGER_URI, true);
+      gBrowser.selectedTab = gBrowser.addTab();
+      switchToTabHavingURI(MANAGER_URI, true);
+    } else {
+      info("Loading manager window in dialog");
+      Services.obs.addObserver(function (aSubject, aTopic, aData) {
+        Services.obs.removeObserver(arguments.callee, aTopic);
+        setup_manager(aSubject);
+      }, "EM-loaded", false);
 
-    // This must be a new load, else the ping/pong would have
-    // found the window above.
-    Services.obs.addObserver(function (aSubject, aTopic, aData) {
-      Services.obs.removeObserver(arguments.callee, aTopic);
-      if (aSubject.location.href != MANAGER_URI)
-        return;
-      setup_manager(aSubject);
-    }, "EM-loaded", false);
-    return deferred.promise;
-  }
+      openDialog(MANAGER_URI);
+    }
+  });
 
-  openDialog(MANAGER_URI);
-  Services.obs.addObserver(function (aSubject, aTopic, aData) {
-    Services.obs.removeObserver(arguments.callee, aTopic);
-    setup_manager(aSubject);
-  }, "EM-loaded", false);
-
-  return deferred.promise;
+  // The promise resolves with the manager window, so it is passed to the callback
+  return log_callback(p, aCallback);
 }
 
 function close_manager(aManagerWindow, aCallback, aLongerTimeout) {
-  let deferred = Promise.defer();
-  requestLongerTimeout(aLongerTimeout ? aLongerTimeout : 2);
+  let p = new Promise((resolve, reject) => {
+    requestLongerTimeout(aLongerTimeout ? aLongerTimeout : 2);
 
-  ok(aManagerWindow != null, "Should have an add-ons manager window to close");
-  is(aManagerWindow.location, MANAGER_URI, "Should be closing window with correct URI");
+    ok(aManagerWindow != null, "Should have an add-ons manager window to close");
+    is(aManagerWindow.location, MANAGER_URI, "Should be closing window with correct URI");
 
-  aManagerWindow.addEventListener("unload", function() {
-    this.removeEventListener("unload", arguments.callee, false);
-    if (aCallback) {
-      log_exceptions(aCallback);
-    }
-    deferred.resolve();
-  }, false);
+    aManagerWindow.addEventListener("unload", function() {
+      try {
+        dump("Manager window unload handler");
+        this.removeEventListener("unload", arguments.callee, false);
+        resolve();
+      } catch(e) {
+        reject(e);
+      }
+    }, false);
+  });
 
+  info("Telling manager window to close");
   aManagerWindow.close();
+  info("Manager window close() call returned");
 
-  return deferred.promise;
+  return log_callback(p, aCallback);
 }
 
 function restart_manager(aManagerWindow, aView, aCallback, aLoadCallback) {
   if (!aManagerWindow) {
-    open_manager(aView, aCallback, aLoadCallback);
-    return;
+    return open_manager(aView, aCallback, aLoadCallback);
   }
 
-  close_manager(aManagerWindow, function() {
-    open_manager(aView, aCallback, aLoadCallback);
-  });
+  return close_manager(aManagerWindow)
+    .then(() => open_manager(aView, aCallback, aLoadCallback));
 }
 
 function wait_for_window_open(aCallback) {
   Services.wm.addListener({
     onOpenWindow: function(aWindow) {
       Services.wm.removeListener(this);
 
       let domwindow = aWindow.QueryInterface(Ci.nsIInterfaceRequestor)
@@ -432,35 +440,27 @@ function is_element_hidden(aElement, aMs
 }
 
 /**
  * Install an add-on and call a callback when complete.
  *
  * The callback will receive the Addon for the installed add-on.
  */
 function install_addon(path, cb, pathPrefix=TESTROOT) {
-  let deferred = Promise.defer();
-
-  AddonManager.getInstallForURL(pathPrefix + path, (install) => {
-    install.addListener({
-      onInstallEnded: () => {
-        executeSoon(() => {
-          if (cb) {
-            cb(install.addon);
-          }
+  let p = new Promise((resolve, reject) => {
+    AddonManager.getInstallForURL(pathPrefix + path, (install) => {
+      install.addListener({
+        onInstallEnded: () => resolve(install.addon),
+      });
 
-          deferred.resolve(install.addon);
-        });
-      },
-    });
+      install.install();
+    }, "application/x-xpinstall");
+  });
 
-    install.install();
-  }, "application/x-xpinstall");
-
-  return deferred.promise;
+  return log_callback(p, cb);
 }
 
 function CategoryUtilities(aManagerWindow) {
   this.window = aManagerWindow;
 
   var self = this;
   this.window.addEventListener("unload", function() {
     self.window.removeEventListener("unload", arguments.callee, false);
@@ -512,32 +512,24 @@ CategoryUtilities.prototype = {
     return !is_hidden(aCategory);
   },
 
   isTypeVisible: function(aCategoryType) {
     return this.isVisible(this.get(aCategoryType));
   },
 
   open: function(aCategory, aCallback) {
-    let deferred = Promise.defer();
 
     isnot(this.window, null, "Should not open category when manager window is not loaded");
     ok(this.isVisible(aCategory), "Category should be visible if attempting to open it");
 
     EventUtils.synthesizeMouse(aCategory, 2, 2, { }, this.window);
+    let p = new Promise((resolve, reject) => wait_for_view_load(this.window, resolve));
 
-    wait_for_view_load(this.window, (win) => {
-      if (aCallback) {
-        log_exceptions(aCallback, win);
-      }
-
-      deferred.resolve(win);
-    });
-
-    return deferred.promise;
+    return log_callback(p, aCallback);
   },
 
   openType: function(aCategoryType, aCallback) {
     return this.open(this.get(aCategoryType), aCallback);
   }
 }
 
 function CertOverrideListener(host, bits) {