Adapting the UI to bring it closer to the spec, and fixing image fetching.

chrome/browser/notifications/sync_notifier/synced_notification.cc

 // Copyright (c) 2012 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/notifications/sync_notifier/synced_notification.h"
 
 #include "base/basictypes.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/protocol/synced_notification_specifics.pb.h"
 #include "ui/gfx/image/image.h"
 #include "ui/message_center/message_center_util.h"
 #include "ui/message_center/notification_types.h"
 
 namespace {
 const char kExtensionScheme[] = "chrome-extension://";
+const char kDefaultSyncedNotificationScheme[] = "https:";
 
 // Today rich notifications only supports two buttons, make sure we don't
 // try to supply them with more than this number of buttons.
 const unsigned int kMaxNotificationButtonIndex = 2;
 
 bool UseRichNotifications() {
   return message_center::IsRichNotificationEnabled();
 }
 
+// Schema-less specs default badly in windows.  If we find one, add the schema
+// we expect instead of allowing windows specific GURL code to make it default
+// to "file:".
+std::string AddDefaultSchemaIfNeeded(std::string& url_spec) {

[dewittj, 2013/07/19 22:37:19]

This function makes me sad. Is there anything else we can do within GURL?  like
has_scheme? Or if that doesn't work, just disallow file:// images in
notifications entirely and do a match using SchemeIsFile or SchemeIsFileSystem?

Worst case I'd prefer you to use the string_util StartsWithASCII function
instead of this hand-rolled one.

[Pete Williamson, 2013/07/19 22:49:07]

Yes, I share your sadness for this function.  My first plan is to get the server
side to send URLs with a full scheme instead of schemeless.

I could definitely convert this into a GURL, then use GURL functions to find out
if it has file scheme.  The File scheme is being added by the GURL constructor
itself, so if I did that, I would have to parse the URL once to see that it had
a file: scheme, and then I would fixup the URL, and later parse it again.  I
think that URL parsing is a bit expensive, which is why I opted for a cheaper
fix of just checking the first two characters. 

I did look for some functions that could easily tell me this was a schemeless
URL, but I did not find anything, and I've very open if any of the reviewers
have one to suggest.  If not, I plan to go with StartsWithAscii as you suggest.

+#ifdef OS_WIN
+  if (url_spec.length() > 1 && url_spec[0]  == '/' && url_spec[1] == '/')
+    return std::string(kDefaultSyncedNotificationScheme) + url_spec;
+#endif  // OS_WINDOWS
+
+  return url_spec;
+}
+
 }  // namespace
 
 namespace notifier {
 
 COMPILE_ASSERT(static_cast<sync_pb::CoalescedSyncedNotification_ReadState>(
                    SyncedNotification::kUnread) ==
                sync_pb::CoalescedSyncedNotification_ReadState_UNREAD,
                local_enum_must_match_protobuf_enum);
 COMPILE_ASSERT(static_cast<sync_pb::CoalescedSyncedNotification_ReadState>(
                    SyncedNotification::kRead) ==
@@ skipping 26 matching lines @@
 }
 
 void SyncedNotification::OnFetchComplete(const GURL url,
                                          const SkBitmap* bitmap) {
   // TODO(petewil): Add timeout mechanism in case bitmaps take too long.  Do we
   // already have one built into URLFetcher?
   // Make sure we are on the thread we expect.
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // Match the incoming bitmaps to URLs.  In case this is a dup, make sure to
-  // Try all potentially matching urls.
+  // try all potentially matching urls.
   if (GetAppIconUrl() == url && bitmap != NULL) {
     app_icon_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
   }
   if (GetImageUrl() == url && bitmap != NULL) {
     image_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
   }
+  if (GetProfilePictureUrl(0) == url && bitmap != NULL) {
+    sender_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
+  }
 
   // If this URL matches one or more button bitmaps, save them off.
   for (unsigned int i = 0; i < GetButtonCount(); ++i) {
     if (GetButtonIconUrl(i) == url && bitmap != NULL)
       button_bitmaps_[i] = gfx::Image::CreateFrom1xBitmap(*bitmap);
   }
 
   // Count off the bitmaps as they arrive.
   --active_fetcher_count_;
   DCHECK_GE(active_fetcher_count_, 0);
@@ skipping 20 matching lines @@
   profile_ = profile;
   DCHECK_EQ(active_fetcher_count_, 0);
 
   // Ensure our bitmap vector has as many entries as there are buttons,
   // so that when the bitmaps arrive the vector has a slot for them.
   for (unsigned int i = 0; i < GetButtonCount(); ++i) {
     button_bitmaps_.push_back(gfx::Image());
     AddBitmapToFetchQueue(GetButtonIconUrl(i));
   }
 
+  // If there is a profile image bitmap, fetch it
+  if (GetProfilePictureCount() > 0) {
+    // TODO(petewil): When we have the capacity to display more than one bitmap,
+    // modify this code to fetch as many as we can display
+    AddBitmapToFetchQueue(GetProfilePictureUrl(0));
+  }
+
   // If the URL is non-empty, add it to our queue of URLs to fetch.
   AddBitmapToFetchQueue(GetAppIconUrl());
   AddBitmapToFetchQueue(GetImageUrl());
 
   // If there are no bitmaps, call show now.
   if (active_fetcher_count_ == 0) {
     Show(notification_manager, notifier_service, profile);
   }
 }
 
@@ skipping 28 matching lines @@
     notification_manager->CancelById(GetKey());
     DVLOG(2) << "Dismissed notification arrived"
              << GetHeading() << " " << GetText();
     return;
   }
 
   // Set up the fields we need to send and create a Notification object.
   GURL image_url = GetImageUrl();
   string16 text = UTF8ToUTF16(GetText());
   string16 heading = UTF8ToUTF16(GetHeading());
