Notificaitons refactor step 2

ui/message_center/notification.h

Index: ui/message_center/notification.h
diff --git a/ui/message_center/notification.h b/ui/message_center/notification.h
index 7c27d4d6706f7676fc968e46afab5bcebadcbd94..6d20f7118028f75a4575f325f0a2b1223fc2118d 100644
--- a/ui/message_center/notification.h
+++ b/ui/message_center/notification.h
@@ -5,13 +5,15 @@
 #ifndef UI_MESSAGE_CENTER_NOTIFICATION_H_
 #define UI_MESSAGE_CENTER_NOTIFICATION_H_
 
+#include <string>
 #include <vector>
 
 #include "base/string16.h"
 #include "base/time.h"
+#include "base/values.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/message_center/message_center_export.h"
-#include "ui/notifications/notification_types.h"
+#include "ui/message_center/notification_types.h"
 
 namespace message_center {
 
@@ -19,38 +21,96 @@ struct MESSAGE_CENTER_EXPORT NotificationItem {
   string16 title;
   string16 message;
 
-  NotificationItem(string16 title, string16 message);
+  NotificationItem(const string16& title, const string16& message);
 };
 
-struct MESSAGE_CENTER_EXPORT Notification {
-  Notification();
+struct MESSAGE_CENTER_EXPORT ButtonInfo {
+  string16 title;
+  gfx::ImageSkia icon;
+
+  ButtonInfo(const string16& title);
+};
+
+class MESSAGE_CENTER_EXPORT Notification {
+ public:
+  Notification(NotificationType type,
+               const std::string& id,
+               const string16& title,
+               const string16& message,
+               const string16& display_source,
+               const std::string& extension_id,
+               const DictionaryValue* optional_fields);  // May be NULL.
   virtual ~Notification();
 
-  ui::notifications::NotificationType type;
-  std::string id;
-  string16 title;
-  string16 message;
-  string16 display_source;
-  std::string extension_id;
-
-  // Begin unpacked values from optional_fields
-  int priority;
-  base::Time timestamp;
-  int unread_count;
-  std::vector<string16> button_titles;
-  string16 expanded_message;
-  std::vector<NotificationItem> items;
-  // End unpacked values
-
-  // Images fetched asynchronously
-  gfx::ImageSkia primary_icon;
-  gfx::ImageSkia image;
-  std::vector<gfx::ImageSkia> button_icons;
-
-  bool is_read;  // True if this has been seen in the message center
-  bool shown_as_popup;  // True if this has been shown as a popup notification
+  NotificationType type() const { return type_; }
+  const std::string& id() const { return id_; }
+  const string16& title() const { return title_; }
+  const string16& message() const { return message_; }
+  const string16& display_source() const { return display_source_; }
+  const std::string& extension_id() const { return extension_id_; }
+
+  // Begin unpacked values from optional_fields.
+  int priority() { return priority_; }
+  base::Time timestamp() const { return timestamp_; }
+  int unread_count() const { return unread_count_; }
+  const string16& expanded_message() const { return expanded_message_; }
+  const std::vector<NotificationItem>& items() const { return items_; }
+  // End unpacked values.
+
+  // Images fetched asynchronously.
+  const gfx::ImageSkia& primary_icon() const { return primary_icon_; }
+  void set_primary_icon(const gfx::ImageSkia& icon) { primary_icon_ = icon; }
+
+  const gfx::ImageSkia& image() const { return image_; }
+  void set_image(const gfx::ImageSkia& image) { image_ = image; }
+
+  // Buttons, with icons fetched asynchronously.
+  const std::vector<ButtonInfo>& buttons() const { return buttons_; }
+  bool SetButtonIcon(size_t index, const gfx::ImageSkia& icon);
+
+  // Status in MessageCenter.
+  bool is_read() const { return is_read_; }
+  void set_is_read(bool is_read) { is_read_ = is_read; }
+
+  bool shown_as_popup() const { return shown_as_popup_; }
+  void set_shown_as_popup(bool shown_as_popup) {
+    shown_as_popup_ = shown_as_popup;
+  }
+
+  // Used to keep the order of notifications with the same timestamp.
+  // The notification with lesser serial_number is considered 'older'.
+  // Wrap up is not a concern since it only may affect visual sorting and it's
+  // very unlikely to have 4 Billion of notifications shown between reboots.

[stevenjb, 2013/02/25 18:11:08]

nit: last two lines should be well understood and are unnecessary. Also, is this
really necessary at all? Unless we're using time as a unique key, this seems
like we're adding complexity for an extremely unlikely edge case that we don't
really care about...?

[Dmitry Titov, 2013/02/25 23:21:15]

Updated comment.

We discussed sorting last week with the team and decided to sort on timestamp
(exposed as eventTime in API) in order to render. This is to address the
scenarios like a Chrombook waking up and potentially several apps getting a
bunch of push messages/updates about events happened in the last hour s - so
they can use eventTime reasonably to keep the notifications in the same order as
they are on the computer that was not sleeping.
At the same time, an app that posts 2 notifications in a row will naturally
expect the same order all the time, even if the timestamp is the same. Serial
number keeps the order automatically.
One of those apps are our own tests - they fire a lot of notifications (so the
timestamps often happen to be the same) and they rely on visual order that
corresponds the order of creating those notifications.

+  unsigned serial_number() { return serial_number_; }
+
+ private:
+  // Unpacks the provided |optional_fields| and applies the values to override
+  // the notification's data members.
+  void ApplyOptionalFields(const DictionaryValue* optional_fields);
+
+  NotificationType type_;
+  std::string id_;
+  string16 title_;
+  string16 message_;
+  string16 display_source_;
+  std::string extension_id_;
+  int priority_;
+  base::Time timestamp_;
+  unsigned serial_number_;
+  int unread_count_;
+  string16 expanded_message_;
+  std::vector<NotificationItem> items_;
+  gfx::ImageSkia primary_icon_;
+  gfx::ImageSkia image_;
+  std::vector<ButtonInfo> buttons_;
+  bool is_read_;  // True if this has been seen in the message center.
+  bool shown_as_popup_;  // True if this has been shown as a popup notification.
+
+  static unsigned s_next_serial_number_;

[stevenjb, 2013/02/25 18:11:08]

nit: make this file local instead of exposing it here

[Dmitry Titov, 2013/02/25 23:21:15]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(Notification);
 };
 
 }  // namespace message_center
 
-#endif // UI_MESSAGE_CENTER_NOTIFICATION_H_
+#endif  // UI_MESSAGE_CENTER_NOTIFICATION_H_