[CrOS] Make LowDiskNotification use NotificationDisplayService instead

of direct access to MessageCenter.

Manual test: add `LowDiskSpace(10000);` to end of constructor.

Bug: 783018
Change-Id: I3c1f7dd591e4c84b9d41742e6c042e63c7003138
Reviewed-on: https://chromium-review.googlesource.com/775059
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517673}

chrome/browser/chromeos/ui/low_disk_notification.cc

@@ -13,6 +13,7 @@
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/grit/generated_resources.h"
@@ -23,7 +24,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/chromeos/resources/grit/ui_chromeos_resources.h"
-#include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_types.h"
 #include "ui/message_center/notifier_id.h"
@@ -37,31 +37,12 @@ const uint64_t kNotificationSevereThreshold = 512 << 20;  // 512MB
 constexpr base::TimeDelta kNotificationInterval =
     base::TimeDelta::FromMinutes(2);
 
-class LowDiskNotificationDelegate
-    : public message_center::NotificationDelegate {
- public:
-  LowDiskNotificationDelegate() {}
-
-  // message_center::NotificationDelegate
-  void ButtonClick(int button_index) override {
-    chrome::ShowSettingsSubPageForProfile(
-        ProfileManager::GetActiveUserProfile(), kStoragePage);
-  }
-
- private:
-  ~LowDiskNotificationDelegate() override {}
-
-  DISALLOW_COPY_AND_ASSIGN(LowDiskNotificationDelegate);
-};
-
 }  // namespace
 
 namespace chromeos {
 
 LowDiskNotification::LowDiskNotification()
-    : message_center_(g_browser_process->message_center()),
-      notification_interval_(kNotificationInterval),
-      weak_ptr_factory_(this) {
+    : notification_interval_(kNotificationInterval), weak_ptr_factory_(this) {
   DCHECK(DBusThreadManager::Get()->GetCryptohomeClient());
   DBusThreadManager::Get()->GetCryptohomeClient()->AddObserver(this);
 }
@@ -88,14 +69,16 @@ void LowDiskNotification::LowDiskSpace(uint64_t free_disk_bytes) {
   if (severity != last_notification_severity_ ||
       (severity == HIGH &&
        now - last_notification_time_ > notification_interval_)) {
-    message_center_->AddNotification(CreateNotification(severity));
+    Profile* profile = ProfileManager::GetLastUsedProfile();
+    NotificationDisplayService::GetForProfile(profile)->Display(
+        NotificationCommon::TRANSIENT, *CreateNotification(severity, profile));
     last_notification_time_ = now;
     last_notification_severity_ = severity;
   }
 }
 
 std::unique_ptr<message_center::Notification>
-LowDiskNotification::CreateNotification(Severity severity) {
+LowDiskNotification::CreateNotification(Severity severity, Profile* profile) {
   base::string16 title;
   base::string16 message;
   gfx::Image icon;
@@ -128,12 +111,17 @@ LowDiskNotification::CreateNotification(Severity severity) {
       message_center::NotifierId::SYSTEM_COMPONENT,
       ash::system_notifier::kNotifierDisk);
 
+  auto on_click = base::Bind(
+      [](Profile* profile, int button_index) {
+        chrome::ShowSettingsSubPageForProfile(profile, kStoragePage);
+      },
+      profile);
   std::unique_ptr<message_center::Notification> notification =
       ash::system_notifier::CreateSystemNotification(
           message_center::NOTIFICATION_TYPE_SIMPLE, kLowDiskId, title, message,
           icon, base::string16(), GURL(), notifier_id, optional_fields,
-          new LowDiskNotificationDelegate(), kNotificationStorageFullIcon,
-          warning_level);
+          new message_center::HandleNotificationButtonClickDelegate(on_click),
+          kNotificationStorageFullIcon, warning_level);
 
   return notification;
 }
@@ -147,11 +135,6 @@ LowDiskNotification::Severity LowDiskNotification::GetSeverity(
   return Severity::NONE;
 }
 
-void LowDiskNotification::SetMessageCenterForTest(
-    message_center::MessageCenter* message_center) {
-  message_center_ = message_center;
-}
-
 void LowDiskNotification::SetNotificationIntervalForTest(
     base::TimeDelta notification_interval) {
   notification_interval_ = notification_interval;

chrome/browser/chromeos/ui/low_disk_notification.h

@@ -15,9 +15,10 @@
 #include "base/time/time.h"
 #include "chromeos/dbus/cryptohome_client.h"
 
+class Profile;
+
 namespace message_center {
 class Notification;
-class MessageCenter;
 }
 
 namespace chromeos {
@@ -50,22 +51,19 @@ class LowDiskNotification : public CryptohomeClient::Observer {
   // Creates a notification for the specified severity.  If the severity does
   // not match a known value MEDIUM is used by default.
   std::unique_ptr<message_center::Notification> CreateNotification(
-      Severity severity);
+      Severity severity,
+      Profile* profile);
 
   // Gets the severity of the low disk status based on the amount of free space
   // left on the disk.
   Severity GetSeverity(uint64_t free_disk_bytes);
 
-  // Sets the MessageCenter instance to use.  Should only be used in tests.
-  void SetMessageCenterForTest(message_center::MessageCenter* message_center);
-
   // Sets the minimum time to wait between notifications of the same severity.
   // Should only be used in tests.
   void SetNotificationIntervalForTest(base::TimeDelta interval);
 
   base::Time last_notification_time_;
   Severity last_notification_severity_ = NONE;
-  message_center::MessageCenter* message_center_;
   base::TimeDelta notification_interval_;
   base::ThreadChecker thread_checker_;
   base::WeakPtrFactory<LowDiskNotification> weak_ptr_factory_;

chrome/browser/chromeos/ui/low_disk_notification_unittest.cc

@@ -8,17 +8,17 @@
 
 #include <utility>
 
-#include "base/test/scoped_task_environment.h"
-#include "base/threading/platform_thread.h"
 #include "base/time/time.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/grit/generated_resources.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/fake_cryptohome_client.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/message_center/fake_message_center.h"
-#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification.h"
 
 namespace {
 
@@ -30,39 +30,44 @@ const uint64_t kHighNotification = (512 << 20) - 1;
 
 namespace chromeos {
 
-class LowDiskNotificationTest : public testing::Test,
-                                public message_center::FakeMessageCenter {
+class LowDiskNotificationTest : public BrowserWithTestWindowTest {
  public:
   LowDiskNotificationTest() {}
+  ~LowDiskNotificationTest() override {}
 
   void SetUp() override {
     DBusThreadManager::GetSetterForTesting()->SetCryptohomeClient(
         std::unique_ptr<CryptohomeClient>(new FakeCryptohomeClient));
-    message_center::MessageCenter::Initialize();
+
+    BrowserWithTestWindowTest::SetUp();
+
+    tester_ = std::make_unique<NotificationDisplayServiceTester>(
+        profile_manager()->profile_manager()->GetLastUsedProfile());
+    tester_->SetNotificationAddedClosure(base::Bind(
+        &LowDiskNotificationTest::OnNotificationAdded, base::Unretained(this)));
     low_disk_notification_.reset(new LowDiskNotification());
-    low_disk_notification_->SetMessageCenterForTest(this);
-    low_disk_notification_->SetNotificationIntervalForTest(
-        base::TimeDelta::FromMilliseconds(10));
     notification_count_ = 0;
   }
 
   void TearDown() override {
     low_disk_notification_.reset();
-    last_notification_.reset();
-    message_center::MessageCenter::Shutdown();
-    DBusThreadManager::Shutdown();
+    BrowserWithTestWindowTest::TearDown();
   }
 
-  void AddNotification(
-      std::unique_ptr<message_center::Notification> notification) override {
-    last_notification_ = std::move(notification);
-    notification_count_++;
+  base::Optional<message_center::Notification> GetNotification() {
+    return tester_->GetNotification("low_disk");
   }
 
+  void SetNotificationThrottlingInterval(int ms) {
+    low_disk_notification_->SetNotificationIntervalForTest(
+        base::TimeDelta::FromMilliseconds(ms));
+  }
+
+  void OnNotificationAdded() { notification_count_++; }
+
  protected:
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  std::unique_ptr<NotificationDisplayServiceTester> tester_;
   std::unique_ptr<LowDiskNotification> low_disk_notification_;
-  std::unique_ptr<message_center::Notification> last_notification_;
   int notification_count_;
 };
 
@@ -70,8 +75,9 @@ TEST_F(LowDiskNotificationTest, MediumLevelNotification) {
   base::string16 expected_title =
       l10n_util::GetStringUTF16(IDS_LOW_DISK_NOTIFICATION_TITLE);
   low_disk_notification_->LowDiskSpace(kMediumNotification);
-  EXPECT_NE(nullptr, last_notification_);
-  EXPECT_EQ(expected_title, last_notification_->title());
+  auto notification = GetNotification();
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(expected_title, notification->title());
   EXPECT_EQ(1, notification_count_);
 }
 
@@ -80,28 +86,29 @@ TEST_F(LowDiskNotificationTest, HighLevelReplacesMedium) {
       l10n_util::GetStringUTF16(IDS_CRITICALLY_LOW_DISK_NOTIFICATION_TITLE);
   low_disk_notification_->LowDiskSpace(kMediumNotification);
   low_disk_notification_->LowDiskSpace(kHighNotification);
-  EXPECT_NE(nullptr, last_notification_);
-  EXPECT_EQ(expected_title, last_notification_->title());
+  auto notification = GetNotification();
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(expected_title, notification->title());
   EXPECT_EQ(2, notification_count_);
 }
 
 TEST_F(LowDiskNotificationTest, NotificationsAreThrottled) {
+  SetNotificationThrottlingInterval(10000000);
   low_disk_notification_->LowDiskSpace(kHighNotification);
-  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(5));
   low_disk_notification_->LowDiskSpace(kHighNotification);
   EXPECT_EQ(1, notification_count_);
 }
 
 TEST_F(LowDiskNotificationTest, HighNotificationsAreShownAfterThrottling) {
+  SetNotificationThrottlingInterval(-1);
   low_disk_notification_->LowDiskSpace(kHighNotification);
-  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(15));
   low_disk_notification_->LowDiskSpace(kHighNotification);
   EXPECT_EQ(2, notification_count_);
 }
 
 TEST_F(LowDiskNotificationTest, MediumNotificationsAreNotShownAfterThrottling) {
+  SetNotificationThrottlingInterval(-1);
   low_disk_notification_->LowDiskSpace(kMediumNotification);
-  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(15));
   low_disk_notification_->LowDiskSpace(kMediumNotification);
   EXPECT_EQ(1, notification_count_);
 }