+  string16 description = UTF8ToUTF16(GetDescription());
+  string16 annotation = UTF8ToUTF16(GetAnnotation());
   // TODO(petewil): Eventually put the display name of the sending service here.
   string16 display_source = UTF8ToUTF16(GetOriginUrl().spec());
   string16 replace_key = UTF8ToUTF16(GetKey());
+  string16 notification_heading = heading;
+  string16 notification_text = text;
 
   // 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(GetKey(), notifier_service);
 
   // Some inputs and fields are only used if there is a notification center.
   if (UseRichNotifications()) {
     base::Time creation_time =
         base::Time::FromDoubleT(static_cast<double>(GetCreationTime()));
@@ skipping 15 matching lines @@
     // Fill the optional fields with the information we need to make a
     // notification.
     message_center::RichNotificationData rich_notification_data;
     rich_notification_data.timestamp = creation_time;
     if (priority != SyncedNotification::kUndefinedPriority)
       rich_notification_data.priority = priority;
 
     // Fill in the button data.
     // TODO(petewil): Today Rich notifiations are limited to two buttons.
     // When rich notifications supports more, remove the
-    // "&& i < kMaxNotificationButtonIndex" below.
+    // "&& i < kMaxNotificationButtonIndex" clause below.
     for (unsigned int i = 0;
          i < button_count
          && i < button_bitmaps_.size()
          && i < kMaxNotificationButtonIndex;
          ++i) {
       // Stop at the first button with no title
       std::string title = GetButtonTitle(i);
       if (title.empty())
         break;
       message_center::ButtonInfo button_info(UTF8ToUTF16(title));
       if (!button_bitmaps_[i].IsEmpty())
         button_info.icon = button_bitmaps_[i];
       rich_notification_data.buttons.push_back(button_info);
     }
 
     // Fill in the bitmap images.
     if (!image_bitmap_.IsEmpty())
       rich_notification_data.image = image_bitmap_;
 
     // Fill the individual notification fields for a multiple notification.
     if (notification_count > 1) {
       for (int ii = 0; ii < notification_count; ++ii) {
         message_center::NotificationItem item(
             UTF8ToUTF16(GetContainedNotificationTitle(ii)),
             UTF8ToUTF16(GetContainedNotificationMessage(ii)));
         rich_notification_data.items.push_back(item);
       }
     }
 
+    // Set the heading and text appropriately for the message type.
+    notification_text = annotation;
+    if (notification_type == message_center::NOTIFICATION_TYPE_IMAGE) {
+      // For an image, fill in the description field.
+      notification_text = description;
+    } else if (notification_count == 1) {
+      // For a single collapsed info entry, use the contained message if any.
+      std::string comment_body = GetContainedNotificationMessage(0);
+      std::string comment_header = GetContainedNotificationTitle(0);
+      if (!comment_header.empty() && !comment_body.empty())
+        notification_text = UTF8ToUTF16(comment_header) + UTF8ToUTF16(" ") +
+            UTF8ToUTF16(comment_body);
+    }
+
+    // If there is a single person sending, use their picture instead of the app
+    // icon.
+    // TODO(petewil): Someday combine multiple profile photos here.
+    gfx::Image icon_bitmap = app_icon_bitmap_;
+    if (GetProfilePictureCount() == 1)  {
+      icon_bitmap = sender_bitmap_;
+    }
+
     Notification ui_notification(notification_type,
                                  GetOriginUrl(),
-                                 heading,
-                                 text,
-                                 app_icon_bitmap_,
+                                 notification_heading,
+                                 notification_text,
+                                 icon_bitmap,
                                  WebKit::WebTextDirectionDefault,
                                  display_source,
                                  replace_key,
                                  rich_notification_data,
                                  delegate.get());
     notification_manager->Add(ui_notification, profile);
   } else {
-
+    // In this case we have a Webkit Notification, not a Rich Notification.
     Notification ui_notification(GetOriginUrl(),
                                  GetAppIconUrl(),
-                                 heading,
-                                 text,
+                                 notification_heading,
+                                 notification_text,
                                  WebKit::WebTextDirectionDefault,
                                  display_source,
                                  replace_key,
                                  delegate.get());
 
     notification_manager->Add(ui_notification, profile);
   }
 
   DVLOG(1) << "Showing Synced Notification! " << heading << " " << text
            << " " << GetAppIconUrl() << " " << replace_key << " "
            << GetReadState();
 
   return;
 }
 
 // This should detect even small changes in case the server updated the
