Notificaitons refactor step 2

ui/message_center/notification_list.h

Index: ui/message_center/notification_list.h
diff --git a/ui/message_center/notification_list.h b/ui/message_center/notification_list.h
index ec8b4db65d362f5facbefb4482c0efcd778490a6..898454e499da30fdd45f5d22657c30949674f7b0 100644
--- a/ui/message_center/notification_list.h
+++ b/ui/message_center/notification_list.h
@@ -6,7 +6,6 @@
 #define UI_MESSAGE_CENTER_NOTIFICATION_LIST_H_
 
 #include <list>
-#include <map>
 #include <string>
 
 #include "base/string16.h"
@@ -16,7 +15,7 @@
 #include "ui/gfx/native_widget_types.h"
 #include "ui/message_center/message_center_export.h"
 #include "ui/message_center/notification.h"
-#include "ui/notifications/notification_types.h"
+#include "ui/message_center/notification_types.h"
 
 namespace base {
 class DictionaryValue;
@@ -27,7 +26,8 @@ namespace message_center {
 // A helper class to manage the list of notifications.
 class MESSAGE_CENTER_EXPORT NotificationList {
  public:
-  typedef std::list<Notification> Notifications;
+  // A 'list' because we use sort() to ensure display order by priority/time.

[dewittj, 2013/02/20 23:47:54]

What if we use a std::set<Notification*> which is sorted, to prevent re-sorting
all the time?  I might think we want two sets, a set of those notifications that
are waiting to be shown as popups and a set of notifications that are only in
message center (since you seem to be sorting them in different ways).

[Dmitry Titov, 2013/02/23 00:58:51]

Done.

+  typedef std::list<Notification*> Notifications;
 
   class MESSAGE_CENTER_EXPORT Delegate {
    public:
@@ -69,7 +69,7 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   // Affects whether or not a message has been "read".
   void SetMessageCenterVisible(bool visible);
 
-  void AddNotification(ui::notifications::NotificationType type,
+  void AddNotification(NotificationType type,
                        const std::string& id,
                        const string16& title,
                        const string16& message,
@@ -83,8 +83,7 @@ class MESSAGE_CENTER_EXPORT NotificationList {
                                  const string16& message,
                                  const base::DictionaryValue* optional_fields);
 
-  // Returns true if the notification was removed.
-  bool RemoveNotification(const std::string& id);
+  void RemoveNotification(const std::string& id);
 
   void RemoveAllNotifications();
 
@@ -111,9 +110,10 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   // means that all notifications have been shown).
   bool HasPopupNotifications();
 
-  // Modifies |notifications| to contain the |kMaxVisiblePopupNotifications|
-  // least recent notifications that have not been shown as a popup.
-  void GetPopupNotifications(Notifications* notifications);
+  // Returns the recent notifications of the priority higher then LOW,
+  // that have not been shown as a popup. kMaxVisiblePopupNotifications are
+  // used to limit the number of notifications for the DEFAULT priority.
+  Notifications GetPopupNotifications();
 
   // Marks the popups for the |priority| as shown.
   void MarkPopupsAsShown(int priority);
@@ -133,7 +133,8 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   // specified time-delta from now.
   void EnterQuietModeWithExpire(const base::TimeDelta& expires_in);
 
-  void GetNotifications(Notifications* notifications) const;
+  // Returns all notifications, in a (priority-timestamp) order.
+  const Notifications& GetNotifications();
   size_t NotificationCount() const;
   size_t unread_count() const { return unread_count_; }
 
@@ -141,35 +142,18 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   static const size_t kMaxVisibleMessageCenterNotifications;
 
  private:
-  typedef std::map<int, Notifications> NotificationMap;
-
-  // Iterates through the list and stores the first notification matching |id|
-  // (should always be unique) to |iter|. Returns true if it's found.
-  bool GetNotification(const std::string& id, Notifications::iterator* iter);
+  // Iterates through the list and returns the first notification matching |id|.
+  Notifications::iterator GetNotification(const std::string& id);
 
   void EraseNotification(Notifications::iterator iter);
 
-  void PushNotification(Notification& notification);
-
-  // Returns the recent notifications of the |priority| that have not been shown
-  // as a popup. kMaxVisiblePopupNotifications are used to limit the number of
-  // notifications for the default priority.
-  void GetPopupIterators(int priority,
-                         Notifications::iterator* first,
-                         Notifications::iterator* last);
-
-  // Given a dictionary of optional notification fields (or NULL), unpacks all
-  // recognized values into the given Notification struct. We assume prior
-  // proper initialization of |notification| fields that correspond to
-  // |optional_fields|.
-  void UnpackOptionalFields(const base::DictionaryValue* optional_fields,
-                            Notification* notification);
+  void PushNotification(scoped_ptr<Notification> notification);
 
   // Sets the current quiet mode status to |quiet_mode|.
   void SetQuietModeInternal(bool quiet_mode);
 
   Delegate* delegate_;
-  NotificationMap notifications_;
+  Notifications notifications_;

[Jun Mukai, 2013/02/20 22:52:16]

can we use std::priority_queue rather than std::list?  Then, you don't need to
sort at GetNotifications anymore.

[Dmitry Titov, 2013/02/23 00:58:51]

I've replaced it with a std::set<T*, Comparer>. It can keep the order while
inserting/removing.

Also, since we need 2 different sort orders - for popups and for Center - I have
2 sorted sets.

   bool message_center_visible_;
   size_t unread_count_;
   bool quiet_mode_;