Replace the 1 in 10 grace period with an accumulating budget based on SES.

chrome/browser/push_messaging/push_messaging_notification_manager.cc

Index: chrome/browser/push_messaging/push_messaging_notification_manager.cc
diff --git a/chrome/browser/push_messaging/push_messaging_notification_manager.cc b/chrome/browser/push_messaging/push_messaging_notification_manager.cc
index 8ac84582c9d696c9afac4c7a1e472eb683d7e660..9164121f1b14e6181f496ece2d4b1a3ea33e3084 100644
--- a/chrome/browser/push_messaging/push_messaging_notification_manager.cc
+++ b/chrome/browser/push_messaging/push_messaging_notification_manager.cc
@@ -186,20 +186,29 @@ void PushMessagingNotificationManager::DidGetNotificationsFromDatabase(
     }
   }
 
-  // Don't track push messages that didn't show a notification but were exempt
-  // from needing to do so.
-  if (notification_shown || notification_needed) {
+  if (notification_needed && !notification_shown) {
+    // Only track budget for messages that needed to show a notification but
+    // did not.
     BackgroundBudgetService* service =
         BackgroundBudgetServiceFactory::GetForProfile(profile_);
-    std::string notification_history = service->GetBudget(origin);
-    DidGetBudget(origin, service_worker_registration_id, notification_shown,
-                 notification_needed, message_handled_closure,
-                 notification_history);
-  } else {
+    double budget = service->GetBudget(origin);
+    DidGetBudget(origin, service_worker_registration_id,
+                 message_handled_closure, budget);
+    return;
+  }
+
+  if (notification_needed && notification_shown) {
+    RecordUserVisibleStatus(
+        content::PUSH_USER_VISIBLE_STATUS_REQUIRED_AND_SHOWN);
+  } else if (!notification_needed && !notification_shown) {
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_AND_NOT_SHOWN);
-    message_handled_closure.Run();
+  } else {
+    RecordUserVisibleStatus(
+        content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_BUT_SHOWN);
   }
+
+  message_handled_closure.Run();
 }
 
 bool PushMessagingNotificationManager::IsTabVisible(
@@ -238,46 +247,26 @@ bool PushMessagingNotificationManager::IsTabVisible(
 void PushMessagingNotificationManager::DidGetBudget(
     const GURL& origin,
     int64_t service_worker_registration_id,
-    bool notification_shown,
-    bool notification_needed,
     const base::Closure& message_handled_closure,
-    const std::string& data) {
+    const double budget) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // We remember whether the last (up to) 10 pushes showed notifications.
-  const size_t MISSED_NOTIFICATIONS_LENGTH = 10;
-  // data is a string like "0001000", where '0' means shown, and '1' means
-  // needed but not shown. We manipulate it in bitset form.
-  std::bitset<MISSED_NOTIFICATIONS_LENGTH> missed_notifications(data);
-
-  DCHECK(notification_shown || notification_needed);  // Caller must ensure this
-  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>());
-  BackgroundBudgetService* service =
-      BackgroundBudgetServiceFactory::GetForProfile(profile_);
-  service->StoreBudget(origin, updated_data);
-
-  if (notification_shown) {
-    RecordUserVisibleStatus(
-        notification_needed
-            ? content::PUSH_USER_VISIBLE_STATUS_REQUIRED_AND_SHOWN
-            : content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_BUT_SHOWN);
-    message_handled_closure.Run();
-    return;
-  }
-  DCHECK(needed_but_not_shown);
-  if (missed_notifications.count() <= 1) {  // Apply grace.
+  // If the service needed to show a notification but did not, update the
+  // budget. Currently this just subtracts 1.0, but we may expand the cost to
+  // include things like whether wifi is available in the mobile case.
+  double cost = 1.0;

[Peter Beverloo, 2016/05/04 15:41:52]

nit: this should be a constant at the top of this file

[harkness, 2016/05/09 15:33:27]

Done.

+  if (budget >= cost) {
+    // Update the stored budget.
+    BackgroundBudgetService* service =
+        BackgroundBudgetServiceFactory::GetForProfile(profile_);
+    service->StoreBudget(origin, budget - cost);
+
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_REQUIRED_BUT_NOT_SHOWN_USED_GRACE);

[Peter Beverloo, 2016/05/04 15:41:52]

I think we should introduce a new enum value for budget, so that we can
distinguish patterns more easily from grace uses.

[harkness, 2016/05/09 15:33:27]

Do you mind if I do that in my next CL, when I do all my uma changes, since I'll
be touching all those files for that already?

[Peter Beverloo, 2016/05/09 17:12:06]

OK.

     message_handled_closure.Run();
     return;
   }
+
   RecordUserVisibleStatus(
       content::PUSH_USER_VISIBLE_STATUS_REQUIRED_BUT_NOT_SHOWN_GRACE_EXCEEDED);
   rappor::SampleDomainAndRegistryFromGURL(