[Notification scheduler]: Polish Notification Data.

- Change a vector of icons to small_icon and large_icon explicitly.
- Wrap Skbitmap to an Icon struct for future extension.

Bug: 963290
Change-Id: I6bd9459dc05e169e281659b67747a55c464e45bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758708Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688236}

chrome/browser/notifications/proto/notification_data.proto

@@ -22,7 +22,7 @@ enum ActionButtonType {
 }
 
 // Stores data used to display a notification in the UI.
-// Next tag: 6
+// Next tag: 7
 message NotificationData {
   // Represents the button on the notification.
   // Next tag: 4
@@ -38,13 +38,17 @@ message NotificationData {
   // Body text of the notification.
   optional string message = 2;
 
-  // The unique identifiers of the icons on the notification. Each time we
-  // should only load icons asynchronously into memory.
-  repeated string icon_uuid = 3;
-
   // Custom data associated with a notification.
-  repeated CustomData custom_data = 4;
+  repeated CustomData custom_data = 3;
 
   // A list of buttons on the notification.
-  repeated Button buttons = 5;
+  repeated Button buttons = 4;
+
+  // The unique identifier of the small icon on notification, which must be
+  // loaded asynchronously into memory.
+  optional string small_icon_uuid = 5;
+
+  // The unique identifier of the large icon on notification, which must be
+  // loaded asynchronously into memory.
+  optional string large_icon_uuid = 6;
 }

chrome/browser/notifications/scheduler/internal/notification_entry.cc

@@ -21,7 +21,8 @@ bool NotificationEntry::operator==(const NotificationEntry& other) const {
   return type == other.type && guid == other.guid &&
          create_time == other.create_time &&
          notification_data == other.notification_data &&
-         icons_uuid == other.icons_uuid &&
+         small_icon_uuid == other.small_icon_uuid &&
+         large_icon_uuid == other.large_icon_uuid &&
          schedule_params == other.schedule_params;
 }
 

chrome/browser/notifications/scheduler/internal/notification_entry.h

@@ -37,9 +37,13 @@ struct NotificationEntry {
   // shown.
   NotificationData notification_data;
 
-  // The unique identifier of the icons, which must be loaded asynchronously
-  // into memory.
-  std::vector<std::string> icons_uuid;
+  // The unique identifier of the small icon on notification, which must be
+  // loaded asynchronously into memory.
+  std::string small_icon_uuid;
+
+  // The unique identifier of the large icon on notification, which must be
+  // loaded asynchronously into memory.
+  std::string large_icon_uuid;
 
   // Scheduling details.
   ScheduleParams schedule_params;

chrome/browser/notifications/scheduler/internal/proto_conversion.cc

@@ -415,11 +415,8 @@ void NotificationEntryToProto(NotificationEntry* entry,
   proto->set_create_time(TimeToMilliseconds(entry->create_time));
   auto* proto_notification_data = proto->mutable_notification_data();
   NotificationDataToProto(&entry->notification_data, proto_notification_data);
-  for (const auto& icon_id : entry->icons_uuid) {
-    auto* proto_icon_id = proto_notification_data->add_icon_uuid();
-    *proto_icon_id = icon_id;
-  }
-
+  proto_notification_data->set_small_icon_uuid(entry->small_icon_uuid);
+  proto_notification_data->set_large_icon_uuid(entry->large_icon_uuid);
   auto* proto_schedule_params = proto->mutable_schedule_params();
   ScheduleParamsToProto(&entry->schedule_params, proto_schedule_params);
 }
@@ -431,10 +428,8 @@ void NotificationEntryFromProto(proto::NotificationEntry* proto,
   entry->create_time = MillisecondsToTime(proto->create_time());
   NotificationDataFromProto(proto->mutable_notification_data(),
                             &entry->notification_data);
-  for (int i = 0; i < proto->notification_data().icon_uuid_size(); ++i) {
-    entry->icons_uuid.emplace_back(proto->notification_data().icon_uuid(i));
-  }
-
+  entry->small_icon_uuid = proto->notification_data().small_icon_uuid();
+  entry->large_icon_uuid = proto->notification_data().large_icon_uuid();
   ScheduleParamsFromProto(proto->mutable_schedule_params(),
                           &entry->schedule_params);
 }

chrome/browser/notifications/scheduler/internal/proto_conversion_unittest.cc

@@ -191,7 +191,8 @@ TEST(ProtoConversionTest, NotificationEntryConversion) {
   // Test notification data.
   entry.notification_data.title = base::UTF8ToUTF16("title");
   entry.notification_data.message = base::UTF8ToUTF16("message");
-  entry.icons_uuid = {"icon_uuid_0", "icon_uuid_1"};
+  entry.small_icon_uuid = "small_icon_uuid";
+  entry.large_icon_uuid = "large_icon_uuid";
   entry.notification_data.custom_data = {{"url", "https://www.example.com"}};
   TestNotificationEntryConversion(&entry);
 

chrome/browser/notifications/scheduler/internal/scheduled_notification_manager.cc

@@ -130,7 +130,7 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
     notification_store_->Delete(
         guid,
         base::BindOnce(&ScheduledNotificationManagerImpl::OnNotificationDeleted,
-                       weak_ptr_factory_.GetWeakPtr(), entry->icons_uuid));
+                       weak_ptr_factory_.GetWeakPtr()));
 
     if (delegate_)
       delegate_->DisplayNotification(std::move(entry));
@@ -176,7 +176,7 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
           entry.guid,
           base::BindOnce(
               &ScheduledNotificationManagerImpl::OnNotificationDeleted,
-              weak_ptr_factory_.GetWeakPtr(), entry.icons_uuid));
+              weak_ptr_factory_.GetWeakPtr()));
     }
     notifications_.erase(type);
   }
