Migrate three more Chrome OS system notifications to use of

NotificationDisplayService.

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

chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_apitest.cc

@@ -18,8 +18,9 @@
 #include "chrome/browser/chromeos/file_system_provider/request_value.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/extensions/extension_apitest.h"
-#include "ui/base/ui_base_types.h"
-#include "ui/message_center/message_center.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
+#include "ui/message_center/notification.h"
+#include "ui/message_center/notification_delegate.h"
 
 namespace extensions {
 namespace {
@@ -62,11 +63,15 @@ class NotificationButtonClicker : public RequestManager::Observer {
 
  private:
   void ClickButton() {
-    g_browser_process->message_center()->ClickOnNotificationButton(
-        file_system_info_.mount_path().value(), ui::DIALOG_BUTTON_OK);
+    base::Optional<message_center::Notification> notification =
+        NotificationDisplayServiceTester::Get()->GetNotification(
+            file_system_info_.mount_path().value());
+    if (notification)
+      notification->delegate()->ButtonClick(0);
   }
 
   ProvidedFileSystemInfo file_system_info_;
+
   DISALLOW_COPY_AND_ASSIGN(NotificationButtonClicker);
 };
 
@@ -110,6 +115,7 @@ class AbortOnUnresponsivePerformer : public Observer {
  private:
   Service* service_;  // Not owned.
   std::vector<std::unique_ptr<NotificationButtonClicker>> clickers_;
+
   DISALLOW_COPY_AND_ASSIGN(AbortOnUnresponsivePerformer);
 };
 
@@ -126,7 +132,15 @@ class FileSystemProviderApiTest : public ExtensionApiTest {
         test_data_dir_.AppendASCII("file_system_provider/test_util"),
         kFlagEnableIncognito);
     ASSERT_TRUE(extension);
+
+    display_service_ = std::make_unique<NotificationDisplayServiceTester>(
+        browser()->profile());
   }
+
+  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FileSystemProviderApiTest);
 };
 
 IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, Mount) {

chrome/browser/chromeos/file_system_provider/notification_manager.cc

@@ -6,13 +6,12 @@
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/chrome_app_icon_loader.h"
+#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/signin/core/account_id/account_id.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_delegate.h"
 #include "ui/message_center/notification_types.h"
@@ -60,8 +59,8 @@ NotificationManager::NotificationManager(
 
 NotificationManager::~NotificationManager() {
   if (callbacks_.size()) {
-    g_browser_process->message_center()->RemoveNotification(
-        file_system_info_.mount_path().value(), false /* by_user */);
+    NotificationDisplayService::GetForProfile(profile_)->Close(
+        NotificationCommon::TRANSIENT, GetNotificationId());
   }
 }
 
@@ -69,26 +68,18 @@ void NotificationManager::ShowUnresponsiveNotification(
     int id,
     const NotificationCallback& callback) {
   callbacks_[id] = callback;
-
-  if (callbacks_.size() == 1) {
-    g_browser_process->message_center()->AddNotification(CreateNotification());
-  } else {
-    g_browser_process->message_center()->UpdateNotification(
-        file_system_info_.mount_path().value(), CreateNotification());
-  }
+  ShowNotification();
 }
 
 void NotificationManager::HideUnresponsiveNotification(int id) {
   callbacks_.erase(id);
 
   if (callbacks_.size()) {
-    g_browser_process->message_center()->UpdateNotification(
-        file_system_info_.mount_path().value(), CreateNotification());
-    return;
+    ShowNotification();
+  } else {
+    NotificationDisplayService::GetForProfile(profile_)->Close(
+        NotificationCommon::TRANSIENT, GetNotificationId());
   }
-
-  g_browser_process->message_center()->RemoveNotification(
-      file_system_info_.mount_path().value(), false /* by_user */);
 }
 
 void NotificationManager::OnButtonClick(int button_index) {
@@ -102,12 +93,14 @@ void NotificationManager::OnClose() {
 void NotificationManager::OnAppImageUpdated(const std::string& id,
                                             const gfx::ImageSkia& image) {
   extension_icon_.reset(new gfx::Image(image));
-  g_browser_process->message_center()->UpdateNotification(
-      file_system_info_.mount_path().value(), CreateNotification());
+  ShowNotification();
+}
+
+std::string NotificationManager::GetNotificationId() {
+  return file_system_info_.mount_path().value();
 }
 
-std::unique_ptr<message_center::Notification>
-NotificationManager::CreateNotification() {
+void NotificationManager::ShowNotification() {
   if (!extension_icon_.get())
     icon_loader_->FetchImage(file_system_info_.provider_id());
 
@@ -118,26 +111,25 @@ NotificationManager::CreateNotification() {
 
   message_center::NotifierId notifier_id(
       message_center::NotifierId::SYSTEM_COMPONENT,
-      file_system_info_.mount_path().value());
+      "chrome://file_system_provider_notification");
   notifier_id.profile_id =
       multi_user_util::GetAccountIdFromProfile(profile_).GetUserEmail();
 
-  std::unique_ptr<message_center::Notification> notification(
-      new message_center::Notification(
-          message_center::NOTIFICATION_TYPE_SIMPLE,
-          file_system_info_.mount_path().value(),
-          base::UTF8ToUTF16(file_system_info_.display_name()),
-          l10n_util::GetStringUTF16(
-              callbacks_.size() == 1
-                  ? IDS_FILE_SYSTEM_PROVIDER_UNRESPONSIVE_WARNING
-                  : IDS_FILE_SYSTEM_PROVIDER_MANY_UNRESPONSIVE_WARNING),
-          extension_icon_.get() ? *extension_icon_.get() : gfx::Image(),
-          base::string16(),  // display_source
-          GURL(), notifier_id, rich_notification_data,
-          new ProviderNotificationDelegate(this)));
-
-  notification->SetSystemPriority();
-  return notification;
+  message_center::Notification notification(
+      message_center::NOTIFICATION_TYPE_SIMPLE, GetNotificationId(),
+      base::UTF8ToUTF16(file_system_info_.display_name()),
+      l10n_util::GetStringUTF16(
+          callbacks_.size() == 1
+              ? IDS_FILE_SYSTEM_PROVIDER_UNRESPONSIVE_WARNING
+              : IDS_FILE_SYSTEM_PROVIDER_MANY_UNRESPONSIVE_WARNING),
+      extension_icon_.get() ? *extension_icon_.get() : gfx::Image(),
+      base::string16(),  // display_source
+      GURL(), notifier_id, rich_notification_data,
+      new ProviderNotificationDelegate(this));
+  notification.SetSystemPriority();
+
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationCommon::TRANSIENT, notification);
 }
 
 void NotificationManager::OnNotificationResult(NotificationResult result) {

chrome/browser/chromeos/file_system_provider/notification_manager.h

@@ -22,10 +22,6 @@ class Image;
 class ImageSkia;
 }  // message gfx
 
-namespace message_center {
-class Notification;
-}  // namespace message_center
-
 namespace chromeos {
 namespace file_system_provider {
 
@@ -58,8 +54,12 @@ class NotificationManager : public NotificationManagerInterface,
  private:
   typedef std::map<int, NotificationCallback> CallbackMap;
 
-  // Creates a notification object for the actual state of the manager.
-  std::unique_ptr<message_center::Notification> CreateNotification();
+  std::string GetNotificationId();
+
+  // Creates and displays a notification object for the actual state of the
+  // manager. This will either add a new one or update the existing
+  // notification.
+  void ShowNotification();
 
   // Handles a notification result by calling all registered callbacks and
   // clearing the list.

chrome/browser/chromeos/file_system_provider/service.cc

@@ -63,7 +63,14 @@ Service::Service(Profile* profile,
   extension_registry_->AddObserver(this);
 }
 
-Service::~Service() {
+Service::~Service() {}
+
+// static
+Service* Service::Get(content::BrowserContext* context) {
+  return ServiceFactory::Get(context);
+}
+
+void Service::Shutdown() {
   extension_registry_->RemoveObserver(this);
 
   // Provided file systems should be already unmounted because of receiving
@@ -85,11 +92,6 @@ Service::~Service() {
   DCHECK_EQ(0u, file_system_map_.size());
 }
 
-// static
-Service* Service::Get(content::BrowserContext* context) {
-  return ServiceFactory::Get(context);
-}
-
 void Service::AddObserver(Observer* observer) {
   DCHECK(observer);
   observers_.AddObserver(observer);

chrome/browser/chromeos/file_system_provider/service.h

@@ -81,6 +81,12 @@ class Service : public KeyedService,
   Service(Profile* profile, extensions::ExtensionRegistry* extension_registry);
   ~Service() override;
 
+  // Gets the singleton instance for the |context|.
+  static Service* Get(content::BrowserContext* context);
+
+  // KeyedService:
+  void Shutdown() override;
+
   // Sets a custom ProvidedFileSystemInterface factory. Used by unit tests,
   // where an event router is not available.
   void SetDefaultFileSystemFactoryForTesting(
@@ -144,9 +150,6 @@ class Service : public KeyedService,
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Gets the singleton instance for the |context|.
-  static Service* Get(content::BrowserContext* context);
-
   // extensions::ExtensionRegistryObserver overrides.
   void OnExtensionUnloaded(content::BrowserContext* browser_context,
                            const extensions::Extension* extension,

chrome/browser/chromeos/file_system_provider/service_factory.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
+#include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "extensions/browser/extension_registry.h"
@@ -36,6 +37,7 @@ ServiceFactory::ServiceFactory()
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(extensions::ExtensionRegistryFactory::GetInstance());
   DependsOn(extensions::ExtensionSystemFactory::GetInstance());
+  DependsOn(NotificationDisplayServiceFactory::GetInstance());
 }
 
 ServiceFactory::~ServiceFactory() {}

chrome/browser/chromeos/hats/hats_notification_controller.cc

@@ -15,7 +15,7 @@
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
@@ -172,8 +172,8 @@ void HatsNotificationController::ButtonClick(int /* button_index */) {
   HatsDialog::CreateAndShow(IsGoogleUser(profile_->GetProfileUserName()));
 
   // Remove the notification.
-  g_browser_process->notification_ui_manager()->CancelById(
-      kNotificationId, NotificationUIManager::GetProfileID(profile_));
+  NotificationDisplayService::GetForProfile(profile_)->Close(
+      NotificationCommon::TRANSIENT, kNotificationId);
 }
 
 // message_center::NotificationDelegate override:
@@ -223,7 +223,9 @@ void HatsNotificationController::OnPortalDetectionCompleted(
         message_center::kSystemNotificationColorNormal)));
     notification.set_vector_small_image(kNotificationGoogleIcon);
   }
-  g_browser_process->notification_ui_manager()->Add(notification, profile_);
+
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationCommon::TRANSIENT, notification);
 }
 
 void HatsNotificationController::UpdateLastInteractionTime() {

chrome/browser/chromeos/login/quick_unlock/quick_unlock_notification_controller.cc

@@ -5,12 +5,11 @@
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_notification_controller.h"
 
 #include "ash/system/system_notifier.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_factory.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_storage.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_utils.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
@@ -171,7 +170,8 @@ void QuickUnlockNotificationController::Observe(
 
   std::unique_ptr<message_center::Notification> notification =
       CreateNotification();
-  g_browser_process->notification_ui_manager()->Add(*notification, profile_);
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationCommon::TRANSIENT, *notification);
 }
 
 // message_center::NotificationDelegate override:
@@ -191,8 +191,8 @@ void QuickUnlockNotificationController::Click() {
   SetNotificationPreferenceWasShown();
 
   // Remove the notification from tray.
-  g_browser_process->notification_ui_manager()->CancelById(
-      params_.notification_id, NotificationUIManager::GetProfileID(profile_));
+  NotificationDisplayService::GetForProfile(profile_)->Close(
+      NotificationCommon::TRANSIENT, params_.notification_id);
 }
 
 void QuickUnlockNotificationController::SetNotificationPreferenceWasShown() {

chrome/browser/notifications/notification_display_service_tester.cc

@@ -11,6 +11,11 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "ui/message_center/notification.h"
 
+namespace {
+// Pointer to currently active tester, which is assumed to be a singleton.
+NotificationDisplayServiceTester* g_tester = nullptr;
+}  // namespace
+
 NotificationDisplayServiceTester::NotificationDisplayServiceTester(
     Profile* profile)
     : profile_(profile) {
@@ -19,13 +24,20 @@ NotificationDisplayServiceTester::NotificationDisplayServiceTester(
   display_service_ = static_cast<StubNotificationDisplayService*>(
       NotificationDisplayServiceFactory::GetInstance()->SetTestingFactoryAndUse(
           profile_, &StubNotificationDisplayService::FactoryForTests));
+  g_tester = this;
 }
 
 NotificationDisplayServiceTester::~NotificationDisplayServiceTester() {
+  g_tester = nullptr;
   NotificationDisplayServiceFactory::GetInstance()->SetTestingFactory(profile_,
                                                                       nullptr);
 }
 
+// static
+NotificationDisplayServiceTester* NotificationDisplayServiceTester::Get() {
+  return g_tester;
+}
+
 void NotificationDisplayServiceTester::SetNotificationAddedClosure(
     base::RepeatingClosure closure) {
   display_service_->SetNotificationAddedClosure(std::move(closure));
@@ -37,6 +49,12 @@ NotificationDisplayServiceTester::GetDisplayedNotificationsForType(
   return display_service_->GetDisplayedNotificationsForType(type);
 }
 
+base::Optional<message_center::Notification>
+NotificationDisplayServiceTester::GetNotification(
+    const std::string& notification_id) {
+  return display_service_->GetNotification(notification_id);
+}
+
 const NotificationCommon::Metadata*
 NotificationDisplayServiceTester::GetMetadataForNotification(
     const message_center::Notification& notification) {

chrome/browser/notifications/notification_display_service_tester.h

@@ -9,6 +9,7 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/notifications/notification_common.h"
 
 class Profile;
@@ -28,6 +29,9 @@ class NotificationDisplayServiceTester {
   explicit NotificationDisplayServiceTester(Profile* profile);
   ~NotificationDisplayServiceTester();
 
+  // Returns the currently active tester, if any.
+  static NotificationDisplayServiceTester* Get();
+
   // Sets |closure| to be invoked when any notification has been added.
   void SetNotificationAddedClosure(base::RepeatingClosure closure);
 
@@ -38,6 +42,9 @@ class NotificationDisplayServiceTester {
   const NotificationCommon::Metadata* GetMetadataForNotification(
       const message_center::Notification& notification);
 
+  base::Optional<message_center::Notification> GetNotification(
+      const std::string& notification_id);
+
   // Simulates the notification identified by |notification_id| being closed due
   // to external events, such as the user dismissing it when |by_user| is set.
   // When |silent| is set, the notification handlers won't be informed of the

chrome/browser/notifications/stub_notification_display_service.cc

@@ -40,6 +40,20 @@ StubNotificationDisplayService::GetDisplayedNotificationsForType(
   return notifications;
 }
 
+base::Optional<message_center::Notification>
+StubNotificationDisplayService::GetNotification(
+    const std::string& notification_id) {
+  auto iter = std::find_if(notifications_.begin(), notifications_.end(),
+                           [notification_id](const NotificationData& data) {
+                             return data.notification.id() == notification_id;
+                           });
+
+  if (iter == notifications_.end())
+    return base::nullopt;
+
+  return iter->notification;
+}
+
 const NotificationCommon::Metadata*
 StubNotificationDisplayService::GetMetadataForNotification(
     const message_center::Notification& notification) {
@@ -71,10 +85,13 @@ void StubNotificationDisplayService::RemoveNotification(
 
   if (!silent) {
     NotificationHandler* handler = GetNotificationHandler(notification_type);
-    DCHECK(handler);
-
-    handler->OnClose(profile_, iter->notification.origin_url(), notification_id,
-                     by_user);
+    if (notification_type == NotificationCommon::TRANSIENT) {
+      DCHECK(!handler);
+      iter->notification.delegate()->Close(by_user);
+    } else {
+      handler->OnClose(profile_, iter->notification.origin_url(),
+                       notification_id, by_user);
+    }
   }
 
   notifications_.erase(iter);
@@ -84,11 +101,15 @@ void StubNotificationDisplayService::RemoveAllNotifications(
     NotificationCommon::Type notification_type,
     bool by_user) {
   NotificationHandler* handler = GetNotificationHandler(notification_type);
-  DCHECK(handler);
+  DCHECK_NE(!!handler, notification_type == NotificationCommon::TRANSIENT);
   for (auto iter = notifications_.begin(); iter != notifications_.end();) {
     if (iter->type == notification_type) {
-      handler->OnClose(profile_, iter->notification.origin_url(),
-                       iter->notification.id(), by_user);
+      if (handler) {
+        handler->OnClose(profile_, iter->notification.origin_url(),
+                         iter->notification.id(), by_user);
+      } else {
+        iter->notification.delegate()->Close(by_user);
+      }
       iter = notifications_.erase(iter);
     } else {
       iter++;
@@ -105,9 +126,10 @@ void StubNotificationDisplayService::Display(
   Close(notification_type, notification.id());
 
   NotificationHandler* handler = GetNotificationHandler(notification_type);
-  DCHECK(handler);
-
-  handler->OnShow(profile_, notification.id());
+  if (notification_type == NotificationCommon::TRANSIENT)
+    DCHECK(!handler);
+  else
+    handler->OnShow(profile_, notification.id());
   if (notification_added_closure_)
     notification_added_closure_.Run();
 

chrome/browser/notifications/stub_notification_display_service.h

@@ -11,6 +11,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "ui/message_center/notification.h"
@@ -37,6 +38,9 @@ class StubNotificationDisplayService : public NotificationDisplayService {
   std::vector<message_center::Notification> GetDisplayedNotificationsForType(
       NotificationCommon::Type type) const;
 
+  base::Optional<message_center::Notification> GetNotification(
+      const std::string& notification_id);
+
   const NotificationCommon::Metadata* GetMetadataForNotification(
       const message_center::Notification& notification);