From 9a97c9710a828904ac78fb2502a4518a0eb939e2 Mon Sep 17 00:00:00 2001 From: Alexandre Lissy Date: Thu, 14 May 2015 10:59:00 +0200 Subject: [PATCH] Bug 1159128 - Only save valid app notifications. r=mhenretty --HG-- extra : rebase_source : a1aa14fc6960a5f7c28d1abd918f3024eb329d4c --- .../notification/nsINotificationStorage.idl | 16 +++- dom/notification/NotificationDB.jsm | 22 ++++- dom/notification/NotificationStorage.js | 24 +++-- .../notification/NotificationTest.js | 17 ++++ .../mochitest/notification/mochitest.ini | 3 + .../test_notification_noresend.html | 88 +++++++++++++++++++ .../test_notification_resend.html | 28 +++--- .../content/SpecialPowersObserverAPI.js | 27 ++++++ .../specialpowers/content/specialpowersAPI.js | 17 ++++ 9 files changed, 222 insertions(+), 20 deletions(-) create mode 100644 dom/tests/mochitest/notification/test_notification_noresend.html diff --git a/dom/interfaces/notification/nsINotificationStorage.idl b/dom/interfaces/notification/nsINotificationStorage.idl index b6fda42a8b74..2362bfbd58b0 100644 --- a/dom/interfaces/notification/nsINotificationStorage.idl +++ b/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++ diff --git a/dom/notification/NotificationDB.jsm b/dom/notification/NotificationDB.jsm index fa5d54db6fbb..09cf91a85a84 100644 --- a/dom/notification/NotificationDB.jsm +++ b/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), diff --git a/dom/notification/NotificationStorage.js b/dom/notification/NotificationStorage.js index 6c3470d8654a..eec321f76312 100644 --- a/dom/notification/NotificationStorage.js +++ b/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) { diff --git a/dom/tests/mochitest/notification/NotificationTest.js b/dom/tests/mochitest/notification/NotificationTest.js index 4d684efa3bea..046460b989bf 100644 --- a/dom/tests/mochitest/notification/NotificationTest.js +++ b/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 { diff --git a/dom/tests/mochitest/notification/mochitest.ini b/dom/tests/mochitest/notification/mochitest.ini index 9f5e7323889b..19027cec30c1 100644 --- a/dom/tests/mochitest/notification/mochitest.ini +++ b/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 diff --git a/dom/tests/mochitest/notification/test_notification_noresend.html b/dom/tests/mochitest/notification/test_notification_noresend.html new file mode 100644 index 000000000000..7bd372db2ad3 --- /dev/null +++ b/dom/tests/mochitest/notification/test_notification_noresend.html @@ -0,0 +1,88 @@ + + + + Testing mozResendAllNotifications() resend behavior for Pages + + + + + + +Bug 1159128 +

+ +

+
+
+
diff --git a/dom/tests/mochitest/notification/test_notification_resend.html b/dom/tests/mochitest/notification/test_notification_resend.html
index 1e2d49f964fd..da5a84559d65 100644
--- a/dom/tests/mochitest/notification/test_notification_resend.html
+++ b/dom/tests/mochitest/notification/test_notification_resend.html
@@ -1,7 +1,7 @@
 
 
 
-  Testing mozResendAllNotifications() resend behavior
+  Testing mozResendAllNotifications() resend behavior for Apps
   
   
   
@@ -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();
   });
 
diff --git a/testing/specialpowers/content/SpecialPowersObserverAPI.js b/testing/specialpowers/content/SpecialPowersObserverAPI.js
index cbd4a3eaf061..6d5ab3b3661c 100644
--- a/testing/specialpowers/content/SpecialPowersObserverAPI.js
+++ b/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");
         }
diff --git a/testing/specialpowers/content/specialpowersAPI.js b/testing/specialpowers/content/specialpowersAPI.js
index 2051bc25055c..d4d4d645fcea 100644
--- a/testing/specialpowers/content/specialpowersAPI.js
+++ b/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;