@@ -213,7 +213,7 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
             entry->guid,
             base::BindOnce(
                 &ScheduledNotificationManagerImpl::OnNotificationDeleted,
-                weak_ptr_factory_.GetWeakPtr(), entry->icons_uuid));
+                weak_ptr_factory_.GetWeakPtr()));
       } else if (clients_.count(entry->type)) {
         notifications_[entry->type].emplace(entry->guid, std::move(*it));
       }
@@ -242,16 +242,6 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
         notifications_.erase(type);
       return;
     }
-
-    // TODO(hesen): Address only first icon for now. Handle all icons situation
-    // in following CLs.
-    if (entry->notification_data.icons.empty())
-      return;
-
-    encode_icons_callback_.Run(
-        std::move(entry->notification_data.icons.front()),
-        base::BindOnce(&ScheduledNotificationManagerImpl::OnIconEncoded,
-                       weak_ptr_factory_.GetWeakPtr(), type, std::move(guid)));
   }
 
   void OnIconEncoded(SchedulerClientType type,
@@ -276,21 +266,9 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
     // TODO(hesen): Error handling.
     if (!entry || !success)
       return;
-    entry->icons_uuid.emplace_back(std::move(icon_uuid));
   }
 
-  void OnNotificationDeleted(const std::vector<std::string>& icon_uuids,
-                             bool success) {
-    // TODO(hesen): Error handling.
-    if (!success)
-      return;
-    for (const auto& icon_uuid : icon_uuids) {
-      icon_store_->Delete(
-          std::move(icon_uuid),
-          base::BindOnce(&ScheduledNotificationManagerImpl::OnIconDeleted,
-                         weak_ptr_factory_.GetWeakPtr()));
-    }
-  }
+  void OnNotificationDeleted(bool success) { NOTIMPLEMENTED(); }
 
   void OnIconDeleted(bool success) { NOTIMPLEMENTED(); }
 

chrome/browser/notifications/scheduler/public/notification_data.cc

@@ -15,13 +15,15 @@ bool NotificationData::Button::operator==(const Button& other) const {
 
 NotificationData::Button::~Button() = default;
 
+NotificationData::Icon::Icon() = default;
+NotificationData::Icon::~Icon() = default;
+
 NotificationData::NotificationData() = default;
 
 NotificationData::NotificationData(const NotificationData& other) = default;
 
 bool NotificationData::operator==(const NotificationData& other) const {
   return title == other.title && message == other.message &&
-         icons.size() == other.icons.size() &&
          custom_data == other.custom_data && buttons == other.buttons;
 }
 

chrome/browser/notifications/scheduler/public/notification_data.h

@@ -38,6 +38,14 @@ struct NotificationData {
     std::string id;
   };
 
+  struct Icon {
+    Icon();
+    ~Icon();
+
+    // The icon bitmap.
+    SkBitmap bitmap;
+  };
+
   using CustomData = std::map<std::string, std::string>;
   NotificationData();
   NotificationData(const NotificationData& other);
@@ -50,9 +58,11 @@ struct NotificationData {
   // The body text of the notification.
   base::string16 message;
 
-  // A list of icons. On Android, the first element will be used as small icon,
-  // the second as large icon(optional).
-  std::vector<SkBitmap> icons;
+  // The small icon of the notification.
+  Icon small_icon;
+
+  // The large icon of the notification.
+  Icon large_icon;
 
   // Custom key value pair data associated with each notification. Will be sent
   // back after user interaction.

chrome/browser/notifications/scheduler/test/test_utils.cc

@@ -105,10 +105,8 @@ std::string DebugString(const NotificationEntry* entry) {
            << " : " << static_cast<int>(mapping.second);
   }
 
-  stream << " \n icons_id:";
-  for (const auto& icon_id : entry->icons_uuid)
-    stream << icon_id << "  ";
-
+  stream << " \n small_icons_id:" << entry->small_icon_uuid << "  ";
+  stream << " \n large_icons_id:" << entry->large_icon_uuid << "  ";
   return stream.str();
 }