Bug 1398518 - Fix test_ext_notifications.html intermittency and re-enable it. r=mixedpuppy, a=test-only
authorLuca Greco <lgreco@mozilla.com>
Wed, 30 Jan 2019 18:05:34 +0000
changeset 515660 4289fab8bad0b145af0185eafa48eccbd8bfac41
parent 515659 7d8476c9a200318df5f60eabeb57b6b6fa7c91fb
child 515661 84fd80a53ef3a0e917520839fdf1aedde3c7c6d5
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy, test-only
bugs1398518
milestone66.0
Bug 1398518 - Fix test_ext_notifications.html intermittency and re-enable it. r=mixedpuppy, a=test-only Based on what I've been able to reproduce locally using --verify, there is a chance that the notifications created by the test case named `test_notifications_populated_getAll` may have been removed before the `browser.notifications.getAll` is going to retrieve them, and when this happens the test gets stuck because `browser.test.notifyPass("getAll populated");` is never reached, and the test timeouts. This patch includes the following changes to prevent the intermittent failure described above: - add a new head_notifications.js support file and create a new MockAlertsService test helper that loads a chrome script which replace the alerts service with a mock service (based on a similar mock defined in dom/notification/test/mochitest/MockServices.js), the mocked alert service doesn't close the notification until the test case itself has called `MockAlertsService.clearNotifications` (so that the test shouldn't fail intermittently as described above). - applies the changes needed on test_ext_notifications.html to make use of the MockAlertsService Differential Revision: https://phabricator.services.mozilla.com/D17863
toolkit/components/extensions/test/mochitest/head_notifications.js
toolkit/components/extensions/test/mochitest/mochitest-common.ini
toolkit/components/extensions/test/mochitest/test_ext_notifications.html
copy from dom/notification/test/mochitest/MockServices.js
copy to toolkit/components/extensions/test/mochitest/head_notifications.js
--- a/dom/notification/test/mochitest/MockServices.js
+++ b/toolkit/components/extensions/test/mochitest/head_notifications.js
@@ -1,117 +1,108 @@
-var MockServices = (function () {
-  "use strict";
+"use strict";
 
-  const MOCK_ALERTS_CID = SpecialPowers.wrap(SpecialPowers.Components)
-                          .ID("{48068bc2-40ab-4904-8afd-4cdfb3a385f3}");
+/* exported MockAlertsService */
+
+function mockServicesChromeScript() {
+  const MOCK_ALERTS_CID = Components.ID("{48068bc2-40ab-4904-8afd-4cdfb3a385f3}");
   const ALERTS_SERVICE_CONTRACT_ID = "@mozilla.org/alerts-service;1";
 
-  const MOCK_SYSTEM_ALERTS_CID = SpecialPowers.wrap(SpecialPowers.Components)
-                                 .ID("{e86d888c-e41b-4b78-9104-2f2742a532de}");
-  const SYSTEM_ALERTS_SERVICE_CONTRACT_ID = "@mozilla.org/system-alerts-service;1";
-
-  var registrar = SpecialPowers.wrap(SpecialPowers.Components).manager
-                  .QueryInterface(SpecialPowers.Ci.nsIComponentRegistrar);
-
-  var activeAlertNotifications = Object.create(null);
-
-  var activeAppNotifications = Object.create(null);
+  const {setTimeout} = ChromeUtils.import("resource://gre/modules/Timer.jsm", {});
+  const registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
 
-  window.addEventListener('mock-notification-close-event', function(e) {
-    for (var alertName in activeAlertNotifications) {
-      var notif = activeAlertNotifications[alertName];
-      if (notif.title === e.detail.title) {
-        notif.listener.observe(null, "alertfinished", null);
-        delete activeAlertNotifications[alertName];
-        delete activeAppNotifications[alertName];
-        return;
-      }
-    }
-  });
+  let activeNotifications = Object.create(null);
 
-  var mockAlertsService = {
+  const mockAlertsService = {
     showPersistentNotification: function(persistentData, alert, alertListener) {
       this.showAlert(alert, alertListener);
     },
 
-    showAlert: function(alert, alertListener) {
-      var listener = SpecialPowers.wrap(alertListener);
-      activeAlertNotifications[alert.name] = {
+    showAlert: function(alert, listener) {
+      activeNotifications[alert.name] = {
         listener: listener,
         cookie: alert.cookie,
-        title: alert.title
+        title: alert.title,
       };
 
       // fake async alert show event
       if (listener) {
-        setTimeout(function () {
+        setTimeout(function() {
           listener.observe(null, "alertshow", alert.cookie);
         }, 100);
-        setTimeout(function () {
-          listener.observe(null, "alertclickcallback", alert.cookie);
-        }, 100);
       }
     },
 
     showAlertNotification: function(imageUrl, title, text, textClickable,
                                     cookie, alertListener, name) {
       this.showAlert({
         name: name,
         cookie: cookie,
-        title: title
+        title: title,
       }, alertListener);
     },
 
     closeAlert: function(name) {
-      var alertNotification = activeAlertNotifications[name];
+      let alertNotification = activeNotifications[name];
       if (alertNotification) {
         if (alertNotification.listener) {
           alertNotification.listener.observe(null, "alertfinished", alertNotification.cookie);
         }
-        delete activeAlertNotifications[name];
-      }
-
-      var appNotification = activeAppNotifications[name];
-      if (appNotification) {
-        delete activeAppNotifications[name];
+        delete activeNotifications[name];
       }
     },
 
-    QueryInterface: function(aIID) {
-      if (SpecialPowers.wrap(aIID).equals(SpecialPowers.Ci.nsISupports) ||
-          SpecialPowers.wrap(aIID).equals(SpecialPowers.Ci.nsIAlertsService)) {
-        return this;
+    QueryInterface: ChromeUtils.generateQI(["nsIAlertsService"]),
+
+    createInstance: function(outer, iid) {
+      if (outer != null) {
+        throw Cr.NS_ERROR_NO_AGGREGATION;
       }
-      throw SpecialPowers.Components.results.NS_ERROR_NO_INTERFACE;
+      return this.QueryInterface(iid);
     },
+  };
 
-    createInstance: function(aOuter, aIID) {
-      if (aOuter != null) {
-        throw SpecialPowers.Components.results.NS_ERROR_NO_AGGREGATION;
-      }
-      return this.QueryInterface(aIID);
+  registrar.registerFactory(MOCK_ALERTS_CID, "alerts service",
+                            ALERTS_SERVICE_CONTRACT_ID,
+                            mockAlertsService);
+
+  function closeAllNotifications() {
+    for (let alertName of Object.keys(activeNotifications)) {
+      mockAlertsService.closeAlert(alertName);
     }
-  };
-  mockAlertsService = SpecialPowers.wrapCallbackObject(mockAlertsService);
+  }
+
+  const {addMessageListener, sendAsyncMessage} = this;
 
-  // MockServices API
-  return {
-    register: function () {
-      registrar.registerFactory(MOCK_ALERTS_CID, "alerts service",
-          ALERTS_SERVICE_CONTRACT_ID,
-          mockAlertsService);
+  addMessageListener("mock-alert-service:unregister", () => {
+    closeAllNotifications();
+    activeNotifications = null;
+    registrar.unregisterFactory(MOCK_ALERTS_CID, mockAlertsService);
+    sendAsyncMessage("mock-alert-service:unregistered");
+  });
+
+  addMessageListener("mock-alert-service:close-notifications", closeAllNotifications);
+
+  sendAsyncMessage("mock-alert-service:registered");
+}
 
-      registrar.registerFactory(MOCK_SYSTEM_ALERTS_CID, "system alerts service",
-          SYSTEM_ALERTS_SERVICE_CONTRACT_ID,
-          mockAlertsService);
-    },
-
-    unregister: function () {
-      registrar.unregisterFactory(MOCK_ALERTS_CID, mockAlertsService);
-      registrar.unregisterFactory(MOCK_SYSTEM_ALERTS_CID, mockAlertsService);
-    },
-
-    activeAlertNotifications: activeAlertNotifications,
-
-    activeAppNotifications: activeAppNotifications,
-  };
-})();
+const MockAlertsService = {
+  async register() {
+    if (this._chromeScript) {
+      throw new Error("MockAlertsService already registered");
+    }
+    this._chromeScript = SpecialPowers.loadChromeScript(mockServicesChromeScript);
+    await this._chromeScript.promiseOneMessage("mock-alert-service:registered");
+  },
+  async unregister() {
+    if (!this._chromeScript) {
+      throw new Error("MockAlertsService not registered");
+    }
+    this._chromeScript.sendAsyncMessage("mock-alert-service:unregister");
+    return this._chromeScript.promiseOneMessage("mock-alert-service:unregistered").then(() => {
+      this._chromeScript.destroy();
+      this._chromeScript = null;
+    });
+  },
+  async closeNotifications() {
+    await this._chromeScript.sendAsyncMessage("mock-alert-service:close-notifications");
+  },
+};
--- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini
@@ -33,16 +33,17 @@ support-files =
   file_webNavigation_frameClientRedirect.html
   file_webNavigation_frameRedirect.html
   file_webNavigation_manualSubframe.html
   file_webNavigation_manualSubframe_page1.html
   file_webNavigation_manualSubframe_page2.html
   file_with_about_blank.html
   head.js
   head_cookies.js
+  head_notifications.js
   head_unlimitedStorage.js
   head_webrequest.js
   hsts.sjs
   mochitest_console.js
   oauth.html
   redirect_auto.sjs
   redirection.sjs
   return_headers.sjs
@@ -91,17 +92,16 @@ skip-if = os == 'android' # Android supp
 skip-if = os == 'android' # unsupported.
 [test_ext_idle.html]
 [test_ext_inIncognitoContext_window.html]
 skip-if = os == 'android' # Android does not support multiple windows.
 [test_ext_listener_proxies.html]
 [test_ext_new_tab_processType.html]
 skip-if = (verify && debug && (os == 'linux' || os == 'mac'))
 [test_ext_notifications.html]
-skip-if = os == "win" # Bug 1398518
 [test_ext_protocolHandlers.html]
 skip-if = (toolkit == 'android') # bug 1342577
 [test_ext_redirect_jar.html]
 [test_ext_runtime_connect.html]
 [test_ext_runtime_connect_twoway.html]
 [test_ext_runtime_connect2.html]
 [test_ext_runtime_disconnect.html]
 [test_ext_sendmessage_doublereply.html]
--- a/toolkit/components/extensions/test/mochitest/test_ext_notifications.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_notifications.html
@@ -1,28 +1,33 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <title>Test for notifications</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head_notifications.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
 // A 1x1 PNG image.
 // Source: https://commons.wikimedia.org/wiki/File:1x1.png (Public Domain)
 let image = atob("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAAA1BMVEUAA" +
                  "ACnej3aAAAAAXRSTlMAQObYZgAAAApJREFUCNdjYAAAAAIAAeIhvDMAAAAASUVORK5CYII=");
 const IMAGE_ARRAYBUFFER = Uint8Array.from(image, byte => byte.charCodeAt(0)).buffer;
 
+add_task(async function setup_mock_alert_service() {
+  await MockAlertsService.register();
+});
+
 add_task(async function test_notification() {
   async function background() {
     let opts = {
       type: "basic",
       title: "Testing Notification",
       message: "Carry on",
     };
 
@@ -41,17 +46,17 @@ add_task(async function test_notificatio
   await extension.startup();
   let x = await extension.awaitMessage("running");
   is(x, "0", "got correct id from notifications.create");
   await extension.awaitFinish();
   await extension.unload();
 });
 
 add_task(async function test_notification_events() {
-  function background() {
+  async function background() {
     let opts = {
       type: "basic",
       title: "Testing Notification",
       message: "Carry on",
     };
 
     let createdId = "98";
 
@@ -62,38 +67,48 @@ add_task(async function test_notificatio
     // but we can attempt to add a listener.
     browser.notifications.onClicked.addListener(function() {});
 
     browser.notifications.onShown.addListener(async function listener(id) {
       browser.notifications.onShown.removeListener(listener);
       browser.test.assertEq(createdId, id, "onShown received the expected id.");
       let newId = await browser.notifications.create(id, opts);
       browser.test.assertEq(createdId, newId, "create returned the expected id.");
-      browser.test.sendMessage("created");
+      browser.test.sendMessage("notification-created-twice");
     });
 
     // Test onClosed listener.
     browser.notifications.onClosed.addListener(function listener(id) {
       browser.notifications.onClosed.removeListener(listener);
       browser.test.assertEq(createdId, id, "onClosed received the expected id.");
-      browser.test.sendMessage("closed");
+      browser.test.sendMessage("notification-closed");
     });
 
-    browser.notifications.create(createdId, opts);
+    await browser.notifications.create(createdId, opts);
+
+    browser.test.sendMessage("notification-created-once");
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: ["notifications"],
     },
     background,
   });
+
   await extension.startup();
-  await extension.awaitMessage("closed");
-  await extension.awaitMessage("created");
+
+  await extension.awaitMessage("notification-created-once");
+  await MockAlertsService.closeNotifications();
+
+  await extension.awaitMessage("notification-closed");
+  await extension.awaitMessage("notification-created-twice");
+
+  await MockAlertsService.closeNotifications();
+
   await extension.unload();
 });
 
 add_task(async function test_notification_clear() {
   function background() {
     let opts = {
       type: "basic",
       title: "Testing Notification",
@@ -117,16 +132,17 @@ add_task(async function test_notificatio
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: ["notifications"],
     },
     background,
   });
+
   await extension.startup();
   await extension.awaitFinish();
   await extension.unload();
 });
 
 add_task(async function test_notifications_empty_getAll() {
   async function background() {
     let notifications = await browser.notifications.getAll();
@@ -270,12 +286,16 @@ add_task(async function test_notificatio
     },
   });
 
   await extension.startup();
   await extension.awaitFinish("notifications");
   await extension.unload();
 });
 
+add_task(async function teardown_mock_alert_service() {
+  await MockAlertsService.unregister();
+});
+
 </script>
 
 </body>
 </html>