Deflake PushMessagingBrowserTest.DenyPushPermissionUnregisters & friends

chrome/browser/push_messaging/push_messaging_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/push_messaging/push_messaging_service_impl.h"
 
 #include <bitset>
 #include <vector>
 
+#include "base/barrier_closure.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 299 matching lines @@
   }
 #endif
 
   // Don't track push messages that didn't show a notification but were exempt
   // from needing to do so.
   if (notification_shown || notification_needed) {
     content::ServiceWorkerContext* service_worker_context =
         content::BrowserContext::GetStoragePartitionForSite(
             profile_, requesting_origin)->GetServiceWorkerContext();
 
-    PushMessagingService::GetNotificationsShownByLastFewPushes(
+    GetNotificationsShownByLastFewPushes(
         service_worker_context, service_worker_registration_id,
         base::Bind(&PushMessagingServiceImpl::DidGetNotificationsShown,
                    weak_factory_.GetWeakPtr(),
                    requesting_origin, service_worker_registration_id,
                    notification_shown, notification_needed));
   } else {
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_AND_NOT_SHOWN);
   }
 #endif  // defined(ENABLE_NOTIFICATIONS)
@@ skipping 17 matching lines @@
   std::bitset<MISSED_NOTIFICATIONS_LENGTH> missed_notifications(data);
 
   bool needed_but_not_shown = notification_needed && !notification_shown;
 
   // New entries go at the end, and old ones are shifted off the beginning once
   // the history length is exceeded.
   missed_notifications <<= 1;
   missed_notifications[0] = needed_but_not_shown;
   std::string updated_data(missed_notifications.
       to_string<char, std::string::traits_type, std::string::allocator_type>());
-  PushMessagingService::SetNotificationsShownByLastFewPushes(
+  SetNotificationsShownByLastFewPushes(
       service_worker_context, service_worker_registration_id,
       requesting_origin, updated_data,
       base::Bind(&IgnoreResult));  // This is a heuristic; ignore failure.
 
   if (notification_shown) {
     RecordUserVisibleStatus(
         notification_needed
         ? content::PUSH_USER_VISIBLE_STATUS_REQUIRED_AND_SHOWN
         : content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_BUT_SHOWN);
     return;
@@ skipping 309 matching lines @@
 void PushMessagingServiceImpl::OnContentSettingChanged(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
     ContentSettingsType content_type,
     std::string resource_identifier) {
   if (content_type != CONTENT_SETTINGS_TYPE_PUSH_MESSAGING &&
       content_type != CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
     return;
   }
 
-  for (const auto& id : PushMessagingApplicationId::GetAll(profile_)) {
+  std::vector<PushMessagingApplicationId> all_app_ids =
+      PushMessagingApplicationId::GetAll(profile_);
+
+  base::Closure barrier_closure = base::BarrierClosure(
+      all_app_ids.size(),
+      content_setting_changed_callback_for_testing_.is_null()
+      ? base::Bind(&base::DoNothing)

[Bernhard Bauer, 2015/03/12 17:46:51]

I think I would indent this four more spaces?

[johnme, 2015/03/13 11:56:13]

Done.

+      : content_setting_changed_callback_for_testing_);
+  content_setting_changed_callback_for_testing_.Reset();
+
+  for (const auto& id : all_app_ids) {
     // If |primary_pattern| is not valid, we should always check for a
     // permission change because it can happen for example when the entire
     // Push or Notifications permissions are cleared.
     // Otherwise, the permission should be checked if the pattern matches the
     // origin.
-    if (primary_pattern.IsValid() && !primary_pattern.Matches(id.origin()))
+    if (primary_pattern.IsValid() && !primary_pattern.Matches(id.origin())) {
+      barrier_closure.Run();
       continue;
+    }
 
-    if (HasPermission(id.origin()))
+    if (HasPermission(id.origin())) {
+      barrier_closure.Run();
       continue;
+    }
 
-    PushMessagingService::GetSenderId(
+    GetSenderId(
         profile_, id.origin(), id.service_worker_registration_id(),
         base::Bind(
             &PushMessagingServiceImpl::UnregisterBecausePermissionRevoked,
-            weak_factory_.GetWeakPtr(), id));
+            weak_factory_.GetWeakPtr(), id, barrier_closure));
   }
 }
 
+static void UnregisterCallbackToClosure(

[Bernhard Bauer, 2015/03/12 17:46:51]

Move this to an anonymous namespace instead of making it static?

[johnme, 2015/03/13 11:56:13]

Done.

+    const base::Closure& closure, content::PushUnregistrationStatus status) {
+  closure.Run();
+}
+
 void PushMessagingServiceImpl::UnregisterBecausePermissionRevoked(
-    const PushMessagingApplicationId& id,
+    const PushMessagingApplicationId& id, const base::Closure& closure,
     const std::string& sender_id, bool success, bool not_found) {
+  base::Closure barrier_closure = base::BarrierClosure(2, closure);
+
   // Unregister the PushMessagingApplicationId with the push service.
-  Unregister(id.app_id_guid(), sender_id, UnregisterCallback());
+  Unregister(id.app_id_guid(), sender_id,
+             base::Bind(&UnregisterCallbackToClosure, barrier_closure));
 
   // Clear the associated service worker push registration id.
-  PushMessagingService::ClearPushRegistrationID(
-      profile_, id.origin(), id.service_worker_registration_id());
+  ClearPushRegistrationID(profile_, id.origin(),
+                          id.service_worker_registration_id(), barrier_closure);
+}
+
+void PushMessagingServiceImpl::SetContentSettingChangedCallbackForTesting(
+      const base::Closure& callback) {
+  content_setting_changed_callback_for_testing_ = callback;
 }
 
 // KeyedService methods -------------------------------------------------------
 
 void PushMessagingServiceImpl::Shutdown() {
   GetGCMDriver()->RemoveAppHandler(kPushMessagingApplicationIdPrefix);
 }
 
 // Helper methods --------------------------------------------------------------
 
 bool PushMessagingServiceImpl::HasPermission(const GURL& origin) {
   PushMessagingPermissionContext* permission_context =
       PushMessagingPermissionContextFactory::GetForProfile(profile_);
   DCHECK(permission_context);
 
   return permission_context->GetPermissionStatus(origin, origin) ==
       CONTENT_SETTING_ALLOW;
 }
 
 gcm::GCMDriver* PushMessagingServiceImpl::GetGCMDriver() const {
   gcm::GCMProfileService* gcm_profile_service =
       gcm::GCMProfileServiceFactory::GetForProfile(profile_);
   CHECK(gcm_profile_service);
   CHECK(gcm_profile_service->driver());
   return gcm_profile_service->driver();
 }