Bug 1159128 - Only save valid app notifications. r=mhenretty

dom/interfaces/notification/nsINotificationStorage.idl

@@ -41,7 +41,7 @@ interface nsINotificationStorageCallback : nsISupports
 /**
  * Interface for notification persistence layer.
  */
-[scriptable, uuid(f5145be6-e34b-468b-84da-c8c4c1ad60fe)]
+[scriptable, uuid(cac01fb0-c2eb-4252-b2f4-5b1fac933bd4)]
 interface nsINotificationStorage : nsISupports
 {
 
@@ -94,6 +94,20 @@ interface nsINotificationStorage : nsISupports
    */
   void delete(in DOMString origin,
               in DOMString id);
+
+  /**
+   * Notifications are not supposed to be persistent, according to spec, at
+   * least for now. But we want to be able to have this behavior on B2G. Thus,
+   * this method will check if the origin sending the notifications is a valid
+   * registered app with a manifest or not. Hence, a webpage that has none
+   * will have its notification sent and available (via Notification.get())
+   * during the life time of the page.
+   *
+   * @param origin: Origin from which the notification is sent.
+   *
+   * @return boolean
+   */
+  boolean canPut(in DOMString origin);
 };
 
 %{C++

dom/notification/NotificationDB.jsm

@@ -24,6 +24,10 @@ XPCOMUtils.defineLazyServiceGetter(this, "ppmm",
                                    "@mozilla.org/parentprocessmessagemanager;1",
                                    "nsIMessageListenerManager");
 
+XPCOMUtils.defineLazyServiceGetter(this, "notificationStorage",
+                                   "@mozilla.org/notificationStorage;1",
+                                   "nsINotificationStorage");
+
 const NOTIFICATION_STORE_DIR = OS.Constants.Path.profileDir;
 const NOTIFICATION_STORE_PATH =
         OS.Path.join(NOTIFICATION_STORE_DIR, "notificationstore.json");
@@ -77,14 +81,29 @@ let NotificationDB = {
     }
   },
 