-// notification.
-// TODO(petewil): Should I also ignore the timestamp if other fields match?
+// notification.  We ignore the timestamp if other fields match.
 bool SyncedNotification::EqualsIgnoringReadState(
     const SyncedNotification& other) const {
   if (GetTitle() == other.GetTitle() &&
       GetHeading() == other.GetHeading() &&
       GetDescription() == other.GetDescription() &&
+      GetAnnotation() == other.GetAnnotation() &&
       GetAppId() == other.GetAppId() &&
       GetKey() == other.GetKey() &&
       GetOriginUrl() == other.GetOriginUrl() &&
       GetAppIconUrl() == other.GetAppIconUrl() &&
       GetImageUrl() == other.GetImageUrl() &&
       GetText() == other.GetText() &&
       // We intentionally skip read state
       GetCreationTime() == other.GetCreationTime() &&
       GetPriority() == other.GetPriority() &&
       GetDefaultDestinationTitle() == other.GetDefaultDestinationTitle() &&
       GetDefaultDestinationIconUrl() == other.GetDefaultDestinationIconUrl() &&
       GetNotificationCount() == other.GetNotificationCount() &&
-      GetButtonCount() == other.GetButtonCount()) {
+      GetButtonCount() == other.GetButtonCount() &&
+      GetProfilePictureCount() == other.GetProfilePictureCount()) {
 
     // If all the surface data matched, check, to see if contained data also
     // matches, titles and messages.
     size_t count = GetNotificationCount();
     for (size_t ii = 0; ii < count; ++ii) {
       if (GetContainedNotificationTitle(ii) !=
           other.GetContainedNotificationTitle(ii))
         return false;
       if (GetContainedNotificationMessage(ii) !=
           other.GetContainedNotificationMessage(ii))
         return false;
     }
 
     // Make sure buttons match.
     count = GetButtonCount();
     for (size_t jj = 0; jj < count; ++jj) {
       if (GetButtonTitle(jj) != other.GetButtonTitle(jj))
         return false;
       if (GetButtonIconUrl(jj) != other.GetButtonIconUrl(jj))
         return false;
     }
 
+    // Make sure profile icons match
+    count = GetButtonCount();
+    for (size_t kk = 0; kk < count; ++kk) {
+      if (GetProfilePictureUrl(kk) != other.GetProfilePictureUrl(kk))
+        return false;
+    }
+
     // If buttons and notifications matched, they are equivalent.
     return true;
   }
 
   return false;
 }
 
 // Set the read state on the notification, returns true for success.
 void SyncedNotification::SetReadState(const ReadState& read_state) {
 
@@ skipping 35 matching lines @@
 
 std::string SyncedNotification::GetDescription() const {
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       simple_collapsed_layout().has_description())
     return std::string();
 
   return specifics_.coalesced_notification().render_info().collapsed_info().
       simple_collapsed_layout().description();
 }
 
