Make EasyUnlockNotification on Chrome OS use NotificationDisplayService.

Bug: 783018
Change-Id: Ic1340627bacde0109e92afd45b2cb3c164dd9594
Reviewed-on: https://chromium-review.googlesource.com/783661Reviewed-by: Tim Song <tengs@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521493}

chrome/browser/signin/easy_unlock_notification_controller.cc

@@ -9,7 +9,6 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/signin/easy_unlock_notification_controller_chromeos.h"
-#include "ui/message_center/message_center.h"
 #endif
 
 namespace {
@@ -38,8 +37,7 @@ class EasyUnlockNotificationControllerStub
 std::unique_ptr<EasyUnlockNotificationController>
 EasyUnlockNotificationController::Create(Profile* profile) {
 #if defined(OS_CHROMEOS)
-  return base::MakeUnique<EasyUnlockNotificationControllerChromeOS>(
-      profile, message_center::MessageCenter::Get());
+  return base::MakeUnique<EasyUnlockNotificationControllerChromeOS>(profile);
 #else
   return base::MakeUnique<EasyUnlockNotificationControllerStub>();
 #endif

chrome/browser/signin/easy_unlock_notification_controller_chromeos.cc

@@ -7,6 +7,7 @@
 #include "base/guid.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
@@ -53,12 +54,8 @@ std::unique_ptr<message_center::Notification> CreateNotification(
 }  // namespace
 
 EasyUnlockNotificationControllerChromeOS::
-    EasyUnlockNotificationControllerChromeOS(
-        Profile* profile,
-        message_center::MessageCenter* message_center)
-    : profile_(profile),
-      message_center_(message_center),
-      weak_ptr_factory_(this) {}
+    EasyUnlockNotificationControllerChromeOS(Profile* profile)
+    : profile_(profile), weak_ptr_factory_(this) {}
 
 EasyUnlockNotificationControllerChromeOS::
     ~EasyUnlockNotificationControllerChromeOS() {}
@@ -110,8 +107,8 @@ void EasyUnlockNotificationControllerChromeOS::ShowPairingChangeNotification() {
 void EasyUnlockNotificationControllerChromeOS::
     ShowPairingChangeAppliedNotification(const std::string& phone_name) {
   // Remove the pairing change notification if it is still being shown.
-  message_center_->RemoveNotification(kEasyUnlockPairingChangeNotifierId,
-                                      false /* by_user */);
+  NotificationDisplayService::GetForProfile(profile_)->Close(
+      NotificationHandler::Type::TRANSIENT, kEasyUnlockPairingChangeNotifierId);
 
   message_center::RichNotificationData rich_notification_data;
   rich_notification_data.buttons.push_back(
@@ -154,13 +151,8 @@ void EasyUnlockNotificationControllerChromeOS::ShowPromotionNotification() {
 void EasyUnlockNotificationControllerChromeOS::ShowNotification(
     std::unique_ptr<message_center::Notification> notification) {
   notification->SetSystemPriority();
-  std::string notification_id = notification->id();
-  if (message_center_->FindVisibleNotificationById(notification_id)) {
-    message_center_->UpdateNotification(notification_id,
-                                        std::move(notification));
-  } else {
-    message_center_->AddNotification(std::move(notification));
-  }
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationHandler::Type::TRANSIENT, *notification);
 }
 
 void EasyUnlockNotificationControllerChromeOS::LaunchEasyUnlockSettings() {

chrome/browser/signin/easy_unlock_notification_controller_chromeos.h

@@ -12,7 +12,6 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/signin/easy_unlock_notification_controller.h"
-#include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_delegate.h"
 
@@ -22,9 +21,7 @@ class Profile;
 class EasyUnlockNotificationControllerChromeOS
     : public EasyUnlockNotificationController {
  public:
-  EasyUnlockNotificationControllerChromeOS(
-      Profile* profile,
-      message_center::MessageCenter* message_center);
+  explicit EasyUnlockNotificationControllerChromeOS(Profile* profile);
   ~EasyUnlockNotificationControllerChromeOS() override;
 
   // EasyUnlockNotificationController:
@@ -68,8 +65,6 @@ class EasyUnlockNotificationControllerChromeOS
 
   Profile* profile_;
 
-  message_center::MessageCenter* message_center_;
-
   base::WeakPtrFactory<EasyUnlockNotificationControllerChromeOS>
       weak_ptr_factory_;
 

chrome/browser/signin/easy_unlock_notification_controller_chromeos_unittest.cc

@@ -5,11 +5,9 @@
 #include "chrome/browser/signin/easy_unlock_notification_controller_chromeos.h"
 
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/test/base/testing_profile.h"
-#include "content/public/test/test_browser_thread_bundle.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "ui/message_center/fake_message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_types.h"
 
@@ -17,57 +15,11 @@ namespace {
 
 const char kPhoneName[] = "Nexus 6";
 
-class TestMessageCenter : public message_center::FakeMessageCenter {
- public:
-  TestMessageCenter() : message_center::FakeMessageCenter() {}
-  ~TestMessageCenter() override {}
-
-  // message_center::FakeMessageCenter:
-  message_center::Notification* FindVisibleNotificationById(
-      const std::string& id) override {
-    auto iter = std::find_if(
-        notifications_.begin(), notifications_.end(),
-        [id](const std::shared_ptr<message_center::Notification> notification) {
-          return notification->id() == id;
-        });
-    return iter != notifications_.end() ? iter->get() : nullptr;
-  }
-
-  void AddNotification(
-      std::unique_ptr<message_center::Notification> notification) override {
-    notifications_.push_back(std::move(notification));
-  }
-
-  void UpdateNotification(
-      const std::string& old_id,
-      std::unique_ptr<message_center::Notification> new_notification) override {
-    RemoveNotification(old_id, false /* by_user */);
-    AddNotification(std::move(new_notification));
-  }
-
-  void RemoveNotification(const std::string& id, bool by_user) override {
-    if (!FindVisibleNotificationById(id))
-      return;
-
-    notifications_.erase(std::find_if(
-        notifications_.begin(), notifications_.end(),
-        [id](const std::shared_ptr<message_center::Notification> notification) {
-          return notification->id() == id;
-        }));
-  }
-
- private:
-  std::vector<std::shared_ptr<message_center::Notification>> notifications_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestMessageCenter);
-};
-
 class TestableNotificationController
     : public EasyUnlockNotificationControllerChromeOS {
  public:
-  TestableNotificationController(Profile* profile,
-                                 message_center::MessageCenter* message_center)
-      : EasyUnlockNotificationControllerChromeOS(profile, message_center) {}
+  explicit TestableNotificationController(Profile* profile)
+      : EasyUnlockNotificationControllerChromeOS(profile) {}
 
   ~TestableNotificationController() override {}
 
@@ -81,17 +33,28 @@ class TestableNotificationController
 
 }  // namespace
 
-class EasyUnlockNotificationControllerChromeOSTest : public ::testing::Test {
+class EasyUnlockNotificationControllerChromeOSTest
+    : public BrowserWithTestWindowTest {
  protected:
-  EasyUnlockNotificationControllerChromeOSTest()
-      : notification_controller_(&profile_, &message_center_) {}
+  EasyUnlockNotificationControllerChromeOSTest() {}
 
   ~EasyUnlockNotificationControllerChromeOSTest() override {}
 
-  const content::TestBrowserThreadBundle thread_bundle_;
-  TestingProfile profile_;
-  TestMessageCenter message_center_;
-  testing::StrictMock<TestableNotificationController> notification_controller_;
+  void SetUp() override {
+    BrowserWithTestWindowTest::SetUp();
+
+    display_service_ =
+        std::make_unique<NotificationDisplayServiceTester>(profile());
+    notification_controller_ =
+        std::make_unique<testing::StrictMock<TestableNotificationController>>(
+            profile());
+  }
+
+  // const content::TestBrowserThreadBundle thread_bundle_;
+  // TestMessageCenter message_center_;
+  std::unique_ptr<testing::StrictMock<TestableNotificationController>>
+      notification_controller_;
+  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(EasyUnlockNotificationControllerChromeOSTest);
@@ -101,19 +64,19 @@ TEST_F(EasyUnlockNotificationControllerChromeOSTest,
        TestShowChromebookAddedNotification) {
   const char kNotificationId[] = "easyunlock_notification_ids.chromebook_added";
 
-  notification_controller_.ShowChromebookAddedNotification();
-  message_center::Notification* notification =
-      message_center_.FindVisibleNotificationById(kNotificationId);
+  notification_controller_->ShowChromebookAddedNotification();
+  base::Optional<message_center::Notification> notification =
+      display_service_->GetNotification(kNotificationId);
   ASSERT_TRUE(notification);
   ASSERT_EQ(1u, notification->buttons().size());
   EXPECT_EQ(message_center::SYSTEM_PRIORITY, notification->priority());
 
   // Clicking notification button should launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->ButtonClick(0);
 
   // Clicking the notification itself should also launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->Click();
 }
 
@@ -121,19 +84,19 @@ TEST_F(EasyUnlockNotificationControllerChromeOSTest,
        TestShowPairingChangeNotification) {
   const char kNotificationId[] = "easyunlock_notification_ids.pairing_change";
 
-  notification_controller_.ShowPairingChangeNotification();
-  message_center::Notification* notification =
-      message_center_.FindVisibleNotificationById(kNotificationId);
+  notification_controller_->ShowPairingChangeNotification();
+  base::Optional<message_center::Notification> notification =
+      display_service_->GetNotification(kNotificationId);
   ASSERT_TRUE(notification);
   ASSERT_EQ(2u, notification->buttons().size());
   EXPECT_EQ(message_center::SYSTEM_PRIORITY, notification->priority());
 
   // Clicking 1st notification button should lock screen settings.
-  EXPECT_CALL(notification_controller_, LockScreen());
+  EXPECT_CALL(*notification_controller_, LockScreen());
   notification->ButtonClick(0);
 
   // Clicking 2nd notification button should launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->ButtonClick(1);
 
   // Clicking the notification itself should do nothing.
@@ -145,9 +108,9 @@ TEST_F(EasyUnlockNotificationControllerChromeOSTest,
   const char kNotificationId[] =
       "easyunlock_notification_ids.pairing_change_applied";
 
-  notification_controller_.ShowPairingChangeAppliedNotification(kPhoneName);
-  message_center::Notification* notification =
-      message_center_.FindVisibleNotificationById(kNotificationId);
+  notification_controller_->ShowPairingChangeAppliedNotification(kPhoneName);
+  base::Optional<message_center::Notification> notification =
+      display_service_->GetNotification(kNotificationId);
   ASSERT_TRUE(notification);
   ASSERT_EQ(1u, notification->buttons().size());
   EXPECT_EQ(message_center::SYSTEM_PRIORITY, notification->priority());
@@ -157,11 +120,11 @@ TEST_F(EasyUnlockNotificationControllerChromeOSTest,
             notification->message().find(base::UTF8ToUTF16(kPhoneName)));
 
   // Clicking notification button should launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->ButtonClick(0);
 
   // Clicking the notification itself should also launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->Click();
 }
 
@@ -171,30 +134,30 @@ TEST_F(EasyUnlockNotificationControllerChromeOSTest,
   const char kPairingAppliedId[] =
       "easyunlock_notification_ids.pairing_change_applied";
 
-  notification_controller_.ShowPairingChangeNotification();
-  EXPECT_TRUE(message_center_.FindVisibleNotificationById(kPairingChangeId));
+  notification_controller_->ShowPairingChangeNotification();
+  EXPECT_TRUE(display_service_->GetNotification(kPairingChangeId));
 
-  notification_controller_.ShowPairingChangeAppliedNotification(kPhoneName);
-  EXPECT_FALSE(message_center_.FindVisibleNotificationById(kPairingChangeId));
-  EXPECT_TRUE(message_center_.FindVisibleNotificationById(kPairingAppliedId));
+  notification_controller_->ShowPairingChangeAppliedNotification(kPhoneName);
+  EXPECT_FALSE(display_service_->GetNotification(kPairingChangeId));
+  EXPECT_TRUE(display_service_->GetNotification(kPairingAppliedId));
 }
 
 TEST_F(EasyUnlockNotificationControllerChromeOSTest,
        TestShowPromotionNotification) {
   const char kNotificationId[] = "easyunlock_notification_ids.promotion";
 
-  notification_controller_.ShowPromotionNotification();
-  message_center::Notification* notification =
-      message_center_.FindVisibleNotificationById(kNotificationId);
+  notification_controller_->ShowPromotionNotification();
+  base::Optional<message_center::Notification> notification =
+      display_service_->GetNotification(kNotificationId);
   ASSERT_TRUE(notification);
   ASSERT_EQ(1u, notification->buttons().size());
   EXPECT_EQ(message_center::SYSTEM_PRIORITY, notification->priority());
 
   // Clicking notification button should launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->ButtonClick(0);
 
   // Clicking the notification itself should also launch settings.
-  EXPECT_CALL(notification_controller_, LaunchEasyUnlockSettings());
+  EXPECT_CALL(*notification_controller_, LaunchEasyUnlockSettings());
   notification->Click();
 }