Limit number of notifications that can be received by the client.

chrome/browser/extensions/app_notification_manager.cc

Index: chrome/browser/extensions/app_notification_manager.cc
===================================================================
--- chrome/browser/extensions/app_notification_manager.cc	(revision 109957)
+++ chrome/browser/extensions/app_notification_manager.cc	(working copy)
@@ -93,6 +93,12 @@
           this, storage_path));
 }
 
+bool AppNotificationSortPredicate(const linked_ptr<AppNotification> a1,
+                                  const linked_ptr<AppNotification> a2) {
+  return a1.get()->creation_time().ToInternalValue() <
+      a2.get()->creation_time().ToInternalValue();

[asargent_no_longer_on_chrome, 2011/11/17 23:40:14]

nit: Time has operator< and operator> methods, so you can skip the call to
ToInternalValue here and just compare the creation_time's directly.

[elvin, 2011/11/18 00:10:18]

Done.

+}
+
 bool AppNotificationManager::Add(AppNotification* item) {
   // Do this first since we own the incoming item and hence want to delete
   // it in error paths.
@@ -105,6 +111,14 @@
 
   SyncAddChange(*linked_item);
 
+  if (list.size() > notification_manager_constants::kMaxNotificationPerApp) {
+    AppNotification* removed =
+        std::min_element(list.begin(), list.end(),
+                         AppNotificationSortPredicate)->get();
+    SyncRemoveChange(*removed);
+    list.erase(list.begin());
+  }
+
   if (storage_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE,
@@ -151,7 +165,8 @@
   const AppNotificationList& list = found->second;
   if (list.empty())
     return NULL;
-  return list.rbegin()->get();
+  return std::max_element(list.begin(), list.end(),
+                          AppNotificationSortPredicate)->get();

[asargent_no_longer_on_chrome, 2011/11/17 23:40:14]

I guess it's a little strange that GetLast returns the one with the highest
timestamp, but GetAll above can return the items in a potentially unsorted
order. No need to hold up this CL, but I'll think about whether we should always
sort or switch to storing items in a std::set or something instead of a list.

 }
 
 void AppNotificationManager::ClearAll(const std::string& extension_id) {
@@ -412,6 +427,20 @@
   sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
+void AppNotificationManager::SyncRemoveChange(const AppNotification& notif) {
+  // Skip if either:
+  // - Sync is not enabled by user.
+  // - Change is generated from within the manager.
+  if (notif.is_local() || !models_associated_) {
+    return;
+  }
+
+  SyncChangeList changes;
+  SyncData sync_data = CreateSyncDataFromNotification(notif);
+  changes.push_back(SyncChange(SyncChange::ACTION_DELETE, sync_data));
+  sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
+}
+
 void AppNotificationManager::SyncClearAllChange(
   const AppNotificationList& list) {
   // Skip if either:
@@ -469,12 +498,13 @@
   sync_pb::AppNotificationSpecifics* notif_specifics =
       specifics.MutableExtension(sync_pb::app_notification);
   notif_specifics->set_app_id(notification.extension_id());
+  notif_specifics->set_creation_timestamp_ms(
+      notification.creation_time().ToInternalValue());
   notif_specifics->set_body_text(notification.body());
   notif_specifics->set_guid(notification.guid());
   notif_specifics->set_link_text(notification.link_text());
   notif_specifics->set_link_url(notification.link_url().spec());
   notif_specifics->set_title(notification.title());
-
   return SyncData::CreateLocalData(
       notif_specifics->guid(), notif_specifics->app_id(), specifics);
 }
@@ -487,12 +517,14 @@
 
   // Check for mandatory fields.
   if (!specifics.has_app_id() || !specifics.has_guid() ||
-      !specifics.has_title() || !specifics.has_body_text()) {
+      !specifics.has_title() || !specifics.has_body_text() ||
+      !specifics.has_creation_timestamp_ms()) {
     return NULL;
   }
 
   AppNotification* notification = new AppNotification(
-      false, specifics.guid(), specifics.app_id(),
+      false, base::Time::FromInternalValue(specifics.creation_timestamp_ms()),
+      specifics.guid(), specifics.app_id(),
       specifics.title(), specifics.body_text());
   if (specifics.has_link_text())
     notification->set_link_text(specifics.link_text());