+std::string SyncedNotification::GetAnnotation() const {
+  if (!specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().has_annotation())
+    return std::string();
+
+  return specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().annotation();
+}
+
 std::string SyncedNotification::GetAppId() const {
   if (!specifics_.coalesced_notification().has_app_id())
     return std::string();
   return specifics_.coalesced_notification().app_id();
 }
 
 std::string SyncedNotification::GetKey() const {
   if (!specifics_.coalesced_notification().has_key())
     return std::string();
   return specifics_.coalesced_notification().key();
 }
 
 GURL SyncedNotification::GetOriginUrl() const {
   std::string origin_url(kExtensionScheme);
   origin_url += GetAppId();
   return GURL(origin_url);
 }
 
-// TODO(petewil): This only returns the first icon. Make all the icons
-// available.
 GURL SyncedNotification::GetAppIconUrl() const {
-  if (specifics_.coalesced_notification().render_info().expanded_info().
-      collapsed_info_size() == 0)
+  if (!specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().has_app_icon())
     return GURL();
 
-  if (!specifics_.coalesced_notification().render_info().expanded_info().
-      collapsed_info(0).simple_collapsed_layout().has_app_icon())
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().simple_collapsed_layout().app_icon().url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
+}
+
+// TODO(petewil): This ignores all but the first image.  If Rich Notifications
+// supports more images someday, then fetch all images.
+GURL SyncedNotification::GetImageUrl() const {
+  if (specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().media_size() == 0)
     return GURL();
 
-  return GURL(specifics_.coalesced_notification().render_info().
-              expanded_info().collapsed_info(0).simple_collapsed_layout().
-              app_icon().url());
-}
-
-// TODO(petewil): This currenly only handles the first image from the first
-// collapsed item, someday return all images.
-GURL SyncedNotification::GetImageUrl() const {
-  if (specifics_.coalesced_notification().render_info().expanded_info().
-      simple_expanded_layout().media_size() == 0)
+  if (!specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().media(0).image().has_url())
     return GURL();
 
-  if (!specifics_.coalesced_notification().render_info().expanded_info().
-      simple_expanded_layout().media(0).image().has_url())
-    return GURL();
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().simple_collapsed_layout().media(0).image().url();
 
-  return GURL(specifics_.coalesced_notification().render_info().
-              expanded_info().simple_expanded_layout().media(0).image().url());
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
 }
 
 std::string SyncedNotification::GetText() const {
   if (!specifics_.coalesced_notification().render_info().expanded_info().
       simple_expanded_layout().has_text())
     return std::string();
 
   return specifics_.coalesced_notification().render_info().expanded_info().
       simple_expanded_layout().text();
 }
