Bug 1418311 - Avoid overwriting the notifications map in different contexts, r=mixedpuppy

Prior to this patch, the map that holds the list of notifications for an extension was recreated
for each context. This fixes that issue and maintains a list for the extension across all
contexts.

MozReview-Commit-ID: 2wfABoyyqvF

--HG--
extra : rebase_source : f62e4ceaf9ecd63c2502019f1a04a4314e6d3e58

toolkit/components/extensions/ext-notifications.js

@@ -12,18 +12,15 @@ var {
   ignoreEvent,
 } = ExtensionCommon;
 
-// WeakMap[Extension -> Map[id -> Notification]]
-let notificationsMap = new WeakMap();
-
 // Manages a notification popup (notifications API) created by the extension.
-function Notification(extension, id, options) {
-  this.extension = extension;
+function Notification(extension, notificationsMap, id, options) {
+  this.notificationsMap = notificationsMap;
   this.id = id;
   this.options = options;
 
   let imageURL;
   if (options.iconUrl) {
-    imageURL = this.extension.baseURI.resolve(options.iconUrl);
+    imageURL = extension.baseURI.resolve(options.iconUrl);
   }
 
   try {
@@ -48,22 +45,15 @@ Notification.prototype = {
     } catch (e) {
       // This will fail if the OS doesn't support this function.
     }
-    notificationsMap.get(this.extension).delete(this.id);
+    this.notificationsMap.delete(this.id);
   },
 
   observe(subject, topic, data) {
-    let notifications = notificationsMap.get(this.extension);
-
     let emitAndDelete = event => {
-      notifications.emit(event, data);
-      notifications.delete(this.id);
+      this.notificationsMap.emit(event, data);
+      this.notificationsMap.delete(this.id);
     };
 
-    // Don't try to emit events if the extension has been unloaded
-    if (!notifications) {
-      return;
-    }
-
     switch (topic) {
       case "alertclickcallback":
         emitAndDelete("clicked");
@@ -72,7 +62,7 @@ Notification.prototype = {
         emitAndDelete("closed");
         break;
       case "alertshow":
-        notifications.emit("shown", data);
+        this.notificationsMap.emit("shown", data);
         break;
     }
   },
@@ -83,25 +73,19 @@ this.notifications = class extends ExtensionAPI {
     super(extension);
 
     this.nextId = 0;
+    this.notificationsMap = new Map();
+    ToolkitModules.EventEmitter.decorate(this.notificationsMap);
   }
 
   onShutdown() {
-    let {extension} = this;
-
-    if (notificationsMap.has(extension)) {
-      for (let notification of notificationsMap.get(extension).values()) {
-        notification.clear();
-      }
-      notificationsMap.delete(extension);
+    for (let notification of this.notificationsMap.values()) {
+      notification.clear();
     }
   }
 
   getAPI(context) {
     let {extension} = context;
-
-    let map = new Map();
-    ToolkitModules.EventEmitter.decorate(map);
-    notificationsMap.set(extension, map);
+    let notificationsMap = this.notificationsMap;
 
     return {
       notifications: {
@@ -110,23 +94,19 @@ this.notifications = class extends ExtensionAPI {
             notificationId = String(this.nextId++);
           }
 
-          let notifications = notificationsMap.get(extension);
-          if (notifications.has(notificationId)) {
-            notifications.get(notificationId).clear();
+          if (notificationsMap.has(notificationId)) {
+            notificationsMap.get(notificationId).clear();
           }
 
-          // FIXME: Lots of options still aren't supported, especially
-          // buttons.
-          let notification = new Notification(extension, notificationId, options);
-          notificationsMap.get(extension).set(notificationId, notification);
+          let notification = new Notification(extension, notificationsMap, notificationId, options);
+          notificationsMap.set(notificationId, notification);
 
           return Promise.resolve(notificationId);
         },
 
         clear: function(notificationId) {
-          let notifications = notificationsMap.get(extension);
-          if (notifications.has(notificationId)) {
-            notifications.get(notificationId).clear();
+          if (notificationsMap.has(notificationId)) {
+            notificationsMap.get(notificationId).clear();
             return Promise.resolve(true);
           }
           return Promise.resolve(false);
@@ -134,7 +114,7 @@ this.notifications = class extends ExtensionAPI {
 
         getAll: function() {
           let result = {};
-          notificationsMap.get(extension).forEach((value, key) => {
+          notificationsMap.forEach((value, key) => {
             result[key] = value.options;
           });
           return Promise.resolve(result);
@@ -142,13 +122,13 @@ this.notifications = class extends ExtensionAPI {
 
         onClosed: new EventManager(context, "notifications.onClosed", fire => {
           let listener = (event, notificationId) => {
-            // FIXME: Support the byUser argument (bug 1413188).
+            // TODO Bug 1413188, Support the byUser argument.
             fire.async(notificationId, true);
           };
 
-          notificationsMap.get(extension).on("closed", listener);
+          notificationsMap.on("closed", listener);
           return () => {
-            notificationsMap.get(extension).off("closed", listener);
+            notificationsMap.off("closed", listener);
           };
         }).api(),
 
@@ -157,9 +137,9 @@ this.notifications = class extends ExtensionAPI {
             fire.async(notificationId, true);
           };
 
-          notificationsMap.get(extension).on("clicked", listener);
+          notificationsMap.on("clicked", listener);
           return () => {
-            notificationsMap.get(extension).off("clicked", listener);
+            notificationsMap.off("clicked", listener);
           };
         }).api(),
 
@@ -168,13 +148,13 @@ this.notifications = class extends ExtensionAPI {
             fire.async(notificationId, true);
           };
 
-          notificationsMap.get(extension).on("shown", listener);
+          notificationsMap.on("shown", listener);
           return () => {
-            notificationsMap.get(extension).off("shown", listener);
+            notificationsMap.off("shown", listener);
           };
         }).api(),
 
-        // Intend to implement this later: https://bugzilla.mozilla.org/show_bug.cgi?id=1190681
+        // TODO Bug 1190681, implement button support.
         onButtonClicked: ignoreEvent(context, "notifications.onButtonClicked"),
       },
     };

toolkit/components/extensions/test/mochitest/test_ext_notifications.html

@@ -224,6 +224,57 @@ add_task(async function test_buttons_unsupported() {
   await extension.unload();
 });
 
+add_task(async function test_notifications_different_contexts() {
+  async function background() {
+    let opts = {
+      type: "basic",
+      title: "Testing Notification",
+      message: "Carry on",
+    };
+
+    let id = await browser.notifications.create(opts);
+
+    browser.runtime.onMessage.addListener(async (message, sender) => {
+      await browser.tabs.remove(sender.tab.id);
+
+      // We should be able to clear the notification after creating and
+      // destroying the tab.html page.
+      let wasCleared = await browser.notifications.clear(id);
+      browser.test.assertTrue(wasCleared, "The notification was cleared.");
+      browser.test.notifyPass("notifications");
+    });
+
+    browser.tabs.create({url: browser.runtime.getURL("/tab.html")});
+  }
+
+  async function tabScript() {
+    // We should be able to see the notification created in the background page
+    // in this page.
+    let notifications = await browser.notifications.getAll();
+    browser.test.assertEq(1, Object.keys(notifications).length,
+                          "One notification found.");
+    browser.runtime.sendMessage("continue-test");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["notifications"],
+    },
+    background,
+    files: {
+      "tab.js": tabScript,
+      "tab.html": `<!DOCTYPE html><html><head>
+        <meta charset="utf-8">
+        <script src="tab.js"><\/script>
+      </head></html>`,
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitFinish("notifications");
+  await extension.unload();
+});
+
 </script>
 
 </body>