Minimize the delegate dependencies for native extension notifications.

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

 // Copyright 2017 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/extension_notification_handler.h"
 
 #include "base/logging.h"
+#include "base/strings/nullable_string16.h"
+#include "base/strings/string_piece.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/profiles/profile.h"
+#include "chrome/common/extensions/api/notifications.h"
+#include "extensions/common/constants.h"
+#include "url/gurl.h"
+
+namespace extensions {
+
+namespace {
+
+std::string GetExtensionId(const std::string& extension_url) {
+  GURL url(extension_url);
+  if (!url.is_valid() || !url.SchemeIs(extensions::kExtensionScheme))
+    return "";
+  return url.GetOrigin().host_piece().as_string();
+}
+
+std::unique_ptr<base::ListValue> CreateBaseEventArgs(
+    const std::string& extension_id,
+    const std::string& scoped_notification_id) {
+  // Unscope the notification id before returning it.
+  size_t index_of_separator = extension_id.length() + 1;
+  DCHECK_LT(index_of_separator, scoped_notification_id.length());
+  std::string unscoped_notification_id =
+      scoped_notification_id.substr(index_of_separator);
+
+  std::unique_ptr<base::ListValue> args(new base::ListValue());
+  args->AppendString(unscoped_notification_id);
+  return args;
+}
+
+}  // namespace
 
 ExtensionNotificationHandler::ExtensionNotificationHandler() = default;
 
 ExtensionNotificationHandler::~ExtensionNotificationHandler() = default;
 
+void ExtensionNotificationHandler::OnClose(Profile* profile,

[dewittj, 2017/05/16 22:07:17]

Is it possible to unit test this?

[Miguel Garcia, 2017/05/20 07:04:10]

Yes! Done now

+                                           const std::string& origin,
+                                           const std::string& notification_id,
+                                           bool by_user) {
+  EventRouter::UserGestureState gesture =
+      by_user ? EventRouter::USER_GESTURE_ENABLED
+              : EventRouter::USER_GESTURE_NOT_ENABLED;
+  std::string extension_id(GetExtensionId(origin));

[dewittj, 2017/05/16 22:07:17]

need to check for empty string?

[Miguel Garcia, 2017/05/20 07:04:10]

I have added a DCHECK since it should not be possible at this stage for the id
to be empty.

+
+  std::unique_ptr<base::ListValue> args(
+      CreateBaseEventArgs(extension_id, notification_id));
+  args->AppendBoolean(by_user);
+  SendEvent(profile, extension_id, events::NOTIFICATIONS_ON_CLOSED,
+            api::notifications::OnClosed::kEventName, gesture, std::move(args));
+
+  ExtensionNotificationDisplayHelper* display_helper =
+      ExtensionNotificationDisplayHelperFactory::GetForProfile(profile);
+  if (display_helper)
+    display_helper->EraseDataForNotificationId(notification_id);
+}
+
+void ExtensionNotificationHandler::OnClick(
+    Profile* profile,
+    const std::string& origin,
+    const std::string& notification_id,
+    int action_index,
+    const base::NullableString16& reply) {
+  DCHECK(reply.is_null());
+
+  std::string extension_id(GetExtensionId(origin));
+  std::unique_ptr<base::ListValue> args(
+      CreateBaseEventArgs(extension_id, notification_id));
+  if (action_index > -1)

[dewittj, 2017/05/16 22:07:17]

(Not a required change) It seems like if you already need an if for this,
perhaps it's better written as

if (action_index > -1) {
  args->AppendInteger(action_index);
  SendEvent(profile, extension_id, events::NOTIFICATIONS_ON_BUTTON_CLICKED,
api::notifications::OnButtonClicked::kEventName, ...);
} else {
  SendEvent(...);
}

[Miguel Garcia, 2017/05/20 07:04:10]

Acknowledged.

+    args->AppendInteger(action_index);
+  events::HistogramValue histogram_value =
+      action_index > -1 ? events::NOTIFICATIONS_ON_BUTTON_CLICKED
+                        : events::NOTIFICATIONS_ON_CLICKED;
+  const std::string& event =

[dewittj, 2017/05/16 22:07:17]

nit: event_name

[Miguel Garcia, 2017/05/20 07:04:10]

Done.

+      action_index > -1 ? api::notifications::OnButtonClicked::kEventName
+                        : api::notifications::OnClicked::kEventName;
+
+  SendEvent(profile, extension_id, histogram_value, event,
+            EventRouter::USER_GESTURE_ENABLED, std::move(args));
+}
+
 void ExtensionNotificationHandler::OpenSettings(Profile* profile) {
   // Extension notifications don't display a settings button.
   NOTREACHED();
 }
+
+void ExtensionNotificationHandler::RegisterNotification(
+    const std::string& notification_id,
+    NotificationDelegate* delegate) {}
+
+void ExtensionNotificationHandler::SendEvent(
+    Profile* profile,
+    const std::string& extension_id,
+    events::HistogramValue histogram_value,
+    const std::string& name,

[dewittj, 2017/05/16 22:07:17]

nit: event_name

[Miguel Garcia, 2017/05/20 07:04:10]

Done.

+    EventRouter::UserGestureState user_gesture,
+    std::unique_ptr<base::ListValue> args) {
+  if (extension_id.empty())
+    return;
+
+  EventRouter* event_router = EventRouter::Get(profile);
+  if (!event_router)
+    return;
+
+  std::unique_ptr<Event> event(
+      new Event(histogram_value, name, std::move(args)));
+  event->user_gesture = user_gesture;
+  event_router->DispatchEventToExtension(extension_id, std::move(event));
+}
+
+}  // namespace extensions