@@ skipping 55 matching lines @@
 size_t SyncedNotification::GetNotificationCount() const {
   return specifics_.coalesced_notification().render_info().
       expanded_info().collapsed_info_size();
 }
 
 size_t SyncedNotification::GetButtonCount() const {
   return specifics_.coalesced_notification().render_info().collapsed_info().
       target_size();
 }
 
+size_t SyncedNotification::GetProfilePictureCount() const {
+  return specifics_.coalesced_notification().render_info().collapsed_info().
+      simple_collapsed_layout().profile_image_size();
+}
+
+GURL SyncedNotification::GetProfilePictureUrl(unsigned int which_url) const {
+  if (GetProfilePictureCount() <= which_url)
+    return GURL();
+
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+      collapsed_info().simple_collapsed_layout().profile_image(which_url).
+      image_url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
+}
+
+
 std::string SyncedNotification::GetDefaultDestinationTitle() const {
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       default_destination().icon().has_alt_text()) {
     return std::string();
   }
   return specifics_.coalesced_notification().render_info().collapsed_info().
       default_destination().icon().alt_text();
 }
 
 GURL SyncedNotification::GetDefaultDestinationIconUrl() const {
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       default_destination().icon().has_url()) {
     return GURL();
   }
-  return GURL(specifics_.coalesced_notification().render_info().
-              collapsed_info().default_destination().icon().url());
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().default_destination().icon().url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
 }
 
 GURL SyncedNotification::GetDefaultDestinationUrl() const {
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       default_destination().has_url()) {
     return GURL();
   }
-  return GURL(specifics_.coalesced_notification().render_info().
-              collapsed_info().default_destination().url());
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().default_destination().url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
 }
 
 std::string SyncedNotification::GetButtonTitle(
     unsigned int which_button) const {
   // Must ensure that we have a target before trying to access it.
   if (GetButtonCount() <= which_button)
     return std::string();
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       target(which_button).action().icon().has_alt_text()) {
     return std::string();
   }
   return specifics_.coalesced_notification().render_info().collapsed_info().
       target(which_button).action().icon().alt_text();
 }
 
 GURL SyncedNotification::GetButtonIconUrl(unsigned int which_button) const {
   // Must ensure that we have a target before trying to access it.
   if (GetButtonCount() <= which_button)
     return GURL();
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       target(which_button).action().icon().has_url()) {
     return GURL();
   }
-  return GURL(specifics_.coalesced_notification().render_info().
-              collapsed_info().target(which_button).action().icon().url());
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().target(which_button).action().icon().url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
 }
 
 GURL SyncedNotification::GetButtonUrl(unsigned int which_button) const {
   // Must ensure that we have a target before trying to access it.
   if (GetButtonCount() <= which_button)
     return GURL();
   if (!specifics_.coalesced_notification().render_info().collapsed_info().
       target(which_button).action().has_url()) {
     return GURL();
   }
-  return GURL(specifics_.coalesced_notification().render_info().
-              collapsed_info().target(which_button).action().url());
+  std::string url_spec = specifics_.coalesced_notification().render_info().
+              collapsed_info().target(which_button).action().url();
+
+  url_spec = AddDefaultSchemaIfNeeded(url_spec);
+
+  return GURL(url_spec);
 }
 
 std::string SyncedNotification::GetContainedNotificationTitle(
     int index) const {
   if (specifics_.coalesced_notification().render_info().expanded_info().
       collapsed_info_size() < index + 1)
     return std::string();
 
   return specifics_.coalesced_notification().render_info().expanded_info().
       collapsed_info(index).simple_collapsed_layout().heading();
 }
 
 std::string SyncedNotification::GetContainedNotificationMessage(
     int index) const {
   if (specifics_.coalesced_notification().render_info().expanded_info().
       collapsed_info_size() < index + 1)
     return std::string();
 
   return specifics_.coalesced_notification().render_info().expanded_info().
       collapsed_info(index).simple_collapsed_layout().description();
 }
 
 }  // namespace notifier