Support user-agnostic notifications in NotificationDisplayService

Create an NDS for a generic/"system" profile on demand and
asynchronously to service notification requests that aren't tied to
a particular user. Apply this to WebUsbDetector, and also update
some Chrome OS specific notifications which had been doing pretty
much the same thing, but had assumed/forced synchronicity.

Bug: 783018, 793877
Change-Id: I95f4434bd6557bf846f5ed89052d4cff7f6bcf21
Reviewed-on: https://chromium-review.googlesource.com/891488
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Jun Cai <juncai@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534729}

chrome/browser/BUILD.gn

@@ -859,6 +859,8 @@ split_static_library("browser") {
     "notifications/persistent_notification_handler.h",
     "notifications/platform_notification_service_impl.cc",
     "notifications/platform_notification_service_impl.h",
+    "notifications/system_notification_helper.cc",
+    "notifications/system_notification_helper.h",
     "ntp_snippets/bookmark_last_visit_updater.cc",
     "ntp_snippets/bookmark_last_visit_updater.h",
     "ntp_snippets/content_suggestions_notifier_service_factory.cc",

chrome/browser/chromeos/net/network_portal_notification_controller.cc

@@ -27,8 +27,8 @@
 #include "chrome/browser/chromeos/net/network_portal_web_dialog.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/notifications/notification_handler.h"
+#include "chrome/browser/notifications/system_notification_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
@@ -73,8 +73,7 @@ Profile* GetProfileForPrimaryUser() {
 }
 
 void CloseNotification() {
-  NotificationDisplayService::GetForSystemNotifications()->Close(
-      NotificationHandler::Type::TRANSIENT,
+  SystemNotificationHelper::GetInstance()->Close(
       NetworkPortalNotificationController::kNotificationId);
 }
 
@@ -293,8 +292,8 @@ void NetworkPortalNotificationController::OnPortalDetectionCompleted(
         network->guid());
   }
 
-  NotificationDisplayService::GetForSystemNotifications()->Display(
-      NotificationHandler::Type::TRANSIENT, *GetNotification(network, state));
+  SystemNotificationHelper::GetInstance()->Display(
+      *GetNotification(network, state));
   UMA_HISTOGRAM_ENUMERATION(
       NetworkPortalNotificationController::kNotificationMetric,
       NetworkPortalNotificationController::NOTIFICATION_METRIC_DISPLAYED,

chrome/browser/chromeos/ui/low_disk_notification.cc

@@ -12,7 +12,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/notifications/system_notification_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/grit/generated_resources.h"
@@ -68,8 +68,8 @@ void LowDiskNotification::LowDiskSpace(uint64_t free_disk_bytes) {
   if (severity != last_notification_severity_ ||
       (severity == HIGH &&
        now - last_notification_time_ > notification_interval_)) {
-    NotificationDisplayService::GetForSystemNotifications()->Display(
-        NotificationHandler::Type::TRANSIENT, *CreateNotification(severity));
+    SystemNotificationHelper::GetInstance()->Display(
+        *CreateNotification(severity));
     last_notification_time_ = now;
     last_notification_severity_ = severity;
   }

chrome/browser/notifications/notification_platform_bridge_chromeos.cc

@@ -6,7 +6,6 @@
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/notifications/chrome_ash_message_center_client.h"
 #include "chrome/browser/notifications/notification_display_service_impl.h"
 #include "chrome/browser/profiles/profile.h"
@@ -28,16 +27,6 @@ Profile* GetProfileFromId(const std::string& profile_id, bool incognito) {
 
 }  // namespace
 
-// static
-NotificationDisplayService*
-NotificationDisplayService::GetForSystemNotifications() {
-  // System notifications (such as those for network state) aren't tied to a
-  // particular user and can show up before any user is logged in, so fall back
-  // to the signin profile, which is guaranteed to exist.
-  return NotificationDisplayService::GetForProfile(
-      chromeos::ProfileHelper::GetSigninProfile());
-}
-
 // static
 NotificationPlatformBridge* NotificationPlatformBridge::Create() {
   return new NotificationPlatformBridgeChromeOs();

chrome/browser/notifications/system_notification_helper.cc

+// Copyright 2018 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 "chrome/browser/notifications/system_notification_helper.h"
+
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/notifications/notification_display_service.h"
+#include "chrome/browser/profiles/profile_manager.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#endif
+
+SystemNotificationHelper* SystemNotificationHelper::GetInstance() {
+  return base::Singleton<SystemNotificationHelper>::get();
+}
+
+SystemNotificationHelper::SystemNotificationHelper() = default;
+SystemNotificationHelper::~SystemNotificationHelper() = default;
+
+void SystemNotificationHelper::Display(
+    const message_center::Notification& notification) {
+  pending_notifications_[notification.id()] = notification;
+  g_browser_process->profile_manager()->CreateProfileAsync(
+      GetProfilePath(),
+      base::AdaptCallbackForRepeating(
+          base::BindOnce(&SystemNotificationHelper::DoDisplayNotification,
+                         weak_factory_.GetWeakPtr(), notification.id())),
+      base::string16(), std::string(), std::string());
+}
+
+void SystemNotificationHelper::Close(const std::string& notification_id) {
+  size_t erased = pending_notifications_.erase(notification_id);
+  Profile* profile =
+      g_browser_process->profile_manager()->GetProfileByPath(GetProfilePath());
+  if (!profile)
+    return;
+
+  // If the profile has finished loading, we should have already removed the
+  // notification from the pending list in DoDisplayNotification().
+  DCHECK_EQ(0u, erased);
+  NotificationDisplayService::GetForProfile(profile->GetOffTheRecordProfile())
+      ->Close(NotificationHandler::Type::TRANSIENT, notification_id);
+}
+
+void SystemNotificationHelper::DoDisplayNotification(
+    const std::string& notification_id,
+    Profile* profile,
+    Profile::CreateStatus status) {
+  auto iter = pending_notifications_.find(notification_id);
+  if (iter == pending_notifications_.end())
+    return;
+
+  if (profile) {
+    // We use the incognito profile both to match
+    // ProfileHelper::GetSigninProfile() and to be sure we don't store anything
+    // about it across program restarts.
+    NotificationDisplayService::GetForProfile(profile->GetOffTheRecordProfile())
+        ->Display(NotificationHandler::Type::TRANSIENT, iter->second);
+  }
+  pending_notifications_.erase(iter);
+}
+
+// static
+Profile* SystemNotificationHelper::GetProfileForTesting() {
+  return g_browser_process->profile_manager()
+      ->GetProfile(GetProfilePath())
+      ->GetOffTheRecordProfile();
+}
+
+// static
+base::FilePath SystemNotificationHelper::GetProfilePath() {
+#if defined(OS_CHROMEOS)
+  // System notifications (such as those for network state) aren't tied to a
+  // particular user and can show up before any user is logged in, so use the
+  // signin profile, which is guaranteed to already exist.
+  return chromeos::ProfileHelper::GetSigninProfileDir();
+#else
+  // The "system profile" probably hasn't been loaded yet.
+  return g_browser_process->profile_manager()->GetSystemProfilePath();
+#endif
+}

chrome/browser/notifications/system_notification_helper.h

+// Copyright 2018 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 CHROME_BROWSER_NOTIFICATIONS_SYSTEM_NOTIFICATION_HELPER_H_
+#define CHROME_BROWSER_NOTIFICATIONS_SYSTEM_NOTIFICATION_HELPER_H_
+
+#include "base/files/file_path.h"
+#include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/profiles/profile.h"
+#include "ui/message_center/public/cpp/notification.h"
+
+// This class assists in displaying notifications that do not have an associated
+// Profile. It uses a system profile which may not be loaded. Thus, Display() is
+// not always synchronous.
+class SystemNotificationHelper {
+ public:
+  // Returns the singleton instance.
+  static SystemNotificationHelper* GetInstance();
+
+  // Displays a notification which isn't tied to a normal user profile. The
+  // notification will be displayed asynchronously if the generic profile has
+  // not yet been loaded.
+  void Display(const message_center::Notification& notification);
+
+  // Closes a notification which isn't tied to a normal user profile.
+  void Close(const std::string& notification_id);
+
+  // Loads the profile used for the profile-agnostic NotificationDisplayService.
+  static Profile* GetProfileForTesting();
+
+ private:
+  friend struct base::DefaultSingletonTraits<SystemNotificationHelper>;
+
+  SystemNotificationHelper();
+  ~SystemNotificationHelper();
+
+  void DoDisplayNotification(const std::string& notification_id,
+                             Profile* profile,
+                             Profile::CreateStatus status);
+
+  // Returns the path for a non-user-specific profile. This will be used for
+  // system notifications which aren't tied to a particular normal profile.
+  static base::FilePath GetProfilePath();
+
+  // Notifications are added to this queue when the system profile has not yet
+  // been loaded. They are removed in Close() if it's called before the profile
+  // loads, or after the notification is displayed if that happens first.
+  std::map<std::string, message_center::Notification> pending_notifications_;
+
+  base::WeakPtrFactory<SystemNotificationHelper> weak_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(SystemNotificationHelper);
+};
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_SYSTEM_NOTIFICATION_HELPER_H_

chrome/browser/ui/ash/network/network_state_notifier.cc

@@ -12,7 +12,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/chromeos/net/shill_error.h"
-#include "chrome/browser/notifications/notification_display_service.h"
+#include "chrome/browser/notifications/system_notification_helper.h"
 #include "chrome/browser/ui/ash/system_tray_client.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/network/network_configuration_handler.h"
@@ -91,8 +91,7 @@ void ShowErrorNotification(const std::string& service_path,
           GetErrorNotificationVectorIcon(network_type),
           message_center::SystemNotificationWarningLevel::CRITICAL_WARNING);
   notification->set_priority(message_center::SYSTEM_PRIORITY);
-  NotificationDisplayService::GetForSystemNotifications()->Display(
-      NotificationHandler::Type::TRANSIENT, *notification);
+  SystemNotificationHelper::GetInstance()->Display(*notification);
 }
 
 bool ShouldConnectFailedNotificationBeShown(const std::string& error_name,
@@ -288,8 +287,7 @@ void NetworkStateNotifier::UpdateCellularActivating(
           cellular->network_technology() == shill::kNetworkTechnologyLte
               ? IDR_AURA_UBER_TRAY_NETWORK_NOTIFICATION_LTE
               : IDR_AURA_UBER_TRAY_NETWORK_NOTIFICATION_3G);
-  NotificationDisplayService::GetForSystemNotifications()->Display(
-      NotificationHandler::Type::TRANSIENT,
+  SystemNotificationHelper::GetInstance()->Display(
       *message_center::Notification::CreateSystemNotification(
           kNetworkActivateNotificationId,
           l10n_util::GetStringUTF16(IDS_NETWORK_CELLULAR_ACTIVATED_TITLE),
@@ -326,8 +324,7 @@ void NetworkStateNotifier::ShowMobileActivationErrorForGuid(
                    << guid;
     return;
   }
-  NotificationDisplayService::GetForSystemNotifications()->Display(
-      NotificationHandler::Type::TRANSIENT,
+  SystemNotificationHelper::GetInstance()->Display(
       *message_center::Notification::CreateSystemNotification(
           kNetworkActivateNotificationId,
           l10n_util::GetStringUTF16(IDS_NETWORK_ACTIVATION_ERROR_TITLE),
@@ -341,8 +338,7 @@ void NetworkStateNotifier::ShowMobileActivationErrorForGuid(
 }
 
 void NetworkStateNotifier::RemoveConnectNotification() {
-  NotificationDisplayService::GetForSystemNotifications()->Close(
-      NotificationHandler::Type::TRANSIENT, kNetworkConnectNotificationId);
+  SystemNotificationHelper::GetInstance()->Close(kNetworkConnectNotificationId);
 }
 
 void NetworkStateNotifier::ConnectErrorPropertiesSucceeded(

chrome/browser/usb/web_usb_detector.cc

@@ -11,6 +11,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/net/referrer.h"
+#include "chrome/browser/notifications/system_notification_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -34,7 +35,6 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.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"
@@ -109,8 +109,7 @@ class WebUsbNotificationDelegate : public TabStripModelObserver,
       // If the disposition is not already set, go ahead and set it.
       if (disposition_ == WEBUSB_NOTIFICATION_CLOSED)
         disposition_ = WEBUSB_NOTIFICATION_CLOSED_MANUAL_NAVIGATION;
-      message_center::MessageCenter::Get()->RemoveNotification(
-          notification_id_, false /* by_user */);
+      SystemNotificationHelper::GetInstance()->Close(notification_id_);
     }
   }
 
@@ -198,33 +197,25 @@ void WebUsbDetector::OnDeviceAdded(scoped_refptr<device::UsbDevice> device) {
   std::string notification_id = device->guid();
 
   message_center::RichNotificationData rich_notification_data;
-  std::unique_ptr<message_center::Notification> notification(
-      new message_center::Notification(
-          message_center::NOTIFICATION_TYPE_SIMPLE, notification_id,
-          l10n_util::GetStringFUTF16(
-              IDS_WEBUSB_DEVICE_DETECTED_NOTIFICATION_TITLE, product_name),
-          l10n_util::GetStringFUTF16(
-              IDS_WEBUSB_DEVICE_DETECTED_NOTIFICATION,
-              url_formatter::FormatUrlForSecurityDisplay(
-                  landing_page,
-                  url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC)),
-          gfx::Image(gfx::CreateVectorIcon(vector_icons::kUsbIcon, 64,
-                                           gfx::kChromeIconGrey)),
-          base::string16(), GURL(),
-          message_center::NotifierId(
-              message_center::NotifierId::SYSTEM_COMPONENT, kNotifierWebUsb),
-          rich_notification_data,
-          new WebUsbNotificationDelegate(landing_page, notification_id)));
-
-  notification->SetSystemPriority();
-  message_center::MessageCenter::Get()->AddNotification(
-      std::move(notification));
+  message_center::Notification notification(
+      message_center::NOTIFICATION_TYPE_SIMPLE, notification_id,
+      l10n_util::GetStringFUTF16(IDS_WEBUSB_DEVICE_DETECTED_NOTIFICATION_TITLE,
+                                 product_name),
+      l10n_util::GetStringFUTF16(
+          IDS_WEBUSB_DEVICE_DETECTED_NOTIFICATION,
+          url_formatter::FormatUrlForSecurityDisplay(
+              landing_page, url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC)),
+      gfx::Image(gfx::CreateVectorIcon(vector_icons::kUsbIcon, 64,
+                                       gfx::kChromeIconGrey)),
+      base::string16(), GURL(),
+      message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
+                                 kNotifierWebUsb),
+      rich_notification_data,
+      new WebUsbNotificationDelegate(landing_page, notification_id));
+  notification.SetSystemPriority();
+  SystemNotificationHelper::GetInstance()->Display(notification);
 }
 
 void WebUsbDetector::OnDeviceRemoved(scoped_refptr<device::UsbDevice> device) {
-  std::string notification_id = device->guid();
-  message_center::MessageCenter* message_center =
-      message_center::MessageCenter::Get();
-  if (message_center->FindVisibleNotificationById(notification_id))
-    message_center->RemoveNotification(notification_id, false /* by_user */);
+  SystemNotificationHelper::GetInstance()->Close(device->guid());
 }