Bug 1256635 - Implement browser.notifications.onClicked, r=kmag
authorbsilverberg <bsilverberg@mozilla.com>
Wed, 16 Mar 2016 16:49:10 +0100
changeset 291367 f91054a848d692f7c6f66a5be829ffa5421a898a
parent 291366 0228e00caf591a590ac553d3ccf297c647b4c9c0
child 291368 5fbd6d5079f498694e20506054759c867b30a622
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1256635
milestone48.0a1
Bug 1256635 - Implement browser.notifications.onClicked, r=kmag Note that we cannot add a real test for onClicked without mocking out the AlertService. I did test it manually and it seems to work fine. MozReview-Commit-ID: 68nooxoUJat
toolkit/components/extensions/ext-notifications.js
toolkit/components/extensions/test/mochitest/test_ext_notifications.html
--- a/toolkit/components/extensions/ext-notifications.js
+++ b/toolkit/components/extensions/ext-notifications.js
@@ -1,24 +1,25 @@
 "use strict";
 
 var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "EventEmitter",
+                                  "resource://devtools/shared/event-emitter.js");
+
 var {
   EventManager,
   ignoreEvent,
 } = ExtensionUtils;
 
 // WeakMap[Extension -> Map[id -> Notification]]
 var notificationsMap = new WeakMap();
 
-// WeakMap[Extension -> Set[callback]]
-var notificationCallbacksMap = new WeakMap();
-
 // Manages a notification popup (notifications API) created by the extension.
 function Notification(extension, id, options) {
   this.extension = extension;
   this.id = id;
   this.options = options;
 
   let imageURL;
   if (options.iconUrl) {
@@ -46,40 +47,39 @@ Notification.prototype = {
       svc.closeAlert(this.id);
     } catch (e) {
       // This will fail if the OS doesn't support this function.
     }
     notificationsMap.get(this.extension).delete(this.id);
   },
 
   observe(subject, topic, data) {
-    if (topic != "alertfinished") {
-      return;
+    if (topic === "alertclickcallback") {
+      notificationsMap.get(this.extension).emit("clicked", data);
     }
-
-    for (let callback of notificationCallbacksMap.get(this.extension)) {
-      callback(this);
+    if (topic === "alertfinished") {
+      notificationsMap.get(this.extension).emit("closed", data);
     }
 
     notificationsMap.get(this.extension).delete(this.id);
   },
 };
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("startup", (type, extension) => {
-  notificationsMap.set(extension, new Map());
-  notificationCallbacksMap.set(extension, new Set());
+  let map = new Map();
+  EventEmitter.decorate(map);
+  notificationsMap.set(extension, map);
 });
 
 extensions.on("shutdown", (type, extension) => {
   for (let notification of notificationsMap.get(extension).values()) {
     notification.clear();
   }
   notificationsMap.delete(extension);
-  notificationCallbacksMap.delete(extension);
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 var nextId = 0;
 
 extensions.registerSchemaAPI("notifications", "notifications", (extension, context) => {
   return {
     notifications: {
@@ -114,25 +114,35 @@ extensions.registerSchemaAPI("notificati
         let result = {};
         notificationsMap.get(extension).forEach((value, key) => {
           result[key] = value.options;
         });
         return Promise.resolve(result);
       },
 
       onClosed: new EventManager(context, "notifications.onClosed", fire => {
-        let listener = notification => {
+        let listener = (event, notificationId) => {
           // FIXME: Support the byUser argument.
-          fire(notification.id, true);
+          fire(notificationId, true);
         };
 
-        notificationCallbacksMap.get(extension).add(listener);
+        notificationsMap.get(extension).on("closed", listener);
         return () => {
-          notificationCallbacksMap.get(extension).delete(listener);
+          notificationsMap.get(extension).off("closed", listener);
         };
       }).api(),
 
-      // FIXME
+      onClicked: new EventManager(context, "notifications.onClicked", fire => {
+        let listener = (event, notificationId) => {
+          fire(notificationId, true);
+        };
+
+        notificationsMap.get(extension).on("clicked", listener);
+        return () => {
+          notificationsMap.get(extension).off("clicked", listener);
+        };
+      }).api(),
+
+      // Intend to implement this later: https://bugzilla.mozilla.org/show_bug.cgi?id=1190681
       onButtonClicked: ignoreEvent(context, "notifications.onButtonClicked"),
-      onClicked: ignoreEvent(context, "notifications.onClicked"),
     },
   };
 });
--- a/toolkit/components/extensions/test/mochitest/test_ext_notifications.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_notifications.html
@@ -34,24 +34,31 @@ add_task(function* test_notification() {
   });
   yield extension.startup();
   let x = yield extension.awaitMessage("running");
   is(x, "0", "got correct id from notifications.create");
   yield extension.awaitFinish();
   yield extension.unload();
 });
 
-add_task(function* test_notification_on_closed() {
+add_task(function* test_notification_events() {
   function backgroundScript() {
     let opts = {
       type: "basic",
       title: "Testing Notification",
       message: "Carry on",
     };
 
+    // Test an ignored listener.
+    browser.notifications.onButtonClicked.addListener(function() {});
+
+    // We cannot test onClicked listener without a mock
+    // but we can attempt to add a listener.
+    browser.notifications.onClicked.addListener(function() {});
+
     // Test onClosed listener.
     browser.notifications.onClosed.addListener(id => {
       browser.test.sendMessage("closed", id);
       browser.test.notifyPass("background test passed");
     });
 
     browser.notifications.create("5", opts).then(id => {
       return browser.notifications.create("5", opts);