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

chrome/browser/extensions/app_notification_manager.cc

 // Copyright (c) 2011 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/extensions/app_notification_manager.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/location.h"
@@ skipping 75 matching lines @@
 
 void AppNotificationManager::Init() {
   FilePath storage_path = profile_->GetPath().AppendASCII("App Notifications");
   BrowserThread::PostTask(
       BrowserThread::FILE,
       FROM_HERE,
       base::Bind(&AppNotificationManager::LoadOnFileThread,
           this, storage_path));
 }
 
+bool AppNotificationSortPredicate(const linked_ptr<AppNotification> a1,
+                                  const linked_ptr<AppNotification> a2) {
+  return a1.get()->creation_timestamp_ms() < a2.get()->creation_timestamp_ms();
+}
+
 bool AppNotificationManager::Add(AppNotification* item) {
   // Do this first since we own the incoming item and hence want to delete
   // it in error paths.
   linked_ptr<AppNotification> linked_item(item);
   if (!loaded())
     return false;
   const std::string& extension_id = item->extension_id();
   AppNotificationList& list = GetAllInternal(extension_id);
   list.push_back(linked_item);
 
   SyncAddChange(*linked_item);
 
+  if (list.size() > notification_manager_constants::kMaxNotificationPerApp) {
+    AppNotification* removed =
+        std::min_element(list.begin(), list.end(),
+                         AppNotificationSortPredicate)->get();
+    SyncRemoveChange(*removed, true);
+    list.erase(list.begin());
+  }

[Munjal (Google), 2011/11/17 19:45:32]

Actually, I am wondering if we should add in a sorted manner into the list. That
way we can always return the head in GetLast and delete the last one here (or
vice versa depending on whether sort is asc or desc).

[asargent_no_longer_on_chrome, 2011/11/17 22:36:24]

+1 - keeping them sorted is a good idea

+
   if (storage_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE,
         FROM_HERE,
         base::Bind(&AppNotificationManager::SaveOnFileThread,
             this, extension_id, CopyAppNotificationList(list)));
   }
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
@@ skipping 26 matching lines @@
 const AppNotification* AppNotificationManager::GetLast(
     const std::string& extension_id) {
   if (!loaded())
     return NULL;
   NotificationMap::iterator found = notifications_->find(extension_id);
   if (found == notifications_->end())
     return NULL;
   const AppNotificationList& list = found->second;
   if (list.empty())
     return NULL;
-  return list.rbegin()->get();
+  return std::max_element(list.begin(), list.end(),
+                          AppNotificationSortPredicate)->get();
 }
 
 void AppNotificationManager::ClearAll(const std::string& extension_id) {
   if (!loaded())
     return;
   NotificationMap::iterator found = notifications_->find(extension_id);
   if (found != notifications_->end()) {
     SyncClearAllChange(found->second);
     notifications_->erase(found);
   }
@@ skipping 240 matching lines @@
     return;
 
   // TODO(munjal): crbug.com/10059. Work with Lingesh/Antony to resolve.
 
   SyncChangeList changes;
   SyncData sync_data = CreateSyncDataFromNotification(notif);
   changes.push_back(SyncChange(SyncChange::ACTION_ADD, sync_data));
   sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
+void AppNotificationManager::SyncRemoveChange(const AppNotification& notif,
+                                              bool shouldProcessSyncerChanges) {

[Munjal (Google), 2011/11/17 19:45:32]

Actually seems like you call this with true for second parameter. Remove second
parameter then?

[elvin, 2011/11/17 22:28:17]

Done.

+  // Skip if either:
+  // - Sync is not enabled by user.
+  // - Change is generated from within the manager.
+  if (notif.is_local() ||
+       ((!models_associated_ || processing_syncer_changes_)
+         && !shouldProcessSyncerChanges))

[Munjal (Google), 2011/11/17 19:45:32]

Don't we want this to happen even when we are processing syncer changes?

Would the following suffice?
if (notif.is_local() || !modles_associated_)
  reutnr;

[elvin, 2011/11/17 22:28:17]

Done.

+    return;

[Munjal (Google), 2011/11/17 19:45:32]

If your if is multiple lines you need braces even for single line in the body.

[elvin, 2011/11/17 22:28:17]

Done.

+
+  // TODO(munjal): crbug.com/10059. Work with Lingesh/Antony to resolve.

[Munjal (Google), 2011/11/17 19:45:32]

This TODO is not relevant here I think.

[elvin, 2011/11/17 22:28:17]

Done.

+
+  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:
   // - Sync is not enabled by user.
   // - Change is generated from within the manager.
   if (!models_associated_ || processing_syncer_changes_)
     return;
 
   SyncChangeList changes;
   for (AppNotificationList::const_iterator iter = list.begin();
@@ skipping 37 matching lines @@
 }
 
 // static
 SyncData AppNotificationManager::CreateSyncDataFromNotification(
     const AppNotification& notification) {
   DCHECK(!notification.is_local());
   sync_pb::EntitySpecifics specifics;
   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_timestamp_ms());
   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);
 }
 
 // static
 AppNotification* AppNotificationManager::CreateNotificationFromSyncData(
     const SyncData& sync_data) {
   sync_pb::AppNotificationSpecifics specifics =
       sync_data.GetSpecifics().GetExtension(sync_pb::app_notification);
 
   // 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, 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());
   if (specifics.has_link_url())
     notification->set_link_url(GURL(specifics.link_url()));
   return notification;
 }