Extract selection of NotificationPlatformBridge

This introduces a new NotificationPlatformBridgeDelegator that is
responsible for selecting the correct platform bridge for notification
events.

Bug: None
Change-Id: I2aaff169887e281ab46fc51b8297a8cdf610ed9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401161Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806945}

chrome/browser/BUILD.gn

@@ -916,6 +916,8 @@ static_library("browser") {
     "notifications/notification_permission_context.h",
     "notifications/notification_platform_bridge.cc",
     "notifications/notification_platform_bridge.h",
+    "notifications/notification_platform_bridge_delegator.cc",
+    "notifications/notification_platform_bridge_delegator.h",
     "notifications/notification_trigger_scheduler.cc",
     "notifications/notification_trigger_scheduler.h",
     "notifications/notification_ui_manager.h",

chrome/browser/notifications/notification_display_service_impl.cc

@@ -10,33 +10,25 @@
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "build/buildflag.h"
 #include "chrome/browser/browser_features.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/non_persistent_notification_handler.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
-#include "chrome/browser/notifications/notification_platform_bridge.h"
 #include "chrome/browser/notifications/persistent_notification_handler.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/updates/announcement_notification/announcement_notification_handler.h"
-#include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
-#include "ui/base/ui_base_features.h"
+#include "extensions/buildflags/buildflags.h"
 #include "ui/message_center/public/cpp/notification.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/api/notifications/extension_notification_handler.h"
 #endif
 
-#if BUILDFLAG(ENABLE_MESSAGE_CENTER)
-#include "chrome/browser/notifications/notification_platform_bridge_message_center.h"
-#endif
-
 #if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_MAC) || \
     defined(OS_WIN)
 #include "chrome/browser/nearby_sharing/nearby_notification_handler.h"
@@ -44,81 +36,8 @@
 #include "chrome/browser/sharing/sharing_notification_handler.h"
 #endif
 
-#if defined(OS_WIN)
-#include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/notifications/notification_platform_bridge_win.h"
-#endif
-
 namespace {
 
-#if !defined(OS_CHROMEOS)
-bool NativeNotificationsEnabled(Profile* profile) {
-#if defined(OS_LINUX) || defined(OS_CHROMEOS)
-  if (profile) {
-    PrefService* prefs = profile->GetPrefs();
-    if (!prefs->GetBoolean(prefs::kAllowNativeNotifications))
-      return false;
-  }
-#endif
-
-  return base::FeatureList::IsEnabled(features::kNativeNotifications);
-}
-#endif
-
-// Returns the NotificationPlatformBridge to use for the current platform.
-// Will return a nullptr for platforms that don't support native notifications.
-//
-// Platforms behave as follows:
-//
-//   * Android
-//     Always uses native notifications.
-//
-//   * Mac OS X, Linux, Windows 10 RS1+
-//     Uses native notifications by default, but can fall back to the message
-//     center if base::kNativeNotifications is disabled or initialization fails.
-//
-//   * Chrome OS
-//     Always uses the message center, either through the message center
-//     notification platform bridge when base::kNativeNotifications is disabled,
-//     which means the message center runs in-process, or through the Chrome OS
-//     specific bridge when the flag is enabled, which displays out-of-process.
-//
-// Please try to keep this comment up to date when changing behaviour on one of
-// the platforms supported by the browser.
-NotificationPlatformBridge* GetNativeNotificationPlatformBridge(
-    Profile* profile) {
-#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-#if defined(OS_ANDROID)
-  DCHECK(NativeNotificationsEnabled(profile));
-  return g_browser_process->notification_platform_bridge();
-#elif defined(OS_WIN)
-  if (NotificationPlatformBridgeWin::NativeNotificationEnabled())
-    return g_browser_process->notification_platform_bridge();
-#elif defined(OS_CHROMEOS)
-  return g_browser_process->notification_platform_bridge();
-#else
-  if (NativeNotificationsEnabled(profile) &&
-      g_browser_process->notification_platform_bridge()) {
-    return g_browser_process->notification_platform_bridge();
-  }
-#endif
-#endif  // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-
-  // The platform does not support, or has not enabled, native notifications.
-  return nullptr;
-}
-
-// Returns the NotificationPlatformBridge to use for the message center. May be
-// a nullptr for platforms where the message center is not available.
-std::unique_ptr<NotificationPlatformBridge> CreateMessageCenterBridge(
-    Profile* profile) {
-#if BUILDFLAG(ENABLE_MESSAGE_CENTER)
-  return std::make_unique<NotificationPlatformBridgeMessageCenter>(profile);
-#else
-  return nullptr;
-#endif
-}
-
 void OperationCompleted() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
@@ -141,9 +60,7 @@ void NotificationDisplayServiceImpl::RegisterProfilePrefs(
 }
 
 NotificationDisplayServiceImpl::NotificationDisplayServiceImpl(Profile* profile)
-    : profile_(profile),
-      message_center_bridge_(CreateMessageCenterBridge(profile_)),
-      bridge_(GetNativeNotificationPlatformBridge(profile_)) {
+    : profile_(profile) {
   // TODO(peter): Move these to the NotificationDisplayServiceFactory.
   if (profile_) {
     AddNotificationHandler(
@@ -179,15 +96,11 @@ NotificationDisplayServiceImpl::NotificationDisplayServiceImpl(Profile* profile)
 #endif
   }
 
-  // Initialize the bridge if native notifications are available, otherwise
-  // signal that the bridge could not be initialized.
-  if (bridge_) {
-    bridge_->SetReadyCallback(base::BindOnce(
-        &NotificationDisplayServiceImpl::OnNotificationPlatformBridgeReady,
-        weak_factory_.GetWeakPtr()));
-  } else {
-    OnNotificationPlatformBridgeReady(false /* success */);
-  }
+  bridge_delegator_ = std::make_unique<NotificationPlatformBridgeDelegator>(
+      profile_,
+      base::BindOnce(
+          &NotificationDisplayServiceImpl::OnNotificationPlatformBridgeReady,
+          weak_factory_.GetWeakPtr()));
 }
 
 NotificationDisplayServiceImpl::~NotificationDisplayServiceImpl() {
@@ -253,13 +166,7 @@ NotificationHandler* NotificationDisplayServiceImpl::GetNotificationHandler(
 }
 
 void NotificationDisplayServiceImpl::Shutdown() {
-  if (!bridge_initialized_)
-    return;
-
-  if (message_center_bridge_)
-    message_center_bridge_->DisplayServiceShutDown(profile_);
-  if (bridge_)
-    bridge_->DisplayServiceShutDown(profile_);
+  bridge_delegator_->DisplayServiceShutDown();
 }
 
 void NotificationDisplayServiceImpl::Display(
@@ -273,7 +180,7 @@ void NotificationDisplayServiceImpl::Display(
 
   CHECK(profile_ || notification_type == NotificationHandler::Type::TRANSIENT);
 
-  if (!bridge_initialized_) {
+  if (!bridge_delegator_initialized_) {
     actions_.push(base::BindOnce(&NotificationDisplayServiceImpl::Display,
                                  weak_factory_.GetWeakPtr(), notification_type,
                                  notification, std::move(metadata)));
@@ -283,16 +190,8 @@ void NotificationDisplayServiceImpl::Display(
   for (auto& observer : observers_)
     observer.OnNotificationDisplayed(notification, metadata.get());
 
-#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-  NotificationPlatformBridge* bridge =
-      NotificationPlatformBridge::CanHandleType(notification_type)
-          ? bridge_
-          : message_center_bridge_.get();
-  DCHECK(bridge);
-
-  bridge->Display(notification_type, profile_, notification,
-                  std::move(metadata));
-#endif
+  bridge_delegator_->Display(notification_type, notification,
+                             std::move(metadata));
 
   NotificationHandler* handler = GetNotificationHandler(notification_type);
   if (handler)
@@ -304,34 +203,26 @@ void NotificationDisplayServiceImpl::Close(
     const std::string& notification_id) {
   CHECK(profile_ || notification_type == NotificationHandler::Type::TRANSIENT);
 
-  if (!bridge_initialized_) {
+  if (!bridge_delegator_initialized_) {
     actions_.push(base::BindOnce(&NotificationDisplayServiceImpl::Close,
                                  weak_factory_.GetWeakPtr(), notification_type,
                                  notification_id));
     return;
   }
 
-#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-  NotificationPlatformBridge* bridge =
-      NotificationPlatformBridge::CanHandleType(notification_type)
-          ? bridge_
-          : message_center_bridge_.get();
-  DCHECK(bridge);
-
-  bridge->Close(profile_, notification_id);
-#endif
+  bridge_delegator_->Close(notification_type, notification_id);
 }
 
 void NotificationDisplayServiceImpl::GetDisplayed(
     DisplayedNotificationsCallback callback) {
-  if (!bridge_initialized_) {
+  if (!bridge_delegator_initialized_) {
     actions_.push(base::BindOnce(&NotificationDisplayServiceImpl::GetDisplayed,
                                  weak_factory_.GetWeakPtr(),
                                  std::move(callback)));
     return;
   }
 
-  bridge_->GetDisplayed(profile_, std::move(callback));
+  bridge_delegator_->GetDisplayed(std::move(callback));
 }
 
 void NotificationDisplayServiceImpl::AddObserver(Observer* observer) {
@@ -367,24 +258,9 @@ void NotificationDisplayServiceImpl::ProfileLoadedCallback(
                                                 action_index, reply, by_user);
 }
 
-void NotificationDisplayServiceImpl::OnNotificationPlatformBridgeReady(
-    bool success) {
+void NotificationDisplayServiceImpl::OnNotificationPlatformBridgeReady() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS) && !defined(OS_CHROMEOS)
-  if (NativeNotificationsEnabled(profile_)) {
-    UMA_HISTOGRAM_BOOLEAN("Notifications.UsingNativeNotificationCenter",
-                          success);
-  }
-#endif
-
-  if (!success) {
-    // Fall back to the message center if initialization failed. Initialization
-    // must always succeed on platforms where the message center is unavailable.
-    DCHECK(message_center_bridge_);
-    bridge_ = message_center_bridge_.get();
-  }
-
-  bridge_initialized_ = true;
+  bridge_delegator_initialized_ = true;
 
   // Flush any pending actions that have yet to execute.
   while (!actions_.empty()) {

chrome/browser/notifications/notification_display_service_impl.h

@@ -16,9 +16,9 @@
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/notifications/notification_handler.h"
+#include "chrome/browser/notifications/notification_platform_bridge_delegator.h"
 
 class GURL;
-class NotificationPlatformBridge;
 class Profile;
 
 namespace user_prefs {
@@ -52,7 +52,7 @@ class NotificationDisplayServiceImpl : public NotificationDisplayService {
   // received and dispatched to the right consumer depending on the type of
   // notification. Consumers include, service workers, pages, extensions...
   //
-  // TODO(peter): Remove this in favor of multiple targetted methods.
+  // TODO(peter): Remove this in favor of multiple targeted methods.
   virtual void ProcessNotificationOperation(
       NotificationCommon::Operation operation,
       NotificationHandler::Type notification_type,
@@ -93,21 +93,21 @@ class NotificationDisplayServiceImpl : public NotificationDisplayService {
                                     Profile* profile);
 
  private:
-  // Called when the NotificationPlatformBridge may have been initialized.
-  void OnNotificationPlatformBridgeReady(bool success);
+  // Called when the NotificationPlatformBridgeDelegator has been initialized.
+  void OnNotificationPlatformBridgeReady();
 
   Profile* profile_;
 
-  // Bridge responsible for displaying notifications on the platform. The
-  // message center's bridge is maintained for platforms where it is available.
-  std::unique_ptr<NotificationPlatformBridge> message_center_bridge_;
-  NotificationPlatformBridge* bridge_;
+  // This NotificationPlatformBridgeDelegator delegates to either the native
+  // bridge or to the MessageCenter if there is no native bridge or it does not
+  // support certain notification types.
+  std::unique_ptr<NotificationPlatformBridgeDelegator> bridge_delegator_;
 
   // Tasks that need to be run once the display bridge has been initialized.
   base::queue<base::OnceClosure> actions_;
 
-  // Boolean tracking whether the |bridge_| has been initialized for use.
-  bool bridge_initialized_ = false;
+  // Boolean tracking whether the |bridge_delegator_| has been initialized.
+  bool bridge_delegator_initialized_ = false;
 
   // Map containing the notification handlers responsible for processing events.
   std::map<NotificationHandler::Type, std::unique_ptr<NotificationHandler>>

chrome/browser/notifications/notification_platform_bridge_delegator.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 "chrome/browser/notifications/notification_platform_bridge_delegator.h"
+
+#include <algorithm>
+#include <utility>
+#include <vector>
+
+#include "base/barrier_closure.h"
+#include "base/bind.h"
+#include "base/check.h"
+#include "base/metrics/histogram_functions.h"
+#include "build/build_config.h"
+#include "build/buildflag.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/notifications/notification_display_service_impl.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
+#include "ui/base/ui_base_features.h"
+
+#if BUILDFLAG(ENABLE_MESSAGE_CENTER)
+#include "chrome/browser/notifications/notification_platform_bridge_message_center.h"
+#endif
+
+#if defined(OS_WIN)
+#include "chrome/browser/notifications/notification_platform_bridge_win.h"
+#endif
+
+namespace {
+
+// Returns if the current platform has native notifications enabled.
+// Platforms behave as follows:
+//
+//   * Android, Chrome OS
+//     Always uses native notifications.
+//
+//   * Windows before 10 RS4 (incl. Win8 & Win7)
+//     Always uses message center.
+//
+//   * Mac OS X, Linux, Windows 10 RS4+
+//     Uses native notifications by default, but can fall back to the message
+//     center if features::kNativeNotifications is disabled or initialization
+//     fails. Linux additionally checks if prefs::kAllowNativeNotifications is
+//     disabled and falls back to the message center if so.
+//
+// Please try to keep this comment up to date when changing behaviour on one of
+// the platforms supported by the browser.
+bool NativeNotificationsEnabled(Profile* profile) {
+#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
+#if defined(OS_CHROMEOS) || defined(OS_ANDROID)
+  return true;
+#elif defined(OS_WIN)
+  return NotificationPlatformBridgeWin::NativeNotificationEnabled();
+#elif defined(OS_LINUX)
+  if (profile) {
+    // Prefs take precedence over flags.
+    PrefService* prefs = profile->GetPrefs();
+    if (!prefs->GetBoolean(prefs::kAllowNativeNotifications))
+      return false;
+  }
+#endif
+  return base::FeatureList::IsEnabled(features::kNativeNotifications);
+#endif  // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
+  return false;
+}
+
+NotificationPlatformBridge* GetNativeNotificationPlatformBridge(
+    Profile* profile) {
+  if (NativeNotificationsEnabled(profile))
+    return g_browser_process->notification_platform_bridge();
+
+  // The platform does not support, or has not enabled, native notifications.
+  return nullptr;
+}
+
+// Returns the NotificationPlatformBridge to use for the message center. May be
+// a nullptr for platforms where the message center is not available.
+std::unique_ptr<NotificationPlatformBridge> CreateMessageCenterBridge(
+    Profile* profile) {
+#if BUILDFLAG(ENABLE_MESSAGE_CENTER)
+  return std::make_unique<NotificationPlatformBridgeMessageCenter>(profile);
+#else
+  return nullptr;
+#endif
+}
+
+}  // namespace
+
+NotificationPlatformBridgeDelegator::NotificationPlatformBridgeDelegator(
+    Profile* profile,
+    base::OnceClosure ready_callback)
+    : profile_(profile),
+      message_center_bridge_(CreateMessageCenterBridge(profile_)),
+      native_bridge_(GetNativeNotificationPlatformBridge(profile_)),
+      ready_callback_(std::move(ready_callback)) {
+  // Initialize the |native_bridge_| if native notifications are available,
+  // otherwise signal that the bridge could not be initialized.
+  if (native_bridge_) {
+    native_bridge_->SetReadyCallback(
+        base::BindOnce(&NotificationPlatformBridgeDelegator::
+                           OnNativeNotificationPlatformBridgeReady,
+                       weak_factory_.GetWeakPtr()));
+  } else {
+    OnNativeNotificationPlatformBridgeReady(/*success=*/false);
+  }
+}
+
+NotificationPlatformBridgeDelegator::~NotificationPlatformBridgeDelegator() =
+    default;
+
+void NotificationPlatformBridgeDelegator::Display(
+    NotificationHandler::Type notification_type,
+    const message_center::Notification& notification,
+    std::unique_ptr<NotificationCommon::Metadata> metadata) {
+  NotificationPlatformBridge* bridge = GetBridgeForType(notification_type);
+  DCHECK(bridge);
+  bridge->Display(notification_type, profile_, notification,
+                  std::move(metadata));
+}
+
+void NotificationPlatformBridgeDelegator::Close(
+    NotificationHandler::Type notification_type,
+    const std::string& notification_id) {
+  NotificationPlatformBridge* bridge = GetBridgeForType(notification_type);
+  DCHECK(bridge);
+  bridge->Close(profile_, notification_id);
+}
+
+void NotificationPlatformBridgeDelegator::GetDisplayed(
+    GetDisplayedNotificationsCallback callback) const {
+  // TODO(knollr): Query both bridges to get all notifications.
+  NotificationPlatformBridge* bridge =
+      native_bridge_ ? native_bridge_ : message_center_bridge_.get();
+  DCHECK(bridge);
+  bridge->GetDisplayed(profile_, std::move(callback));
+}
+
+void NotificationPlatformBridgeDelegator::DisplayServiceShutDown() {
+  if (message_center_bridge_)
+    message_center_bridge_->DisplayServiceShutDown(profile_);
+  if (native_bridge_)
+    native_bridge_->DisplayServiceShutDown(profile_);
+}
+
+NotificationPlatformBridge*
+NotificationPlatformBridgeDelegator::GetBridgeForType(
+    NotificationHandler::Type type) {
+#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
+  // Prefer the native bridge if available and it can handle |type|.
+  if (native_bridge_ && NotificationPlatformBridge::CanHandleType(type))
+    return native_bridge_;
+#endif  // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
+  return message_center_bridge_.get();
+}
+
+void NotificationPlatformBridgeDelegator::
+    OnNativeNotificationPlatformBridgeReady(bool success) {
+  if (!success) {
+    // Fall back to the message center if initialization failed. Initialization
+    // must always succeed on platforms where the message center is unavailable.
+    DCHECK(message_center_bridge_);
+    native_bridge_ = nullptr;
+  }
+
+  base::UmaHistogramBoolean("Notifications.UsingNativeNotificationCenter",
+                            native_bridge_ != nullptr);
+
+  if (ready_callback_)
+    std::move(ready_callback_).Run();
+}

chrome/browser/notifications/notification_platform_bridge_delegator.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 CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_DELEGATOR_H_
+#define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_DELEGATOR_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/optional.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/notifications/displayed_notifications_dispatch_callback.h"
+#include "chrome/browser/notifications/notification_common.h"
+#include "chrome/browser/notifications/notification_handler.h"
+#include "chrome/browser/notifications/notification_platform_bridge.h"
+#include "ui/message_center/public/cpp/notification.h"
+
+class Profile;
+
+// This class is responsible for delegating notification events to either the
+// native NotificationPlatformBridge or fall back to Chrome's own message
+// center implementation. This can happen if there is no native support for
+// notifications on this platform (or it has been disabled via flags). We also
+// delegate to the message center if the given notification type is not
+// supported on the native bridge.
+class NotificationPlatformBridgeDelegator {
+ public:
+  NotificationPlatformBridgeDelegator(Profile* profile,
+                                      base::OnceClosure ready_callback);
+  NotificationPlatformBridgeDelegator(
+      const NotificationPlatformBridgeDelegator&) = delete;
+  NotificationPlatformBridgeDelegator& operator=(
+      const NotificationPlatformBridgeDelegator&) = delete;
+  ~NotificationPlatformBridgeDelegator();
+
+  void Display(NotificationHandler::Type notification_type,
+               const message_center::Notification& notification,
+               std::unique_ptr<NotificationCommon::Metadata> metadata);
+
+  void Close(NotificationHandler::Type notification_type,
+             const std::string& notification_id);
+
+  void GetDisplayed(GetDisplayedNotificationsCallback callback) const;
+
+  void DisplayServiceShutDown();
+
+ private:
+  // Returns the NotificationPlatformBridge to use for |type|. This method is
+  // expected to return a valid bridge, either the native or message center one.
+  NotificationPlatformBridge* GetBridgeForType(NotificationHandler::Type type);
+
+  // Called when the |native_bridge_| may have been initialized.
+  void OnNativeNotificationPlatformBridgeReady(bool success);
+
+  Profile* profile_;
+
+  // Bridge responsible for displaying notifications on the platform. The
+  // message center's bridge is maintained for platforms where it is available.
+  std::unique_ptr<NotificationPlatformBridge> message_center_bridge_;
+  NotificationPlatformBridge* native_bridge_;
+  base::OnceClosure ready_callback_;
+
+  base::WeakPtrFactory<NotificationPlatformBridgeDelegator> weak_factory_{this};
+};
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_DELEGATOR_H_