Minimize the delegate dependencies for non persistent notifications.

chrome/browser/extensions/api/notifications/notifications_api.cc

 // Copyright (c) 2012 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/extensions/api/notifications/notifications_api.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/guid.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/notifications/extension_notification_display_helper.h"
 #include "chrome/browser/extensions/api/notifications/extension_notification_display_helper_factory.h"
+#include "chrome/browser/notifications/native_notification_delegate.h"
 #include "chrome/browser/notifications/notification.h"
+#include "chrome/browser/notifications/notification_common.h"
+#include "chrome/browser/notifications/notification_delegate.h"
 #include "chrome/browser/notifications/notifier_state_tracker.h"
 #include "chrome/browser/notifications/notifier_state_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/api/notifications/notification_style.h"
 #include "chrome/common/features.h"
 #include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
 #include "components/keyed_service/core/keyed_service_shutdown_notifier.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ skipping 164 matching lines @@
   ShutdownNotifierFactory()
       : BrowserContextKeyedServiceShutdownNotifierFactory(
             "NotificationsApiDelegate") {
     DependsOn(ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());
   }
   ~ShutdownNotifierFactory() override {}
 
   DISALLOW_COPY_AND_ASSIGN(ShutdownNotifierFactory);
 };
 
-// Temporary native notification api delagate, it is only used
-// to extract the delegate id.
-// This is an interim state until the work in
-// https://bugs.chromium.org/p/chromium/issues/detail?id=720345
-// is completed. We need a small delegate shim since the
-// Notification object has a non virtual method (delegate_id) that is
-// used all over the place whose implementation returns delegate->id()
-#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-class NativeNotificationApiDelegate : public NotificationDelegate {
- public:
-  NativeNotificationApiDelegate(const std::string& extension_id,
-                                const std::string& notification_id)
-      : scoped_notification_id_(
-            CreateScopedIdentifier(extension_id, notification_id)) {}
-
-  std::string id() const override { return scoped_notification_id_; }
-
- private:
-  ~NativeNotificationApiDelegate() override = default;
-  const std::string scoped_notification_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(NativeNotificationApiDelegate);
-};
-#endif  // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-
 // Message center based notification delegate with all the functionality.
 class NotificationApiDelegate : public NotificationDelegate {
  public:
   NotificationApiDelegate(ChromeAsyncExtensionFunction* api_function,
                           Profile* profile,
                           const std::string& extension_id,
                           const std::string& id)
       : api_function_(api_function),
         event_router_(EventRouter::Get(profile)),
         display_helper_(
@@ skipping 269 matching lines @@
           base::UTF8ToUTF16(api_item.message)));
     }
   }
 
   if (options->is_clickable.get())
     optional_fields.clickable = *options->is_clickable;
 
   // Create the notification api delegate. Ownership passed to the notification.
   NotificationDelegate* api_delegate;
 #if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
-  if (base::FeatureList::IsEnabled(features::kNativeNotifications)) {
-    api_delegate = new NativeNotificationApiDelegate(extension_->id(), id);
+  if (base::FeatureList::IsEnabled(features::kNativeNotifications) &&
+      g_browser_process->notification_platform_bridge()) {

[dewittj, 2017/06/01 18:44:45]

Probably fine but it feels strange to ask the browser process about the bridge
and then never use it.

[Miguel Garcia, 2017/06/02 12:37:13]

You are right, this is an artifact of how to decide to use native notifications
(which is needed for linux) we are trying to generalize it but I will be using
one single delegate before that so this particular check will go away.

+    api_delegate = new NativeNotificationDelegate(
+        CreateScopedIdentifier(extension_->id(), id));
   } else {
     api_delegate =
         new NotificationApiDelegate(this, GetProfile(), extension_->id(), id);
   }
 #else
   api_delegate =
       new NotificationApiDelegate(this, GetProfile(), extension_->id(), id);
 #endif  // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
 
   Notification notification(
@@ skipping 332 matching lines @@
           : api::notifications::PERMISSION_LEVEL_DENIED;
 
   SetResult(
       base::MakeUnique<base::Value>(api::notifications::ToString(result)));
   SendResponse(true);
 
   return true;
 }
 
 }  // namespace extensions