lacros: Rich notification support

Add support for image, list, and progress notifications. This
involves serializing the RichNotificationData structure.

Move some existing crosapi::Bitmap serialization code into a
shared utility file. Add deserialization to SkBitmap. For now
the image format is hard-coded. crbug.com/1116652 tracks more
flexible image serialization.

Add unit tests for both ash side and lacros side.

Bug: 1113889
Test: unit_tests, manual with Notifications Galore chrome app

Change-Id: I27b1c74f828751f35d845e625052386530de9722
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357749
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Darin Fisher <darin@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799386}

chrome/browser/chromeos/crosapi/message_center_ash.cc

@@ -9,8 +9,10 @@
 #include "base/bind.h"
 #include "base/check.h"
 #include "base/memory/scoped_refptr.h"
-#include "base/numerics/safe_conversions.h"
+#include "base/numerics/ranges.h"
 #include "base/optional.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
+#include "chromeos/crosapi/cpp/bitmap_util.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/notification.mojom.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -37,6 +39,62 @@ mc::NotificationType FromMojo(mojom::NotificationType type) {
   }
 }
 
+mc::FullscreenVisibility FromMojo(mojom::FullscreenVisibility visibility) {
+  switch (visibility) {
+    case mojom::FullscreenVisibility::kNone:
+      return mc::FullscreenVisibility::NONE;
+    case mojom::FullscreenVisibility::kOverUser:
+      return mc::FullscreenVisibility::OVER_USER;
+  }
+}
+
+gfx::Image ImageFromBitmap(const crosapi::Bitmap& bitmap) {
+  SkBitmap sk_bitmap = crosapi::SkBitmapFromBitmap(bitmap);
+  // TODO(https://crbug.com/1113889): High DPI support.
+  return gfx::Image::CreateFrom1xBitmap(sk_bitmap);
+}
+
+std::unique_ptr<mc::Notification> FromMojo(
+    mojom::NotificationPtr notification) {
+  mc::RichNotificationData rich_data;
+  rich_data.priority = base::ClampToRange(notification->priority, -2, 2);
+  rich_data.never_timeout = notification->require_interaction;
+  rich_data.timestamp = notification->timestamp;
+  if (notification->image)
+    rich_data.image = ImageFromBitmap(notification->image.value());
+  if (notification->badge)
+    rich_data.small_image = ImageFromBitmap(notification->badge.value());
+  for (const auto& mojo_item : notification->items) {
+    mc::NotificationItem item;
+    item.title = mojo_item->title;
+    item.message = mojo_item->message;
+    rich_data.items.push_back(item);
+  }
+  rich_data.progress = base::ClampToRange(notification->progress, -1, 100);
+  rich_data.progress_status = notification->progress_status;
+  for (const auto& mojo_button : notification->buttons) {
+    mc::ButtonInfo button;
+    button.title = mojo_button->title;
+    rich_data.buttons.push_back(button);
+  }
+  rich_data.pinned = notification->pinned;
+  rich_data.renotify = notification->renotify;
+  rich_data.silent = notification->silent;
+  rich_data.accessible_name = notification->accessible_name;
+  rich_data.fullscreen_visibility =
+      FromMojo(notification->fullscreen_visibility);
+
+  gfx::Image icon;
+  if (notification->icon)
+    icon = ImageFromBitmap(notification->icon.value());
+  GURL origin_url = notification->origin_url.value_or(GURL());
+  // TODO(crbug.com/1113889): NotifierId support.
+  return std::make_unique<mc::Notification>(
+      FromMojo(notification->type), notification->id, notification->title,
+      notification->message, icon, notification->display_source, origin_url,
+      mc::NotifierId(), rich_data, /*delegate=*/nullptr);
+}
+
 // Forwards NotificationDelegate methods to a remote delegate over mojo. If the
 // remote delegate disconnects (e.g. lacros-chrome crashes) the corresponding
 // notification will be removed.
@@ -116,14 +174,10 @@ void MessageCenterAsh::DisplayNotification(
       notification->id, std::move(delegate));
   forwarding_delegate->Init();
 
