Bug 1054415 - delete notification from the database when observing alertfinished. r=mhenretty

b2g/components/AlertsService.js

@@ -21,7 +21,6 @@ XPCOMUtils.defineLazyServiceGetter(this, "notificationStorage",
                                    "@mozilla.org/notificationStorage;1",
                                    "nsINotificationStorage");
 
-
 XPCOMUtils.defineLazyGetter(this, "cpmm", function() {
   return Cc["@mozilla.org/childprocessmessagemanager;1"]
            .getService(Ci.nsIMessageSender);
@@ -162,13 +161,13 @@ AlertsService.prototype = {
           );
         }
       }
+      if (topic === kTopicAlertFinished && listener.dbId) {
+        notificationStorage.delete(listener.manifestURL, listener.dbId);
+      }
     }
 
     // we're done with this notification
     if (topic === kTopicAlertFinished) {
-      if (listener.dbId) {
-        notificationStorage.delete(listener.manifestURL, listener.dbId);
-      }
       delete this._listeners[data.uid];
     }
   },

dom/notification/Notification.cpp

@@ -81,6 +81,8 @@ public:
       return rv.ErrorCode();
     }
 
+    notification->SetStoredState(true);
+
     JSAutoCompartment ac(aCx, mGlobal);
     JS::Rooted<JSObject*> element(aCx, notification->WrapObject(aCx));
     NS_ENSURE_TRUE(element, NS_ERROR_FAILURE);
@@ -369,6 +371,18 @@ NotificationObserver::Observe(nsISupports* aSubject, const char* aTopic,
     }
     mNotification->DispatchTrustedEvent(NS_LITERAL_STRING("click"));
   } else if (!strcmp("alertfinished", aTopic)) {
+    nsCOMPtr<nsINotificationStorage> notificationStorage =
+      do_GetService(NS_NOTIFICATION_STORAGE_CONTRACTID);
+    if (notificationStorage && mNotification->IsStored()) {
+      nsString origin;
+      nsresult rv = Notification::GetOrigin(mNotification->GetOwner(), origin);
+      if (NS_SUCCEEDED(rv)) {
+        nsString id;
+        mNotification->GetID(id);
+        notificationStorage->Delete(origin, id);
+      }
+      mNotification->SetStoredState(false);
+    }
     mNotification->mIsClosed = true;
     mNotification->DispatchTrustedEvent(NS_LITERAL_STRING("close"));
   } else if (!strcmp("alertshow", aTopic)) {
@@ -384,7 +398,7 @@ Notification::Notification(const nsAString& aID, const nsAString& aTitle, const
                            nsPIDOMWindow* aWindow)
   : DOMEventTargetHelper(aWindow),
     mID(aID), mTitle(aTitle), mBody(aBody), mDir(aDir), mLang(aLang),
-    mTag(aTag), mIconUrl(aIconUrl), mIsClosed(false)
+    mTag(aTag), mIconUrl(aIconUrl), mIsClosed(false), mIsStored(false)
 {
   nsAutoString alertName;
   DebugOnly<nsresult> rv = GetOrigin(GetOwner(), alertName);
@@ -475,6 +489,8 @@ Notification::Constructor(const GlobalObject& aGlobal,
     return nullptr;
   }
 
+  notification->SetStoredState(true);
+
   return notification.forget();
 }
 
@@ -781,7 +797,7 @@ Notification::Close()
 void
 Notification::CloseInternal()
 {
-  if (!mIsClosed) {
+  if (mIsStored) {
     // Don't bail out if notification storage fails, since we still
     // want to send the close event through the alert service.
     nsCOMPtr<nsINotificationStorage> notificationStorage =
@@ -793,7 +809,9 @@ Notification::CloseInternal()
         notificationStorage->Delete(origin, mID);
       }
     }
-
+    SetStoredState(false);
+  }
+  if (!mIsClosed) {
     nsCOMPtr<nsIAlertsService> alertService =
       do_GetService(NS_ALERTSERVICE_CONTRACTID);
     if (alertService) {

dom/notification/Notification.h

@@ -77,6 +77,16 @@ public:
     aRetval = mIconUrl;
   }
 
+  void SetStoredState(bool val)
+  {
+    mIsStored = val;
+  }
+
+  bool IsStored()
+  {
+    return mIsStored;
+  }
+
   nsIStructuredCloneContainer* GetDataCloneContainer();
 
   static void RequestPermission(const GlobalObject& aGlobal,
@@ -167,6 +177,12 @@ protected:
 
   bool mIsClosed;
 
+  // We need to make a distinction between the notification being closed i.e.
+  // removed from any pending or active lists, and the notification being
+  // removed from the database. NotificationDB might fail when trying to remove
+  // the notification.
+  bool mIsStored;
+
   static uint32_t sCount;
 
 private:

dom/tests/mochitest/notification/MockServices.js

@@ -16,13 +16,26 @@ var MockServices = (function () {
 
   var activeAppNotifications = Object.create(null);
 
+  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;
+      }
+    }
+  });
+
   var mockAlertsService = {
     showAlertNotification: function(imageUrl, title, text, textClickable,
                                     cookie, alertListener, name) {
       var listener = SpecialPowers.wrap(alertListener);
       activeAlertNotifications[name] = {
         listener: listener,
-        cookie: cookie
+        cookie: cookie,
+        title: title
       };
 
       // fake async alert show event
@@ -105,6 +118,7 @@ var MockServices = (function () {
     },
 
     activeAlertNotifications: activeAlertNotifications,
+
     activeAppNotifications: activeAppNotifications,
   };
 })();

dom/tests/mochitest/notification/NotificationTest.js

@@ -96,6 +96,14 @@ var NotificationTest = (function () {
       // TODO: how??
     },
 
+    fireCloseEvent: function (title) {
+      window.dispatchEvent(new CustomEvent("mock-notification-close-event", {
+        detail: {
+          title: title
+        }
+      }));
+    },
+
     info: info,
 
     customDataMatches: function(dataObj) {

dom/tests/mochitest/notification/test_notification_storage.html

@@ -119,10 +119,27 @@
       promise.then(function (notifications) {
         is(notifications.length, 3, "should return 3 notifications");
         done();
+        n1.close();
+        n2.close();
+        n3.close();
       });
     },
 
-    deleteAllNotifications
+    deleteAllNotifications,
+
+    function (done) {
+      info("Testing 'alertfinished' removes the notification from DB");
+      var n = new Notification("test-title" + Math.random());
+      n.onclose = function() {
+        Notification.get().then(function(notifications) {
+            is(notifications.length, 0, "should return 0 notifications");
+            done();
+        });
+      }
+      window.setTimeout(function() {
+          NotificationTest.fireCloseEvent(n.title);
+      }, 100);
+    }
   ];
 
   MockServices.register();