Push API: Deflake PushEventEnforcesUserVisibleNotification

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..2f205d3bdeed37547aae2dfb52a64f65ef8ca5c3 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -184,11 +184,15 @@ void PushMessagingServiceImpl::ShutdownHandler() {
 void PushMessagingServiceImpl::OnMessage(
     const std::string& app_id,
     const gcm::GCMClient::IncomingMessage& message) {
+  base::Closure message_handled_closure =
+      message_callback_for_testing_.is_null() ? base::Bind(&base::DoNothing)
+                                              : message_callback_for_testing_;
   PushMessagingApplicationId application_id =
       PushMessagingApplicationId::Get(profile_, app_id);
   // Drop message and unregister if app id was unknown (maybe recently deleted).
   if (!application_id.IsValid()) {
     DeliverMessageCallback(app_id, GURL::EmptyGURL(), -1, message,
+                           message_handled_closure,
                            content::PUSH_DELIVERY_STATUS_UNKNOWN_APP_ID);
     return;
   }
@@ -196,7 +200,7 @@ void PushMessagingServiceImpl::OnMessage(
   if (!HasPermission(application_id.origin())) {
     DeliverMessageCallback(app_id, application_id.origin(),
                            application_id.service_worker_registration_id(),
-                           message,
+                           message, message_handled_closure,
                            content::PUSH_DELIVERY_STATUS_PERMISSION_DENIED);
     return;
   }
@@ -238,7 +242,8 @@ void PushMessagingServiceImpl::OnMessage(
       base::Bind(&PushMessagingServiceImpl::DeliverMessageCallback,
                  weak_factory_.GetWeakPtr(),
                  application_id.app_id_guid(), application_id.origin(),
-                 application_id.service_worker_registration_id(), message));
+                 application_id.service_worker_registration_id(), message,
+                 message_handled_closure));
 }
 
 void PushMessagingServiceImpl::DeliverMessageCallback(
@@ -246,6 +251,7 @@ void PushMessagingServiceImpl::DeliverMessageCallback(
     const GURL& requesting_origin,
     int64 service_worker_registration_id,
     const gcm::GCMClient::IncomingMessage& message,
+    const base::Closure& message_handled_closure,
     content::PushDeliveryStatus status) {
   // TODO(mvanouwerkerk): Show a warning in the developer console of the
   // Service Worker corresponding to app_id (and/or on an internals page).
@@ -256,22 +262,27 @@ void PushMessagingServiceImpl::DeliverMessageCallback(
     // deliberately failing in order to avoid having to show notifications).
     case content::PUSH_DELIVERY_STATUS_SUCCESS:
     case content::PUSH_DELIVERY_STATUS_EVENT_WAITUNTIL_REJECTED:
-      RequireUserVisibleUX(requesting_origin, service_worker_registration_id);
+      RequireUserVisibleUX(requesting_origin, service_worker_registration_id,
+                           message_handled_closure);
       break;
     case content::PUSH_DELIVERY_STATUS_INVALID_MESSAGE:
     case content::PUSH_DELIVERY_STATUS_SERVICE_WORKER_ERROR:
+      message_handled_closure.Run();
       break;
     case content::PUSH_DELIVERY_STATUS_UNKNOWN_APP_ID:
     case content::PUSH_DELIVERY_STATUS_PERMISSION_DENIED:
     case content::PUSH_DELIVERY_STATUS_NO_SERVICE_WORKER:
-      Unregister(app_id_guid, message.sender_id, UnregisterCallback());
+      Unregister(app_id_guid, message.sender_id,
+                 base::Bind(&UnregisterCallbackToClosure,
+                 message_handled_closure));
       break;
   }
   RecordDeliveryStatus(status);
 }
 
 void PushMessagingServiceImpl::RequireUserVisibleUX(
-    const GURL& requesting_origin, int64 service_worker_registration_id) {
+    const GURL& requesting_origin, int64 service_worker_registration_id,
+    const base::Closure& message_handled_closure) {
 #if defined(ENABLE_NOTIFICATIONS)
   // TODO(johnme): Relax this heuristic slightly.
   PlatformNotificationServiceImpl* notification_service =
@@ -342,11 +353,15 @@ void PushMessagingServiceImpl::RequireUserVisibleUX(
         base::Bind(&PushMessagingServiceImpl::DidGetNotificationsShown,
                    weak_factory_.GetWeakPtr(),
                    requesting_origin, service_worker_registration_id,
-                   notification_shown, notification_needed));
+                   notification_shown, notification_needed,
+                   message_handled_closure));
   } else {
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_AND_NOT_SHOWN);
+    message_handled_closure.Run();
   }
+#else
+  message_handled_closure.Run();
 #endif  // defined(ENABLE_NOTIFICATIONS)
 }
 
@@ -356,6 +371,7 @@ static void IgnoreResult(bool unused) {
 void PushMessagingServiceImpl::DidGetNotificationsShown(
     const GURL& requesting_origin, int64 service_worker_registration_id,
     bool notification_shown, bool notification_needed,
+    const base::Closure& message_handled_closure,
     const std::string& data, bool success, bool not_found) {
   content::ServiceWorkerContext* service_worker_context =
       content::BrowserContext::GetStoragePartitionForSite(
@@ -385,12 +401,14 @@ void PushMessagingServiceImpl::DidGetNotificationsShown(
         notification_needed
         ? content::PUSH_USER_VISIBLE_STATUS_REQUIRED_AND_SHOWN
         : content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_BUT_SHOWN);
+    message_handled_closure.Run();
     return;
   }
   if (needed_but_not_shown) {
     if (missed_notifications.count() <= 1) {  // apply grace
       RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_REQUIRED_BUT_NOT_SHOWN_USED_GRACE);
+      message_handled_closure.Run();
       return;
     }
     RecordUserVisibleStatus(
@@ -423,9 +441,15 @@ void PushMessagingServiceImpl::DidGetNotificationsShown(
         requesting_origin,
         SkBitmap() /* icon */,
         notification_data);
+    message_handled_closure.Run();
   }
 }
 
+void PushMessagingServiceImpl::SetMessageCallbackForTesting(
+      const base::Closure& callback) {
+  message_callback_for_testing_ = callback;
+}
+
 // Other gcm::GCMAppHandler methods -------------------------------------------
 
 void PushMessagingServiceImpl::OnMessagesDeleted(const std::string& app_id) {