+  filterNonAppNotifications: function(notifications) {
+    let origins = Object.keys(notifications);
+    for (let origin of origins) {
+      let canPut = notificationStorage.canPut(origin);
+      if (!canPut) {
+        if (DEBUG) debug("Origin " + origin + " is not linked to an app manifest, deleting.");
+	delete notifications[origin];
+      }
+    }
+    return notifications;
+  },
+
   // Attempt to read notification file, if it's not there we will create it.
   load: function() {
     var promise = OS.File.read(NOTIFICATION_STORE_PATH, { encoding: "utf-8"});
     return promise.then(
       function onSuccess(data) {
         if (data.length > 0) {
-          this.notifications = JSON.parse(data);
+	  // Preprocessing phase intends to cleanly separate any migration-related
+          // tasks.
+	  this.notifications = this.filterNonAppNotifications(JSON.parse(data));
         }
+
         // populate the list of notifications by tag
         if (this.notifications) {
           for (var origin in this.notifications) {
@@ -97,6 +116,7 @@ let NotificationDB = {
             }
           }
         }
+
         this.loaded = true;
       }.bind(this),
 

dom/notification/NotificationStorage.js

@@ -23,6 +23,10 @@ XPCOMUtils.defineLazyServiceGetter(this, "cpmm",
                                    "@mozilla.org/childprocessmessagemanager;1",
                                    "nsIMessageSender");
 
+XPCOMUtils.defineLazyServiceGetter(this, "appsService",
+                                   "@mozilla.org/AppsService;1",
+                                   "nsIAppsService");
+
 const kMessageNotificationGetAllOk = "Notification:GetAll:Return:OK";
 const kMessageNotificationGetAllKo = "Notification:GetAll:Return:KO";
 const kMessageNotificationSaveKo   = "Notification:Save:Return:KO";
@@ -45,6 +49,7 @@ function NotificationStorage() {
   this._requestCount = 0;
 
   Services.obs.addObserver(this, "xpcom-shutdown", false);
+
   // Register for message listeners.
   this.registerListeners();
 }
@@ -65,12 +70,19 @@ NotificationStorage.prototype = {
 
   observe: function(aSubject, aTopic, aData) {
     if (DEBUG) debug("Topic: " + aTopic);
-    if (aTopic == "xpcom-shutdown") {
+    if (aTopic === "xpcom-shutdown") {
       Services.obs.removeObserver(this, "xpcom-shutdown");
       this.unregisterListeners();
     }
   },
 
+  canPut: function(aOrigin) {
+    if (DEBUG) debug("Querying appService for: " + aOrigin);
+    let rv = !!appsService.getAppByManifestURL(aOrigin);
+    if (DEBUG) debug("appService returned: " + rv);
+    return rv;
+  },
+
   put: function(origin, id, title, dir, lang, body, tag, icon, alertName,
                 data, behavior) {
     if (DEBUG) { debug("PUT: " + id + ": " + title); }
@@ -105,10 +117,12 @@ NotificationStorage.prototype = {
       this._byTag[origin][tag] = notification;
     };
 
-    cpmm.sendAsyncMessage("Notification:Save", {
-      origin: origin,
-      notification: notification
-    });
+    if (this.canPut(origin)) {
+      cpmm.sendAsyncMessage("Notification:Save", {
+        origin: origin,
+        notification: notification
+      });
+    }
   },
 
   get: function(origin, tag, callback) {

dom/tests/mochitest/notification/NotificationTest.js

@@ -44,6 +44,19 @@ var NotificationTest = (function () {
     })(tests);
   }
 
+  function fakeApp(aManifest) {
+    var aApp = {
+      "origin": "{mochitest}",
+      "manifestURL": aManifest
+    };
+
+    SpecialPowers.injectApp("{mochitest}", aApp);
+  }
+
+  function unfakeApp() {
+    SpecialPowers.rejectApp("{mochitest}");
+  }
+
   var fakeCustomData = (function () {
     var buffer = new ArrayBuffer(2);
     var dv = new DataView(buffer).setInt16(0, 42, true);
@@ -107,6 +120,10 @@ var NotificationTest = (function () {
 
     info: info,
 
+    fakeApp: fakeApp,
+
+    unfakeApp: unfakeApp,
+
     customDataMatches: function(dataObj) {
       var url = "http://www.domain.com";
       try {

dom/tests/mochitest/notification/mochitest.ini

@@ -10,3 +10,6 @@ skip-if = toolkit == 'android' || toolkit == 'gonk' #bug 960762
 [test_bug931307.html]
 skip-if = (toolkit == 'gonk' && debug) #debug-only timeout
 [test_notification_resend.html]
+skip-if = e10s # On e10s, faking the app seems to be failing
+[test_notification_noresend.html]
+skip-if = (toolkit == 'gonk') # Mochitest on Gonk registers an app manifest that messes with the logic

dom/tests/mochitest/notification/test_notification_noresend.html

@@ -0,0 +1,88 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Testing mozResendAllNotifications() resend behavior for Pages</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="MockServices.js"></script>
+  <script type="text/javascript" src="NotificationTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1159128">Bug 1159128</a>
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<pre id="test"></pre>
+<script type="text/javascript">
+  var info = NotificationTest.info;
+  var notifications = [];
+
+  SimpleTest.requestFlakyTimeout("untriaged");
+
+  var steps = [
+    function (done) {
+      if (window.Notification) {
+        SpecialPowers.pushPrefEnv({"set": [
+          ["dom.ignore_webidl_scope_checks", true],
+          ]}, done);
+      } else {
+        ok(true, "Notifications are not enabled on the platform.");
+        done();
+      }
+    },
+
+    function (done) {
+      info("Test that we have mozChromeNotifications API");
+      ok(('mozChromeNotifications' in navigator), "should have mozChromeNotifications API");
+      ok(('mozResendAllNotifications' in navigator.mozChromeNotifications), "should have mozResendAllNotifications()");
+      done();
+    },
+
+    function (done) {
+      info("Making sure we have no previous notification pending");
+      var promise = Notification.get();
+      promise.then(function (notifications) {
+        is(notifications.length, 0, "notifications are all cleaned");
+        done();
+      });
+    },
+
+    // The notification is expected to be created and living properly
+    // so it will be accessible via Notification.get(), but NotificationStorage
+    // should not have sent it to NotificationDB.
+    function (done) {
+      info("Sending one notification");
+      var notif = new Notification("title");
+      ok(notif, "Notification object is valid");
+      notifications.push(notif);
+
+      var promise = Notification.get();
+      promise.then(function (notifications) {
+        is(notifications.length, 1, "one notification has been sent");
+        done();
+      });
+    },
+
+    // mozResendAllNotifications will poke directly NotificationDB, so we
+    // expect our notification to NOT have been put there and thus not being
+    // resent.
+    function (done) {
+      info("Trying to resend the notification");
+      var notif = notifications.pop();
+      notif.onclose = function() {
+        done();
+      };
+
+      navigator.mozChromeNotifications.mozResendAllNotifications(function(number) {
+        is(number, 0, "No notification resent");
+        notif.close();
+      });
+    }
+  ];
+
+  MockServices.register();
+  NotificationTest.run(steps, function () {
+    MockServices.unregister();
+  });
+</script>
+</body>
+</html>

dom/tests/mochitest/notification/test_notification_resend.html

@@ -1,7 +1,7 @@
 <!DOCTYPE HTML>
 <html>
 <head>
-  <title>Testing mozResendAllNotifications() resend behavior</title>
+  <title>Testing mozResendAllNotifications() resend behavior for Apps</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="text/javascript" src="MockServices.js"></script>
   <script type="text/javascript" src="NotificationTest.js"></script>
@@ -21,17 +21,6 @@
   SimpleTest.requestFlakyTimeout("untriaged");
 
   var steps = [
-    function (done) {
-      if (window.Notification) {
-        SpecialPowers.pushPrefEnv({"set": [
-          ["dom.ignore_webidl_scope_checks", true],
-          ]}, done);
-      } else {
-        ok(true, "Notifications are not enabled on the platform.");
-        done();
-      }
-    },
-
     function (done) {
       info("Set manifestURL");
       var request = window.navigator.mozApps.getSelf();
@@ -46,6 +35,18 @@
       };
     },
 
+    function (done) {
+      if (window.Notification) {
+        NotificationTest.fakeApp(manifestURL);
+        SpecialPowers.pushPrefEnv({"set": [
+          ["dom.ignore_webidl_scope_checks", true],
+          ]}, done);
+      } else {
+        ok(true, "Notifications are not enabled on the platform.");
+        done();
+      }
+    },
+
     function (done) {
       info("Test that we have mozChromeNotifications API");
       ok(('mozChromeNotifications' in navigator), "should have mozChromeNotifications API");
@@ -194,11 +195,12 @@
 
         notif2.close();
       });
-    }
+    },
   ];
 
   MockServices.register();
   NotificationTest.run(steps, function () {
+    NotificationTest.unfakeApp();
     MockServices.unregister();
   });
 </script>

testing/specialpowers/content/SpecialPowersObserverAPI.js

@@ -375,6 +375,33 @@ SpecialPowersObserverAPI.prototype = {
               scope.UserCustomizations._debug = aMessage.json.value;
               return;
             }
+          case "inject-app":
+	    {
+              let aAppId = aMessage.json.appId;
+	      let aApp   = aMessage.json.app;
+
+              let keys = Object.keys(Webapps.DOMApplicationRegistry.webapps);
+	      let exists = keys.indexOf(aAppId) !== -1;
+	      if (exists) {
+                return false;
+	      }
+
+              Webapps.DOMApplicationRegistry.webapps[aAppId] = aApp;
+	      return true;
+	    }
+	  case "reject-app":
+	    {
+              let aAppId = aMessage.json.appId;
+
+              let keys = Object.keys(Webapps.DOMApplicationRegistry.webapps);
+	      let exists = keys.indexOf(aAppId) !== -1;
+	      if (!exists) {
+                return false;
+	      }
+
+              delete Webapps.DOMApplicationRegistry.webapps[aAppId];
+	      return true;
+	    }
           default:
             throw new SpecialPowersError("Invalid operation for SPWebAppsService");
         }

testing/specialpowers/content/specialpowersAPI.js

@@ -1135,6 +1135,23 @@ SpecialPowersAPI.prototype = {
     });
   },
 
+  // Force-registering an app in the registry
+  injectApp: function(aAppId, aApp) {
+    this._sendSyncMessage("SPWebAppService", {
+      op: "inject-app",
+      appId: aAppId,
+      app: aApp
+    });
+  },
+
+  // Removing app from the registry
+  rejectApp: function(aAppId) {
+    this._sendSyncMessage("SPWebAppService", {
+      op: "reject-app",
+      appId: aAppId
+    });
+  },
+
   _proxiedObservers: {
     "specialpowers-http-notify-request": function(aMessage) {
       let uri = aMessage.json.uri;