Widen Data Pipes and newer protobufs

chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc

Index: chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
index c58e0e6d89579fe57a9c49a8287271d1a15786da..29baf2046d568558355d628b5167086e1ea18878 100644
--- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
+++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
@@ -61,9 +61,9 @@ syncer::SyncMergeResult ChromeNotifierService::MergeDataAndStartSyncing(
     DCHECK(incoming.get());
 
     // Process each incoming remote notification.
-    const std::string& id = incoming->notification_id();
-    DCHECK_GT(id.length(), 0U);
-    SyncedNotification* found = FindNotificationById(id);
+    const std::string& key = incoming->key();
+    DCHECK_GT(key.length(), 0U);
+    SyncedNotification* found = FindNotificationByKey(key);
 
     if (NULL == found) {
       // If there are no conflicts, copy in the data from remote.
@@ -175,7 +175,7 @@ syncer::SyncData ChromeNotifierService::CreateSyncDataFromNotification(
     const SyncedNotification& notification) {
   // Construct the sync_data using the specifics from the notification.
   return syncer::SyncData::CreateLocalData(
-      notification.notification_id(), notification.notification_id(),
+      notification.key(), notification.key(),
       notification.GetEntitySpecifics());
 }
 
@@ -190,9 +190,8 @@ scoped_ptr<SyncedNotification>
   // Check for mandatory fields in the sync_data object.
   if (!specifics.has_coalesced_notification() ||
       !specifics.coalesced_notification().has_key() ||
-      !specifics.coalesced_notification().has_read_state()) {
+      !specifics.coalesced_notification().has_read_state())
     return scoped_ptr<SyncedNotification>();
-  }
 
   // TODO(petewil): Is this the right set?  Should I add more?
   bool is_well_formed_unread_notification =
@@ -219,9 +218,8 @@ scoped_ptr<SyncedNotification>
 
 // This returns a pointer into a vector that we own.  Caller must not free it.
 // Returns NULL if no match is found.
-// This uses the <app_id/coalescing_key> pair as a key.
-SyncedNotification* ChromeNotifierService::FindNotificationById(
-    const std::string& id) {
+SyncedNotification* ChromeNotifierService::FindNotificationByKey(
+    const std::string& key) {
   // TODO(petewil): We can make a performance trade off here.
   // While the vector has good locality of reference, a map has faster lookup.
   // Based on how big we expect this to get, maybe change this to a map.
@@ -230,15 +228,16 @@ SyncedNotification* ChromeNotifierService::FindNotificationById(
       it != notification_data_.end();
       ++it) {
     SyncedNotification* notification = *it;
-    if (id == notification->notification_id())
+    if (key == notification->key())
       return *it;
   }
 
   return NULL;
 }
 
-void ChromeNotifierService::MarkNotificationAsDismissed(const std::string& id) {
-  SyncedNotification* notification = FindNotificationById(id);
+void ChromeNotifierService::MarkNotificationAsDismissed(
+    const std::string& key) {
+  SyncedNotification* notification = FindNotificationByKey(key);
   CHECK(notification != NULL);
 
   notification->NotificationHasBeenDismissed();
@@ -270,37 +269,99 @@ void ChromeNotifierService::Show(SyncedNotification* notification) {
   // Set up the fields we need to send and create a Notification object.
   GURL origin_url(notification->origin_url());
   GURL app_icon_url(notification->app_icon_url());
+  GURL image_url(notification->image_url());
   string16 title = UTF8ToUTF16(notification->title());
   string16 text = UTF8ToUTF16(notification->text());
   string16 heading = UTF8ToUTF16(notification->heading());
   string16 description = UTF8ToUTF16(notification->description());
-
+  double creation_time = static_cast<double>(notification->creation_time());
   // TODO(petewil): What goes in the display source, is empty OK?
   string16 display_source;
-  string16 replace_id = UTF8ToUTF16(notification->notification_id());
+  string16 replace_key = UTF8ToUTF16(notification->key());
+  int priority = notification->priority();
+  int notification_count = notification->notification_count();
+  int button_count = notification->button_count();
+  string16 button_one_title = UTF8ToUTF16(notification->button_one_title());

[dcheng, 2013/03/26 17:49:29]

Comment about UTF8 to UTF16 conversion still applies here and several other
places as well. Unfortunately, doing this just means the data goes from UTF8 to
UTF16 to UTF8.

[Pete Williamson, 2013/03/27 17:07:53]

Done.

+  std::string button_one_icon_url = notification->button_one_icon_url();
+  string16 button_two_title = UTF8ToUTF16(notification->button_two_title());
+  std::string button_two_icon_url = notification->button_two_icon_url();
+
+  // Deduce which notification template to use from the data.
+  message_center::NotificationType notification_type =
+      message_center::NOTIFICATION_TYPE_SIMPLE;
+  if (!image_url.is_empty()) {
+    notification_type = message_center::NOTIFICATION_TYPE_IMAGE;
+  } else if (notification_count > 1) {
+    notification_type = message_center::NOTIFICATION_TYPE_MULTIPLE;
+  } else if (button_count > 0) {
+    notification_type = message_center::NOTIFICATION_TYPE_BASE_FORMAT;
+  }
+
+  // Fill the optional fields with the information we need to make a
+  // notification.
+  scoped_ptr<DictionaryValue> optional_fields(new DictionaryValue());

[dcheng, 2013/03/26 17:49:29]

Consider just saying:
DictionaryValue optional_fields;
if we don't need to transfer ownership.

[Pete Williamson, 2013/03/27 17:07:53]

Done.

+  optional_fields->SetDouble(message_center::kTimestampKey, creation_time);
+  if (priority != 0)

[dcheng, 2013/03/26 17:49:29]

What correlation does priority have to timestamp? Why is it OK to treat a
non-zero priority as a timestamp? Also, isn't this originally an enum?

[Pete Williamson, 2013/03/27 17:07:53]

None, this is a bug, fixed.

+    optional_fields->SetInteger(message_center::kTimestampKey, priority);
+  if (!button_one_title.empty())
+    optional_fields->SetString(message_center::kButtonOneTitleKey,
+                               button_one_title);
+  if (!button_one_icon_url.empty())
+    optional_fields->SetString(message_center::kButtonOneIconUrlKey,
+                               button_one_icon_url);
+  if (!button_two_title.empty())
+    optional_fields->SetString(message_center::kButtonTwoTitleKey,
+                               button_two_title);
+  if (!button_two_icon_url.empty())
+    optional_fields->SetString(message_center::kButtonTwoIconUrlKey,
+                               button_two_icon_url);
+
+  // Fill the individual notification fields for a mutiple notification.
+  if (notification_count > 1) {
+    base::ListValue* items = new base::ListValue();
+
+    for (int ii = 0; ii < notification_count; ++ii) {
+      DictionaryValue* item = new DictionaryValue();
+      item->SetString(message_center::kItemTitleKey,
+                      UTF8ToUTF16(notification->contained_notification_title(
+                          ii)));
+      item->SetString(message_center::kItemMessageKey,
+                      UTF8ToUTF16(notification->contained_notification_message(
+                          ii)));
+      items->Append(item);
+    }
+
+    optional_fields->Set(message_center::kItemsKey, items);
+  }
 
   // TODO(petewil): For now, just punt on dismissed notifications until
   // I change the interface to let NotificationUIManager know the right way.
   if (SyncedNotification::kRead == notification->read_state() ||
       SyncedNotification::kDismissed == notification->read_state() ) {
-    DVLOG(2) << "Not showing dismissed notification"
-              << notification->title() << " " << notification->text();
+    DVLOG(2) << "Dismissed notification arrived"
+             << notification->title() << " " << notification->text();
     return;
   }
-
   // The delegate will eventually catch calls that the notification
   // was read or deleted, and send the changes back to the server.
   scoped_refptr<NotificationDelegate> delegate =
-      new ChromeNotifierDelegate(notification->notification_id(), this);
+      new ChromeNotifierDelegate(notification->key(), this);
 
-  Notification ui_notification(origin_url, app_icon_url, heading, description,
+  Notification ui_notification(notification_type,
+                               origin_url,
+                               app_icon_url,
+                               heading,
+                               text,
                                WebKit::WebTextDirectionDefault,
-                               display_source, replace_id, delegate);
+                               display_source,
+                               replace_key,
+                               optional_fields.get(),
+                               delegate);
 
   notification_manager_->Add(ui_notification, profile_);
 
   DVLOG(1) << "Synced Notification arrived! " << title << " " << text
-           << " " << app_icon_url << " " << replace_id << " "
+           << " " << app_icon_url << " " << replace_key << " "
            << notification->read_state();
 
   return;