-  // TODO(crbug.com/1113889): Icon support.
-  // TODO(crbug.com/1113889): NotifierId support.
-  // TODO(crbug.com/1113889): RichNotificationData support.
-  mc::MessageCenter::Get()->AddNotification(std::make_unique<mc::Notification>(
-      FromMojo(notification->type), notification->id, notification->title,
-      notification->message, gfx::Image(), notification->display_source,
-      notification->origin_url.value_or(GURL()), mc::NotifierId(),
-      mc::RichNotificationData(), std::move(forwarding_delegate)));
+  std::unique_ptr<mc::Notification> mc_notification =
+      FromMojo(std::move(notification));
+  mc_notification->set_delegate(forwarding_delegate);
+  mc::MessageCenter::Get()->AddNotification(std::move(mc_notification));
 }
 
 void MessageCenterAsh::CloseNotification(const std::string& id) {

chrome/browser/chromeos/crosapi/message_center_ash_unittest.cc

@@ -7,18 +7,40 @@
 #include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/task_environment.h"
+#include "base/time/time.h"
+#include "cc/test/pixel_comparator.h"
+#include "chromeos/crosapi/cpp/bitmap_util.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/notification.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/public/cpp/notification_delegate.h"
 #include "url/gurl.h"
 
+using base::ASCIIToUTF16;
+
 namespace crosapi {
 namespace {
 
+// Returns a bitmap filled with red.
+SkBitmap CreateRedBitmap(int width, int height) {
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(width, height);
+  bitmap.eraseColor(SK_ColorRED);
+  return bitmap;
+}
+
+bool BitmapEquals(const SkBitmap& a, const SkBitmap& b) {
+  if (a.width() != b.width() || a.height() != b.height())
+    return false;
+  cc::ExactPixelComparator comparator(/*discard_alpha=*/false);
+  return comparator.Compare(a, b);
+}
+
 class MojoDelegate : public mojom::NotificationDelegate {
  public:
   MojoDelegate() = default;
@@ -56,34 +78,59 @@ class MessageCenterAshTest : public testing::Test {
   ~MessageCenterAshTest() override = default;
 
   // testing::Test:
-  void SetUp() override { message_center::MessageCenter::Initialize(); }
+  void SetUp() override {
+    message_center::MessageCenter::Initialize();
+    message_center_ash_ = std::make_unique<MessageCenterAsh>(
+        message_center_remote_.BindNewPipeAndPassReceiver());
+  }
 
-  void TearDown() override { message_center::MessageCenter::Shutdown(); }
+  void TearDown() override {
+    message_center_ash_.reset();
+    message_center::MessageCenter::Shutdown();
+  }
 
- private:
+ protected:
   base::test::TaskEnvironment task_environment_;
+  mojo::Remote<mojom::MessageCenter> message_center_remote_;
+  std::unique_ptr<MessageCenterAsh> message_center_ash_;
 };
 
-TEST_F(MessageCenterAshTest, Basics) {
-  // Create object under test.
-  mojo::Remote<mojom::MessageCenter> remote;
-  MessageCenterAsh message_center_ash(remote.BindNewPipeAndPassReceiver());
-
-  // Build mojo notification for display.
+TEST_F(MessageCenterAshTest, SerializationSimple) {
+  // Create a notification.
   auto mojo_notification = mojom::Notification::New();
   mojo_notification->type = mojom::NotificationType::kSimple;
   mojo_notification->id = "test_id";
-  mojo_notification->title = base::ASCIIToUTF16("title");
-  mojo_notification->message = base::ASCIIToUTF16("message");
-  mojo_notification->display_source = base::ASCIIToUTF16("source");
+  mojo_notification->title = ASCIIToUTF16("title");
+  mojo_notification->message = ASCIIToUTF16("message");
+  mojo_notification->display_source = ASCIIToUTF16("source");
   mojo_notification->origin_url = GURL("http://example.com/");
+  mojo_notification->priority = 2;
+  mojo_notification->require_interaction = true;
+  base::Time now = base::Time::Now();
+  mojo_notification->timestamp = now;
+  mojo_notification->renotify = true;
+  mojo_notification->accessible_name = ASCIIToUTF16("accessible_name");
+  mojo_notification->fullscreen_visibility =
+      mojom::FullscreenVisibility::kOverUser;
+
+  SkBitmap test_badge = CreateRedBitmap(1, 2);
+  mojo_notification->badge = BitmapFromSkBitmap(test_badge);
+  SkBitmap test_icon = CreateRedBitmap(3, 4);
+  mojo_notification->icon = BitmapFromSkBitmap(test_icon);
+
+  auto button1 = mojom::ButtonInfo::New();
+  button1->title = ASCIIToUTF16("button1");
+  mojo_notification->buttons.push_back(std::move(button1));
+  auto button2 = mojom::ButtonInfo::New();
+  button2->title = ASCIIToUTF16("button2");
+  mojo_notification->buttons.push_back(std::move(button2));
 
   // Display the notification.
   MojoDelegate mojo_delegate;
-  remote->DisplayNotification(
+  message_center_remote_->DisplayNotification(
       std::move(mojo_notification),
       mojo_delegate.receiver_.BindNewPipeAndPassRemote());
-  remote.FlushForTesting();
+  message_center_remote_.FlushForTesting();
 
   // Notification exists and has correct fields.
   auto* message_center = message_center::MessageCenter::Get();
@@ -91,10 +138,148 @@ TEST_F(MessageCenterAshTest, Basics) {
       message_center->FindVisibleNotificationById("test_id");
   ASSERT_TRUE(ui_notification);
   EXPECT_EQ("test_id", ui_notification->id());
-  EXPECT_EQ(base::ASCIIToUTF16("title"), ui_notification->title());
-  EXPECT_EQ(base::ASCIIToUTF16("message"), ui_notification->message());
-  EXPECT_EQ(base::ASCIIToUTF16("source"), ui_notification->display_source());
+  EXPECT_EQ(ASCIIToUTF16("title"), ui_notification->title());
+  EXPECT_EQ(ASCIIToUTF16("message"), ui_notification->message());
+  EXPECT_EQ(ASCIIToUTF16("source"), ui_notification->display_source());
   EXPECT_EQ("http://example.com/", ui_notification->origin_url().spec());
+  EXPECT_EQ(2, ui_notification->priority());
+  EXPECT_TRUE(ui_notification->never_timeout());
+  EXPECT_EQ(now, ui_notification->timestamp());
+  EXPECT_TRUE(ui_notification->renotify());
+  EXPECT_EQ(ASCIIToUTF16("accessible_name"),
+            ui_notification->accessible_name());
+  EXPECT_EQ(message_center::FullscreenVisibility::OVER_USER,
+            ui_notification->fullscreen_visibility());
+
+  EXPECT_TRUE(
+      BitmapEquals(test_badge, ui_notification->small_image().AsBitmap()));
+  EXPECT_TRUE(BitmapEquals(test_icon, ui_notification->icon().AsBitmap()));
+
+  ASSERT_EQ(2u, ui_notification->buttons().size());
+  EXPECT_EQ(ASCIIToUTF16("button1"), ui_notification->buttons()[0].title);
+  EXPECT_EQ(ASCIIToUTF16("button2"), ui_notification->buttons()[1].title);
+}
+
+TEST_F(MessageCenterAshTest, SerializationImage) {
+  // Create a notification with an image.
+  auto mojo_notification = mojom::Notification::New();
+  mojo_notification->type = mojom::NotificationType::kImage;
+  mojo_notification->id = "test_id";
+
+  SkBitmap test_image = CreateRedBitmap(5, 6);
+  mojo_notification->image = BitmapFromSkBitmap(test_image);
+
+  // Display the notification.
+  MojoDelegate mojo_delegate;
+  message_center_remote_->DisplayNotification(
+      std::move(mojo_notification),
+      mojo_delegate.receiver_.BindNewPipeAndPassRemote());
+  message_center_remote_.FlushForTesting();
+
+  // Notification exists and has correct fields.
+  auto* message_center = message_center::MessageCenter::Get();
+  message_center::Notification* ui_notification =
+      message_center->FindVisibleNotificationById("test_id");
+  ASSERT_TRUE(ui_notification);
+  EXPECT_TRUE(BitmapEquals(test_image, ui_notification->image().AsBitmap()));
+}
+
+TEST_F(MessageCenterAshTest, SerializationList) {
+  // Create a notification with some list items.
+  auto mojo_notification = mojom::Notification::New();
+  mojo_notification->type = mojom::NotificationType::kList;
+  mojo_notification->id = "test_id";
+
+  auto item1 = mojom::NotificationItem::New();
+  item1->title = ASCIIToUTF16("title1");
+  item1->message = ASCIIToUTF16("message1");
+  mojo_notification->items.push_back(std::move(item1));
+  auto item2 = mojom::NotificationItem::New();
+  item2->title = ASCIIToUTF16("title2");
+  item2->message = ASCIIToUTF16("message2");
+  mojo_notification->items.push_back(std::move(item2));
+
+  // Display the notification.
+  MojoDelegate mojo_delegate;
+  message_center_remote_->DisplayNotification(
+      std::move(mojo_notification),
+      mojo_delegate.receiver_.BindNewPipeAndPassRemote());
+  message_center_remote_.FlushForTesting();
+
+  // Notification exists and has correct fields.
+  auto* message_center = message_center::MessageCenter::Get();
+  message_center::Notification* ui_notification =
+      message_center->FindVisibleNotificationById("test_id");
+  ASSERT_TRUE(ui_notification);
+  ASSERT_EQ(2u, ui_notification->items().size());
+  EXPECT_EQ(ASCIIToUTF16("title1"), ui_notification->items()[0].title);
+  EXPECT_EQ(ASCIIToUTF16("message1"), ui_notification->items()[0].message);
+  EXPECT_EQ(ASCIIToUTF16("title2"), ui_notification->items()[1].title);
+  EXPECT_EQ(ASCIIToUTF16("message2"), ui_notification->items()[1].message);
+}
+
+TEST_F(MessageCenterAshTest, SerializationProgress) {
+  // Create a notification with partial progress.
+  auto mojo_notification = mojom::Notification::New();
+  mojo_notification->type = mojom::NotificationType::kProgress;
+  mojo_notification->id = "test_id";
+  mojo_notification->progress = 55;
+  mojo_notification->progress_status = ASCIIToUTF16("status");
+
+  // Display the notification.
+  MojoDelegate mojo_delegate1;
+  message_center_remote_->DisplayNotification(
+      std::move(mojo_notification),
+      mojo_delegate1.receiver_.BindNewPipeAndPassRemote());
+  message_center_remote_.FlushForTesting();
+
+  // Notification exists and has correct fields.
+  auto* message_center = message_center::MessageCenter::Get();
+  message_center::Notification* ui_notification =
+      message_center->FindVisibleNotificationById("test_id");
+  ASSERT_TRUE(ui_notification);
+  EXPECT_EQ(55, ui_notification->progress());
+  EXPECT_EQ(ASCIIToUTF16("status"), ui_notification->progress_status());
+
+  // Update progress past 100% by creating a new notification with the same ID.
+  mojo_notification = mojom::Notification::New();
+  mojo_notification->type = mojom::NotificationType::kProgress;
+  mojo_notification->id = "test_id";
+  mojo_notification->progress = 101;
+  mojo_notification->progress_status = ASCIIToUTF16("complete");
+
+  MojoDelegate mojo_delegate2;
+  message_center_remote_->DisplayNotification(
+      std::move(mojo_notification),
+      mojo_delegate2.receiver_.BindNewPipeAndPassRemote());
+  message_center_remote_.FlushForTesting();
+
+  ui_notification = message_center->FindVisibleNotificationById("test_id");
+  ASSERT_TRUE(ui_notification);
+  // Progress was clamped to 100.
+  EXPECT_EQ(100, ui_notification->progress());
+  // Status was updated.
+  EXPECT_EQ(ASCIIToUTF16("complete"), ui_notification->progress_status());
+}
+
+TEST_F(MessageCenterAshTest, UserActions) {
+  // Build mojo notification for display.
+  auto mojo_notification = mojom::Notification::New();
+  mojo_notification->type = mojom::NotificationType::kSimple;
+  mojo_notification->id = "test_id";
+
+  // Display the notification.
+  MojoDelegate mojo_delegate;
+  message_center_remote_->DisplayNotification(
+      std::move(mojo_notification),
+      mojo_delegate.receiver_.BindNewPipeAndPassRemote());
+  message_center_remote_.FlushForTesting();
+
+  // Notification exists.
+  auto* message_center = message_center::MessageCenter::Get();
+  message_center::Notification* ui_notification =
+      message_center->FindVisibleNotificationById("test_id");
+  ASSERT_TRUE(ui_notification);
 
   // Simulate the user clicking on the notification body.
   ui_notification->delegate()->Click(/*button_index=*/base::nullopt,
@@ -120,8 +305,8 @@ TEST_F(MessageCenterAshTest, Basics) {
   EXPECT_EQ(1, mojo_delegate.disabled_count_);
 
   // Close the notification.
-  remote->CloseNotification("test_id");
-  remote.FlushForTesting();
+  message_center_remote_->CloseNotification("test_id");
+  message_center_remote_.FlushForTesting();
   EXPECT_FALSE(message_center->FindVisibleNotificationById("test_id"));
   EXPECT_EQ(1, mojo_delegate.closed_count_);
 }

chrome/browser/chromeos/crosapi/screen_manager_ash.cc

@@ -10,10 +10,9 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "base/files/file_path.h"
-#include "base/numerics/checked_math.h"
-#include "base/numerics/safe_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/crosapi/cpp/bitmap.h"
+#include "chromeos/crosapi/cpp/bitmap_util.h"
 #include "ui/snapshot/snapshot.h"
 
 namespace crosapi {
@@ -119,22 +118,8 @@ void ScreenManagerAsh::OnWindowDestroying(aura::Window* window) {
 
 void ScreenManagerAsh::DidTakeSnapshot(SnapshotCallback callback,
                                        gfx::Image image) {
-  SkBitmap bitmap = image.AsBitmap();
-
-  // This code currently relies on the assumption that the bitmap is unpadded,
-  // and uses 4 bytes per pixel.
-  int size;
-  size = base::CheckMul(bitmap.width(), bitmap.height()).ValueOrDie();
-  size = base::CheckMul(size, 4).ValueOrDie();
-  CHECK_EQ(bitmap.computeByteSize(), base::checked_cast<size_t>(size));
-
-  uint8_t* base = static_cast<uint8_t*>(bitmap.getPixels());
-  std::vector<uint8_t> bytes(base, base + bitmap.computeByteSize());
-
-  Bitmap snapshot;
-  snapshot.width = bitmap.width();
-  snapshot.height = bitmap.height();
-  snapshot.pixels.swap(bytes);
+  SkBitmap sk_bitmap = image.AsBitmap();
+  Bitmap snapshot = BitmapFromSkBitmap(sk_bitmap);
   std::move(callback).Run(std::move(snapshot));
 }
 

chrome/browser/chromeos/crosapi/screen_manager_ash.h

@@ -44,7 +44,7 @@ class ScreenManagerAsh : public mojom::ScreenManager, aura::WindowObserver {
   void OnWindowDestroying(aura::Window* window) final;
 
  private:
-  using SnapshotCallback = base::OnceCallback<void(const Bitmap&)>;
+  using SnapshotCallback = base::OnceCallback<void(Bitmap)>;
   void DidTakeSnapshot(SnapshotCallback callback, gfx::Image image);
 
   // This class generates unique, non-reused IDs for windows on demand. The IDs

chrome/browser/notifications/notification_platform_bridge_lacros.cc

@@ -8,9 +8,12 @@
 
 #include "base/check.h"
 #include "base/notreached.h"
+#include "base/numerics/ranges.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/optional.h"
 #include "chrome/browser/notifications/notification_platform_bridge_delegate.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
+#include "chromeos/crosapi/cpp/bitmap_util.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/notification.mojom.h"
 #include "ui/message_center/public/cpp/notification.h"
@@ -37,6 +40,62 @@ crosapi::mojom::NotificationType ToMojo(message_center::NotificationType type) {
   }
 }
 
+crosapi::mojom::FullscreenVisibility ToMojo(
+    message_center::FullscreenVisibility visibility) {
+  switch (visibility) {
+    case message_center::FullscreenVisibility::NONE:
+      return crosapi::mojom::FullscreenVisibility::kNone;
+    case message_center::FullscreenVisibility::OVER_USER:
+      return crosapi::mojom::FullscreenVisibility::kOverUser;
+  }
+}
+
+crosapi::mojom::NotificationPtr ToMojo(
+    const message_center::Notification& notification) {
+  auto mojo_note = crosapi::mojom::Notification::New();
+  mojo_note->type = ToMojo(notification.type());
+  mojo_note->id = notification.id();
+  mojo_note->title = notification.title();
+  mojo_note->message = notification.message();
+  mojo_note->display_source = notification.display_source();
+  mojo_note->origin_url = notification.origin_url();
+  if (!notification.icon().IsEmpty()) {
+    SkBitmap icon = notification.icon().AsBitmap();
+    mojo_note->icon = crosapi::BitmapFromSkBitmap(icon);
+  }
+  mojo_note->priority = base::ClampToRange(notification.priority(), -2, 2);
+  mojo_note->require_interaction = notification.never_timeout();
+  mojo_note->timestamp = notification.timestamp();
+  if (!notification.image().IsEmpty()) {
+    SkBitmap image = notification.image().AsBitmap();
+    mojo_note->image = crosapi::BitmapFromSkBitmap(image);
+  }
+  if (!notification.small_image().IsEmpty()) {
+    SkBitmap badge = notification.small_image().AsBitmap();
+    mojo_note->badge = crosapi::BitmapFromSkBitmap(badge);
+  }
+  for (const auto& item : notification.items()) {
+    auto mojo_item = crosapi::mojom::NotificationItem::New();
+    mojo_item->title = item.title;
+    mojo_item->message = item.message;
+    mojo_note->items.push_back(std::move(mojo_item));
+  }
+  mojo_note->progress = base::ClampToRange(notification.progress(), -1, 100);
+  mojo_note->progress_status = notification.progress_status();
+  for (const auto& button : notification.buttons()) {
+    auto mojo_button = crosapi::mojom::ButtonInfo::New();
+    mojo_button->title = button.title;
+    mojo_note->buttons.push_back(std::move(mojo_button));
+  }
+  mojo_note->pinned = notification.pinned();
+  mojo_note->renotify = notification.renotify();
+  mojo_note->silent = notification.silent();
+  mojo_note->accessible_name = notification.accessible_name();
+  mojo_note->fullscreen_visibility =
+      ToMojo(notification.fullscreen_visibility());
+  return mojo_note;
+}
+
 }  // namespace
 
 // Keeps track of notifications being displayed in the remote message center.
@@ -118,15 +177,6 @@ void NotificationPlatformBridgeLacros::Display(
   // NotificationPlatformBridgeChromeOs, which includes a profile ID as part of
   // the notification ID. Lacros does not support Chrome OS multi-signin, so we
   // don't need to handle inactive user notification blockers in ash.
-  auto note = crosapi::mojom::Notification::New();
-  note->type = ToMojo(notification.type());
-  note->id = notification.id();
-  note->title = notification.title();
-  note->message = notification.message();
-  note->display_source = notification.display_source();
-  note->origin_url = notification.origin_url();
-  // TODO(https://crbug.com/1113889): Icon.
-  // TODO(https://crbug.com/1113889): RichNotificationData fields.
 
   // Clean up any old notification with the same ID before creating the new one.
   remote_notifications_.erase(notification.id());
@@ -134,7 +184,7 @@ void NotificationPlatformBridgeLacros::Display(
   auto pending_notification = std::make_unique<RemoteNotificationDelegate>(
       notification.id(), bridge_delegate_, weak_factory_.GetWeakPtr());
   (*message_center_remote_)
-      ->DisplayNotification(std::move(note),
+      ->DisplayNotification(ToMojo(notification),
                             pending_notification->BindNotificationDelegate());
   remote_notifications_[notification.id()] = std::move(pending_notification);
 }

chrome/browser/notifications/notification_platform_bridge_lacros_unittest.cc

@@ -6,16 +6,39 @@
 
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "cc/test/pixel_comparator.h"
 #include "chrome/browser/notifications/notification_platform_bridge_delegate.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
+#include "chromeos/crosapi/cpp/bitmap_util.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/notification.mojom.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/message_center/public/cpp/notification.h"
+#include "ui/message_center/public/cpp/notification_types.h"
 #include "url/gurl.h"
 
+using base::ASCIIToUTF16;
+
 namespace {
 
+// Returns an image filled with red.
+gfx::Image CreateRedImage(int width, int height) {
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(width, height);
+  bitmap.eraseColor(SK_ColorRED);
+  return gfx::Image::CreateFrom1xBitmap(bitmap);
+}
+
+bool BitmapEquals(const SkBitmap& a, const SkBitmap& b) {
+  if (a.width() != b.width() || a.height() != b.height())
+    return false;
+  cc::ExactPixelComparator comparator(/*discard_alpha=*/false);
+  return comparator.Compare(a, b);
+}
+
 // Tracks user actions that would be passed into chrome's cross-platform
 // notification subsystem.
 class TestPlatformBridgeDelegate : public NotificationPlatformBridgeDelegate {
@@ -93,70 +116,208 @@ class TestMessageCenter : public crosapi::mojom::MessageCenter {
   mojo::Receiver<crosapi::mojom::MessageCenter> receiver_;
 };
 
-TEST(NotificationPlatformBridgeLacrosTest, Basics) {
-  content::BrowserTaskEnvironment task_environment;
+class NotificationPlatformBridgeLacrosTest : public testing::Test {
+ public:
+  NotificationPlatformBridgeLacrosTest() = default;
+  ~NotificationPlatformBridgeLacrosTest() override = default;
+
+  // Public because this is test code.
+  content::BrowserTaskEnvironment task_environment_;
+  mojo::Remote<crosapi::mojom::MessageCenter> message_center_remote_;
+  TestMessageCenter test_message_center_{
+      message_center_remote_.BindNewPipeAndPassReceiver()};
+  TestPlatformBridgeDelegate bridge_delegate_;
+  NotificationPlatformBridgeLacros bridge_{&bridge_delegate_,
+                                           &message_center_remote_};
+};
+
+TEST_F(NotificationPlatformBridgeLacrosTest, SerializationSimple) {
+  // Create a message_center notification.
+  message_center::RichNotificationData rich_data;
+  rich_data.priority = 2;
+  rich_data.never_timeout = true;
+  base::Time now = base::Time::Now();
+  rich_data.timestamp = now;
+  rich_data.renotify = true;
+  rich_data.accessible_name = ASCIIToUTF16("accessible_name");
+  rich_data.fullscreen_visibility =
+      message_center::FullscreenVisibility::OVER_USER;
 
-  // Create the object under test.
-  mojo::Remote<crosapi::mojom::MessageCenter> message_center_remote;
-  TestMessageCenter test_message_center(
-      message_center_remote.BindNewPipeAndPassReceiver());
-  TestPlatformBridgeDelegate bridge_delegate;
-  NotificationPlatformBridgeLacros bridge(&bridge_delegate,
-                                          &message_center_remote);
+  gfx::Image badge = CreateRedImage(1, 2);
+  rich_data.small_image = badge;
+  gfx::Image icon = CreateRedImage(3, 4);
+
+  message_center::ButtonInfo button1;
+  button1.title = ASCIIToUTF16("button1");
+  message_center::ButtonInfo button2;
+  button2.title = ASCIIToUTF16("button2");
+  rich_data.buttons = {button1, button2};
 
-  // Create a test notification.
-  const base::string16 kTitle = base::ASCIIToUTF16("title");
-  const base::string16 kMessage = base::ASCIIToUTF16("message");
-  const base::string16 kDisplaySource = base::ASCIIToUTF16("display_source");
   message_center::Notification ui_notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, "test_id", kTitle, kMessage,
-      gfx::Image(), kDisplaySource, GURL("http://example.com/"),
-      message_center::NotifierId(), {}, nullptr);
+      message_center::NOTIFICATION_TYPE_SIMPLE, "test_id",
+      ASCIIToUTF16("title"), ASCIIToUTF16("message"), icon,
+      ASCIIToUTF16("display_source"), GURL("http://example.com/"),
+      message_center::NotifierId(), rich_data, nullptr);
 
   // Show the notification.
-  bridge.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
-                 ui_notification, /*metadata=*/nullptr);
-  message_center_remote.FlushForTesting();
-  EXPECT_EQ(1, test_message_center.display_count_);
+  bridge_.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
+                  ui_notification, /*metadata=*/nullptr);
+  message_center_remote_.FlushForTesting();
+  EXPECT_EQ(1, test_message_center_.display_count_);
 
   // Fields were serialized properly.
   crosapi::mojom::Notification* last_notification =
-      test_message_center.last_notification_.get();
+      test_message_center_.last_notification_.get();
   EXPECT_EQ("test_id", last_notification->id);
-  EXPECT_EQ(kTitle, last_notification->title);
-  EXPECT_EQ(kMessage, last_notification->message);
-  EXPECT_EQ(kDisplaySource, last_notification->display_source);
+  EXPECT_EQ(ASCIIToUTF16("title"), last_notification->title);
+  EXPECT_EQ(ASCIIToUTF16("message"), last_notification->message);
+  EXPECT_EQ(ASCIIToUTF16("display_source"), last_notification->display_source);
   EXPECT_EQ("http://example.com/", last_notification->origin_url->spec());
+  EXPECT_EQ(2, last_notification->priority);
+  EXPECT_TRUE(last_notification->require_interaction);
+  EXPECT_EQ(now, last_notification->timestamp);
+  EXPECT_EQ(ASCIIToUTF16("accessible_name"),
+            last_notification->accessible_name);
+  EXPECT_EQ(crosapi::mojom::FullscreenVisibility::kOverUser,
+            last_notification->fullscreen_visibility);
+
+  ASSERT_TRUE(last_notification->badge.has_value());
+  EXPECT_TRUE(BitmapEquals(badge.AsBitmap(), crosapi::SkBitmapFromBitmap(
+                                                 *last_notification->badge)));
+  ASSERT_TRUE(last_notification->icon.has_value());
+  EXPECT_TRUE(BitmapEquals(
+      icon.AsBitmap(), crosapi::SkBitmapFromBitmap(*last_notification->icon)));
+
+  ASSERT_EQ(2u, last_notification->buttons.size());
+  EXPECT_EQ(ASCIIToUTF16("button1"), last_notification->buttons[0]->title);
+  EXPECT_EQ(ASCIIToUTF16("button2"), last_notification->buttons[1]->title);
+}
+
+TEST_F(NotificationPlatformBridgeLacrosTest, SerializationImage) {
+  // Create a message_center notification.
+  gfx::Image image = CreateRedImage(5, 6);
+  message_center::RichNotificationData rich_data;
+  rich_data.image = image;
+  message_center::Notification ui_notification(
+      message_center::NOTIFICATION_TYPE_IMAGE, "test_id", base::string16(),
+      base::string16(), gfx::Image(), base::string16(), GURL(),
+      message_center::NotifierId(), rich_data, nullptr);
+
+  // Show the notification.
+  bridge_.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
+                  ui_notification, /*metadata=*/nullptr);
+  message_center_remote_.FlushForTesting();
+
+  // Notification was shown with correct fields.
+  crosapi::mojom::Notification* last_notification =
+      test_message_center_.last_notification_.get();
+  ASSERT_TRUE(last_notification);
+  ASSERT_TRUE(last_notification->image.has_value());
+  EXPECT_TRUE(BitmapEquals(image.AsBitmap(), crosapi::SkBitmapFromBitmap(
+                                                 *last_notification->image)));
+}
+
+TEST_F(NotificationPlatformBridgeLacrosTest, SerializationList) {
+  // Create a message_center notification.
+  message_center::NotificationItem item1;
+  item1.title = ASCIIToUTF16("title1");
+  item1.message = ASCIIToUTF16("message1");
+  message_center::NotificationItem item2;
+  item2.title = ASCIIToUTF16("title2");
+  item2.message = ASCIIToUTF16("message2");
+  message_center::RichNotificationData rich_data;
+  rich_data.items = {item1, item2};
+  message_center::Notification ui_notification(
+      message_center::NOTIFICATION_TYPE_MULTIPLE, "test_id", base::string16(),
+      base::string16(), gfx::Image(), base::string16(), GURL(),
+      message_center::NotifierId(), rich_data, nullptr);
+
+  // Show the notification.
+  bridge_.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
+                  ui_notification, /*metadata=*/nullptr);
+  message_center_remote_.FlushForTesting();
+
+  // Notification was shown with correct fields.
+  crosapi::mojom::Notification* last_notification =
+      test_message_center_.last_notification_.get();
+  ASSERT_TRUE(last_notification);
+  ASSERT_EQ(2u, last_notification->items.size());
+  EXPECT_EQ(ASCIIToUTF16("title1"), last_notification->items[0]->title);
+  EXPECT_EQ(ASCIIToUTF16("message1"), last_notification->items[0]->message);
+  EXPECT_EQ(ASCIIToUTF16("title2"), last_notification->items[1]->title);
+  EXPECT_EQ(ASCIIToUTF16("message2"), last_notification->items[1]->message);
+}
+
+TEST_F(NotificationPlatformBridgeLacrosTest, SerializationProgress) {
+  // Create a message_center notification.
+  message_center::RichNotificationData rich_data;
+  rich_data.progress = 55;
+  rich_data.progress_status = ASCIIToUTF16("status");
+  message_center::Notification ui_notification(
+      message_center::NOTIFICATION_TYPE_PROGRESS, "test_id", base::string16(),
+      base::string16(), gfx::Image(), base::string16(), GURL(),
+      message_center::NotifierId(), rich_data, nullptr);
+
+  // Show the notification.
+  bridge_.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
+                  ui_notification, /*metadata=*/nullptr);
+  message_center_remote_.FlushForTesting();
+
+  // Notification was shown with correct fields.
+  crosapi::mojom::Notification* last_notification =
+      test_message_center_.last_notification_.get();
+  ASSERT_TRUE(last_notification);
+  EXPECT_EQ(55, last_notification->progress);
+  EXPECT_EQ(ASCIIToUTF16("status"), last_notification->progress_status);
+}
+
+TEST_F(NotificationPlatformBridgeLacrosTest, UserActions) {
+  // Create a test notification.
+  message_center::Notification ui_notification(
+      message_center::NOTIFICATION_TYPE_SIMPLE, "test_id", base::string16(),
+      base::string16(), gfx::Image(), base::string16(), GURL(),
+      message_center::NotifierId(), {}, nullptr);
+
+  // Show the notification.
+  bridge_.Display(NotificationHandler::Type::TRANSIENT, /*profile=*/nullptr,
+                  ui_notification, /*metadata=*/nullptr);
+  message_center_remote_.FlushForTesting();
+  EXPECT_EQ(1, test_message_center_.display_count_);
+
+  // Grab the last notification.
+  crosapi::mojom::Notification* last_notification =
+      test_message_center_.last_notification_.get();
+  EXPECT_EQ("test_id", last_notification->id);
 
   // Grab the mojo::Remote<> for the last notification's delegate.
-  ASSERT_TRUE(test_message_center.last_notification_delegate_remote_);
+  ASSERT_TRUE(test_message_center_.last_notification_delegate_remote_);
   mojo::Remote<crosapi::mojom::NotificationDelegate>&
       notification_delegate_remote =
-          *test_message_center.last_notification_delegate_remote_;
+          *test_message_center_.last_notification_delegate_remote_;
 
-  // Verify remote user actions are forwarded through to |bridge_delegate|.
+  // Verify remote user actions are forwarded through to |bridge_delegate_|.
   notification_delegate_remote->OnNotificationClicked();
   notification_delegate_remote.FlushForTesting();
-  EXPECT_EQ(1, bridge_delegate.clicked_count_);
+  EXPECT_EQ(1, bridge_delegate_.clicked_count_);
 
   notification_delegate_remote->OnNotificationButtonClicked(/*button_index=*/0);
   notification_delegate_remote.FlushForTesting();
-  EXPECT_EQ(1, bridge_delegate.button_clicked_count_);
-  EXPECT_EQ(0, bridge_delegate.last_button_index_);
+  EXPECT_EQ(1, bridge_delegate_.button_clicked_count_);
+  EXPECT_EQ(0, bridge_delegate_.last_button_index_);
 
   notification_delegate_remote->OnNotificationSettingsButtonClicked();
   notification_delegate_remote.FlushForTesting();
-  EXPECT_EQ(1, bridge_delegate.settings_button_clicked_count_);
+  EXPECT_EQ(1, bridge_delegate_.settings_button_clicked_count_);
 
   notification_delegate_remote->OnNotificationDisabled();
   notification_delegate_remote.FlushForTesting();
-  EXPECT_EQ(1, bridge_delegate.disabled_count_);
+  EXPECT_EQ(1, bridge_delegate_.disabled_count_);
 
   // Close the notification.
-  bridge.Close(/*profile=*/nullptr, "test_id");
-  message_center_remote.FlushForTesting();
-  EXPECT_EQ(1, test_message_center.close_count_);
-  EXPECT_EQ("test_id", test_message_center.last_close_id_);
+  bridge_.Close(/*profile=*/nullptr, "test_id");
+  message_center_remote_.FlushForTesting();
+  EXPECT_EQ(1, test_message_center_.close_count_);
+  EXPECT_EQ("test_id", test_message_center_.last_close_id_);
 }
 
 }  // namespace

chromeos/crosapi/cpp/BUILD.gn

@@ -12,7 +12,12 @@ component("cpp") {
   sources = [
     "bitmap.cc",
     "bitmap.h",
+    "bitmap_util.cc",
+    "bitmap_util.h",
   ]
   configs += [ ":crosapi_implementation" ]
-  deps = [ "//base" ]
+  deps = [
+    "//base",
+    "//skia",
+  ]
 }

chromeos/crosapi/cpp/DEPS

+include_rules = [
+  "+third_party/skia/include/core",
+]

chromeos/crosapi/cpp/bitmap.cc

@@ -7,6 +7,8 @@
 namespace crosapi {
 
 Bitmap::Bitmap() = default;
+Bitmap::Bitmap(Bitmap&& other) = default;
+Bitmap& Bitmap::operator=(Bitmap&& other) = default;
 Bitmap::~Bitmap() = default;
 
 }  // namespace crosapi

chromeos/crosapi/cpp/bitmap.h

@@ -14,9 +14,13 @@
 namespace crosapi {
 
 // A 4-byte RGBA bitmap representation. Its size must be exactly equal to
-// width * height * 4.
+// width * height * 4. Move-only because copying |pixels| is expensive.
 struct COMPONENT_EXPORT(CROSAPI) Bitmap {
   Bitmap();
+  Bitmap(const Bitmap&) = delete;
+  Bitmap& operator=(const Bitmap&) = delete;
+  Bitmap(Bitmap&& other);
+  Bitmap& operator=(Bitmap&& other);
   ~Bitmap();
   uint32_t width = 0;
   uint32_t height = 0;

chromeos/crosapi/cpp/bitmap_util.cc

+// Copyright 2020 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 "chromeos/crosapi/cpp/bitmap_util.h"
+
+#include <algorithm>
+#include <vector>
+
+#include "base/check.h"
+#include "base/check_op.h"
+#include "base/numerics/checked_math.h"
+#include "base/numerics/safe_conversions.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
+
+namespace crosapi {
+
+Bitmap BitmapFromSkBitmap(const SkBitmap& sk_bitmap) {
+  int size;
+  size = base::CheckMul(sk_bitmap.width(), sk_bitmap.height()).ValueOrDie();
+  size = base::CheckMul(size, 4).ValueOrDie();
+  CHECK_EQ(sk_bitmap.computeByteSize(), base::checked_cast<size_t>(size));
+
+  uint8_t* base = static_cast<uint8_t*>(sk_bitmap.getPixels());
+  std::vector<uint8_t> bytes(base, base + sk_bitmap.computeByteSize());
+
+  crosapi::Bitmap snapshot;
+  snapshot.width = sk_bitmap.width();
+  snapshot.height = sk_bitmap.height();
+  snapshot.pixels.swap(bytes);
+  return snapshot;
+}
+
+SkBitmap SkBitmapFromBitmap(const Bitmap& snapshot) {
+  SkImageInfo info =
+      SkImageInfo::Make(snapshot.width, snapshot.height, kBGRA_8888_SkColorType,
+                        kPremul_SkAlphaType);
+  CHECK_EQ(info.computeByteSize(info.minRowBytes()), snapshot.pixels.size());
+  SkBitmap sk_bitmap;
+  CHECK(sk_bitmap.tryAllocPixels(info));
+  std::copy(snapshot.pixels.begin(), snapshot.pixels.end(),
+            static_cast<uint8_t*>(sk_bitmap.getPixels()));
+  sk_bitmap.notifyPixelsChanged();
+  return sk_bitmap;
+}
+
+}  // namespace crosapi

chromeos/crosapi/cpp/bitmap_util.h

+// Copyright 2020 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.
+
+#ifndef CHROMEOS_CROSAPI_CPP_BITMAP_UTIL_H_
+#define CHROMEOS_CROSAPI_CPP_BITMAP_UTIL_H_
+
+#include "base/component_export.h"
+
+class SkBitmap;
+
+namespace crosapi {
+
+struct Bitmap;
+
+// Converts an SkBitmap to a crosapi::Bitmap. Assumes that the bitmap is
+// unpadded, and uses 4 bytes per pixel. CHECK fails if the bitmap has an
+// invalid size (e.g. its size is not equal to width * height * 4).
+COMPONENT_EXPORT(CROSAPI) Bitmap BitmapFromSkBitmap(const SkBitmap& sk_bitmap);
+
+// Converts a crosapi::Bitmap to an SkBitmap. Assumes the bitmap is 8-bit
+// ARGB with premultiplied alpha.
+// TODO(https://crbug.com/1116652): Support more flexible image options.
+COMPONENT_EXPORT(CROSAPI) SkBitmap SkBitmapFromBitmap(const Bitmap& bitmap);
+
+}  // namespace crosapi
+
+#endif  // CHROMEOS_CROSAPI_CPP_BITMAP_UTIL_H_

chromeos/crosapi/mojom/BUILD.gn

@@ -21,6 +21,7 @@ mojom("mojom") {
         {
           mojom = "crosapi.mojom.Bitmap"
           cpp = "crosapi::Bitmap"
+          move_only = true
         },
       ]
       traits_headers = [ "//chromeos/crosapi/mojom/bitmap_mojom_traits.h" ]

chromeos/crosapi/mojom/notification.mojom

@@ -4,8 +4,9 @@
 
 module crosapi.mojom;
 
-import "mojo/public/mojom/base/time.mojom";
+import "chromeos/crosapi/mojom/bitmap.mojom";
 import "mojo/public/mojom/base/string16.mojom";
+import "mojo/public/mojom/base/time.mojom";
 import "url/mojom/url.mojom";
 
 // Type of notification to show. See the chrome.notifications extension API:
@@ -25,6 +26,30 @@ enum NotificationType {
   kProgress = 3,
 };
 
+// How to display notifications when a fullscreen window is open. For example,
+// a child account time limit notification should appear over a fullscreen
+// video.
+[Extensible]
+enum FullscreenVisibility {
+  // Do not show over fullscreen windows.
+  kNone = 0,
+
+  // Show on top of user-created fullscreen windows.
+  kOverUser = 1,
+};
+
+// List items for NotificationType::kList.
+struct NotificationItem {
+  mojo_base.mojom.String16 title;
+  mojo_base.mojom.String16 message;
+};
+
+// Data for each button. Implemented as a struct for extensibility (e.g. so we
+// could add icon buttons).
+struct ButtonInfo {
+  mojo_base.mojom.String16 title;
+};
+
 // A notification to show in the system message center. Fields exist to support
 // both the web platform notification API and the chrome.notifications extension
 // API. See documentation at:
@@ -51,6 +76,56 @@ struct Notification {
   // notification. Otherwise empty.
   url.mojom.Url? origin_url;
 
-  // TODO(https://crbug.com/1113889): Support additional fields, such as images
-  // (icon, large image), priority, timestamp, buttons, item lists, etc.
+  // Icon for the notification.
+  Bitmap? icon;
+
+  // Priority from -2 to 2. Zero is the default. Other values are clamped.
+  int32 priority;
+
+  // User must explicitly dismiss.
+  bool require_interaction;
+
+  // Time at which the notification is applicable (past, present or future).
+  // See web platform API "timestamp" and chrome.notifications "eventTime".
+  mojo_base.mojom.Time timestamp;
+
+  // For type kImage, the large image.
+  Bitmap? image;
+
+  // Badge to show the source of the notification (e.g. a 16x16 browser icon).
+  Bitmap? badge;
+
+  // Item list for type kList. Not displayed for other types.
+  array<NotificationItem> items;
+
+  // Progress from 0 to 100 for type kProgress. Negative values result in an
+  // animating "indefinite progress" indicator. Values above 100 are clamped.
+  int32 progress;
+
+  // Status message for type kProgress.
+  mojo_base.mojom.String16 progress_status;
+
+  // Up to 4 buttons. It is not guaranteed that all buttons will be visible,
+  // especially if the labels are long.
+  array<ButtonInfo> buttons;
+
+  // Pinned notifications cannot be closed by the user, only by a call to
+  // MessageCenter::CloseNotification.
+  bool pinned;
+
+  // Whether an announcement should occur (e.g. a popup or vibration) when an
+  // existing notification is updated. See web platform API
+  // Notification.renotify.
+  bool renotify;
+
+  // Whether all announcement mechanisms should be suppressed. See web platform
+  // API Notification.silent.
+  bool silent;
+
+  // Accessible description of the notification's contents. If empty, the
+  // message field will be used instead.
+  mojo_base.mojom.String16 accessible_name;
+
+  // Whether to show on top of fullscreen windows. See enum definition.
+  FullscreenVisibility fullscreen_visibility;
 };