Push API: Forced notifications should use Notifications database

chrome/browser/push_messaging/push_messaging_service_impl.cc

Index: chrome/browser/push_messaging/push_messaging_service_impl.cc
diff --git a/chrome/browser/push_messaging/push_messaging_service_impl.cc b/chrome/browser/push_messaging/push_messaging_service_impl.cc
index 215b479f9717fc82e275a731c48b73cb80469d22..a4db90daee1b68ee5eaa1398d3e5cf80b57880d5 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -37,6 +37,8 @@
 #include "components/rappor/rappor_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_database_data.h"
+#include "content/public/browser/platform_notification_context.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -274,18 +276,25 @@ void PushMessagingServiceImpl::RequireUserVisibleUX(
     const GURL& requesting_origin, int64 service_worker_registration_id) {
 #if defined(ENABLE_NOTIFICATIONS)
   // TODO(johnme): Relax this heuristic slightly.
-  PlatformNotificationServiceImpl* notification_service =
-      PlatformNotificationServiceImpl::GetInstance();
-  // Can't use g_browser_process->notification_ui_manager(), since the test uses
-  // PlatformNotificationServiceImpl::SetNotificationUIManagerForTesting.
-  // TODO(peter): Remove the need to use both APIs here once Notification.get()
-  // is supported.
-  int notification_count = notification_service->GetNotificationUIManager()->
-      GetAllIdsByProfileAndSourceOrigin(profile_, requesting_origin).size();
+  content::PlatformNotificationContext* notification_context =
+      content::BrowserContext::GetStoragePartitionForSite(
+          profile_, requesting_origin)->GetPlatformNotificationContext();
+  notification_context->ReadAllNotificationDataForServiceWorkerRegistration(

[Peter Beverloo, 2015/04/21 18:06:17]

We're on the UI thread here, right? We need to call
ReadAllNotificationDataForServiceWorkerRegistration on the IO thread. This will
cause a DCHECK.

[johnme, 2015/04/23 16:51:17]

Done (heh, this was just an early work in progress).

+      requesting_origin, service_worker_registration_id,
+      base::Bind(&PushMessagingServiceImpl::DidGetNotificationsShowing,
+                 weak_factory_.GetWeakPtr(),
+                 requesting_origin, service_worker_registration_id));
+#endif  // defined(ENABLE_NOTIFICATIONS)
+}
+
+void PushMessagingServiceImpl::DidGetNotificationsShowing(
+    const GURL& requesting_origin, int64 service_worker_registration_id,
+    bool success, const std::vector<content::NotificationDatabaseData>& data) {
   // TODO(johnme): Hiding an existing notification should also count as a useful
   // user-visible action done in response to a push message - but make sure that
   // sending two messages in rapid succession which show then hide a
   // notification doesn't count.
+  int notification_count = success ? data.size() : 0;
   bool notification_shown = notification_count > 0;
 
   bool notification_needed = true;
@@ -347,7 +356,6 @@ void PushMessagingServiceImpl::RequireUserVisibleUX(
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_AND_NOT_SHOWN);
   }
-#endif  // defined(ENABLE_NOTIFICATIONS)
 }
 
 static void IgnoreResult(bool unused) {