Implement notificationclose event

chrome/browser/notifications/platform_notification_service_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 49 matching lines @@
 using content::BrowserThread;
 using content::PlatformNotificationContext;
 using message_center::NotifierId;
 
 namespace {
 
 // Invalid id for a renderer process. Used in cases where we need to check for
 // permission without having an associated renderer process yet.
 const int kInvalidRenderProcessId = -1;
 
-// Callback to provide when deleting the data associated with persistent Web
-// Notifications from the notification database.
-void OnPersistentNotificationDataDeleted(bool success) {
-  UMA_HISTOGRAM_BOOLEAN("Notifications.PersistentNotificationDataDeleted",
-      success);
-}
-
 // Persistent notifications fired through the delegate do not care about the
 // lifetime of the Service Worker responsible for executing the event.
-void OnEventDispatchComplete(content::PersistentNotificationStatus status) {
+void OnClickEventDispatchComplete(
+    content::PersistentNotificationStatus status) {
   UMA_HISTOGRAM_ENUMERATION(
       "Notifications.PersistentWebNotificationClickResult", status,
       content::PersistentNotificationStatus::
           PERSISTENT_NOTIFICATION_STATUS_MAX);
 }
 
+void OnCloseEventDispatchComplete(
+    content::PersistentNotificationStatus status) {
+  // TODO(nsatragno): add Notifications.PersistentWebNotificationCloseResult
+  // metric.
+}
+
 void CancelNotification(const std::string& id, ProfileID profile_id) {
   PlatformNotificationServiceImpl::GetInstance()
       ->GetNotificationUIManager()->CancelById(id, profile_id);
 }
 
 // Callback to run once the profile has been loaded in order to perform a
 // given |operation| in a notification.
 void ProfileLoadedCallback(
     PlatformNotificationServiceImpl::NotificationOperation operation,
     const GURL& origin,
@@ skipping 100 matching lines @@
   if (action_index == -1) {
     content::RecordAction(base::UserMetricsAction(
         "Notifications.Persistent.Clicked"));
   } else {
     content::RecordAction(base::UserMetricsAction(
         "Notifications.Persistent.ClickedActionButton"));
   }
 
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
-            browser_context,
-            persistent_notification_id,
-            origin,
-            action_index,
-            base::Bind(&OnEventDispatchComplete));
+          browser_context, persistent_notification_id, origin, action_index,
+          base::Bind(&OnClickEventDispatchComplete));
 }
 
 void PlatformNotificationServiceImpl::OnPersistentNotificationClose(
     BrowserContext* browser_context,
     int64_t persistent_notification_id,
     const GURL& origin,
     bool by_user) const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (by_user) {
     content::RecordAction(base::UserMetricsAction(
         "Notifications.Persistent.ClosedByUser"));
+
+    content::NotificationEventDispatcher::GetInstance()
+        ->DispatchNotificationCloseEvent(
+            browser_context, persistent_notification_id, origin,
+            base::Bind(&OnCloseEventDispatchComplete));
   } else {
     content::RecordAction(base::UserMetricsAction(

[Peter Beverloo, 2016/01/26 16:13:45]

Even when a notification was clicked by the user, we'd still want it to be
removed from the NotificationDatabase. In other words, //content always has to
be told about the closure.

I think what we'll want here is something like...

if (by_user) { uma_one }
else { uma_two }

content::NED->DispatchNotificationCloseEvent(..., by_user);

and then make the decision on whether or not to fire the event in //content.

[Nina, 2016/01/27 18:48:58]

Good catch - I think this became an issue when I moved the deletion code to
//content.

         "Notifications.Persistent.ClosedProgrammatically"));
   }
-
-  PlatformNotificationContext* context =
-      BrowserContext::GetStoragePartitionForSite(browser_context, origin)
-          ->GetPlatformNotificationContext();
-
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&PlatformNotificationContext::DeleteNotificationData,
-                 context,
-                 persistent_notification_id,
-                 origin,
-                 base::Bind(&OnPersistentNotificationDataDeleted)));
 }
 
 blink::WebNotificationPermission
 PlatformNotificationServiceImpl::CheckPermissionOnUIThread(
     BrowserContext* browser_context,
     const GURL& origin,
     int render_process_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   Profile* profile = Profile::FromBrowserContext(browser_context);
@@ skipping 297 matching lines @@
         extensions::ExtensionRegistry::Get(profile)->GetExtensionById(
             origin.host(), extensions::ExtensionRegistry::EVERYTHING);
     DCHECK(extension);
 
     return base::UTF8ToUTF16(extension->name());
   }
 #endif
 
   return base::string16();
 }