Remove the WebNotificationDelegate class

Its only responsibility is to forward calls to a NotificationHandler of
a specific type, which should instead be done as an implementation
detail of the notification system.

Bug: 
Change-Id: Ia7e541c4b789b68d61209647294e420f9cde0681
Reviewed-on: https://chromium-review.googlesource.com/806220
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521748}

chrome/browser/BUILD.gn

@@ -842,8 +842,6 @@ split_static_library("browser") {
     "notifications/persistent_notification_handler.h",
     "notifications/platform_notification_service_impl.cc",
     "notifications/platform_notification_service_impl.h",
-    "notifications/web_notification_delegate.cc",
-    "notifications/web_notification_delegate.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/extensions/api/notifications/notifications_api.cc

@@ -27,7 +27,6 @@
 #include "chrome/browser/notifications/notification_handler.h"
 #include "chrome/browser/notifications/notifier_state_tracker.h"
 #include "chrome/browser/notifications/notifier_state_tracker_factory.h"
-#include "chrome/browser/notifications/web_notification_delegate.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/notifications/notification_style.h"
 #include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
@@ -376,10 +375,7 @@ bool NotificationsApiFunction::CreateNotification(
       base::UTF8ToUTF16(extension_->name()), extension_->url(),
       message_center::NotifierId(message_center::NotifierId::APPLICATION,
                                  extension_->id()),
-      optional_fields,
-      new WebNotificationDelegate(NotificationHandler::Type::EXTENSION,
-                                  GetProfile(), notification_id,
-                                  extension_->url()));
+      optional_fields, nullptr /* delegate */);
 
   // Apply the "requireInteraction" flag. The value defaults to false.
   notification.set_never_timeout(options->require_interaction &&

chrome/browser/notifications/message_center_display_service.cc

@@ -36,28 +36,56 @@ class PassThroughDelegate : public message_center::NotificationDelegate {
     DCHECK_NE(notification_type, NotificationHandler::Type::TRANSIENT);
   }
 
+  void SettingsClick() override {
+    NotificationDisplayServiceFactory::GetForProfile(profile_)
+        ->ProcessNotificationOperation(
+            NotificationCommon::SETTINGS, notification_type_,
+            notification_.origin_url(), notification_.id(), base::nullopt,
+            base::nullopt, base::nullopt /* by_user */);
+  }
+
+  void DisableNotification() override {
+    NotificationDisplayServiceFactory::GetForProfile(profile_)
+        ->ProcessNotificationOperation(
+            NotificationCommon::DISABLE_PERMISSION, notification_type_,
+            notification_.origin_url(), notification_.id(),
+            base::nullopt /* action_index */, base::nullopt /* reply */,
+            base::nullopt /* by_user */);
+  }
+
   void Close(bool by_user) override {
     NotificationDisplayServiceFactory::GetForProfile(profile_)
         ->ProcessNotificationOperation(
             NotificationCommon::CLOSE, notification_type_,
-            notification_.origin_url(), notification_.id(), base::nullopt,
-            base::nullopt, by_user);
+            notification_.origin_url(), notification_.id(),
+            base::nullopt /* action_index */, base::nullopt /* reply */,
+            by_user);
   }
 
   void Click() override {
     NotificationDisplayServiceFactory::GetForProfile(profile_)
         ->ProcessNotificationOperation(
             NotificationCommon::CLICK, notification_type_,
-            notification_.origin_url(), notification_.id(), base::nullopt,
-            base::nullopt, base::nullopt);
+            notification_.origin_url(), notification_.id(),
+            base::nullopt /* action_index */, base::nullopt /* reply */,
+            base::nullopt /* by_user */);
+  }
+
+  void ButtonClick(int action_index) override {
+    NotificationDisplayServiceFactory::GetForProfile(profile_)
+        ->ProcessNotificationOperation(
+            NotificationCommon::CLICK, notification_type_,
+            notification_.origin_url(), notification_.id(), action_index,
+            base::nullopt /* reply */, base::nullopt /* by_user */);
   }
 
-  void ButtonClick(int button_index) override {
+  void ButtonClickWithReply(int action_index,
+                            const base::string16& reply) override {
     NotificationDisplayServiceFactory::GetForProfile(profile_)
         ->ProcessNotificationOperation(
             NotificationCommon::CLICK, notification_type_,
-            notification_.origin_url(), notification_.id(), button_index,
-            base::nullopt, base::nullopt);
+            notification_.origin_url(), notification_.id(), action_index, reply,
+            base::nullopt /* by_user */);
   }
 
  protected:

chrome/browser/notifications/non_persistent_notification_handler.cc

@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "base/strings/nullable_string16.h"
+#include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "content/public/browser/notification_event_dispatcher.h"
 
@@ -53,6 +54,13 @@ void NonPersistentNotificationHandler::OnClick(
   std::move(completed_closure).Run();
 }
 
-void NonPersistentNotificationHandler::OpenSettings(Profile* profile) {
-  NotificationCommon::OpenNotificationSettings(profile);
+void NonPersistentNotificationHandler::DisableNotifications(
+    Profile* profile,
+    const GURL& origin) {
+  DesktopNotificationProfileUtil::DenyPermission(profile, origin);
+}
+
+void NonPersistentNotificationHandler::OpenSettings(Profile* profile,
+                                                    const GURL& origin) {
+  NotificationCommon::OpenNotificationSettings(profile, origin);
 }

chrome/browser/notifications/non_persistent_notification_handler.h

@@ -27,7 +27,8 @@ class NonPersistentNotificationHandler : public NotificationHandler {
                const base::Optional<int>& action_index,
                const base::Optional<base::string16>& reply,
                base::OnceClosure completed_closure) override;
-  void OpenSettings(Profile* profile) override;
+  void DisableNotifications(Profile* profile, const GURL& origin) override;
+  void OpenSettings(Profile* profile, const GURL& origin) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NonPersistentNotificationHandler);

chrome/browser/notifications/notification_common.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/notifications/notification_common.h"
 
+#include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
@@ -28,18 +29,17 @@ const PersistentNotificationMetadata* PersistentNotificationMetadata::From(
 }
 
 // static
-void NotificationCommon::OpenNotificationSettings(
-    content::BrowserContext* browser_context) {
-#if defined(OS_ANDROID)
-  // Android settings are opened directly from Java
-  NOTIMPLEMENTED();
-#elif defined(OS_CHROMEOS)
-  chrome::ShowContentSettingsExceptionsForProfile(
-      Profile::FromBrowserContext(browser_context),
-      CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+void NotificationCommon::OpenNotificationSettings(Profile* profile,
+                                                  const GURL& origin) {
+// TODO(peter): Use the |origin| to direct the user to a more appropriate
+// settings page to toggle permission.
+
+#if defined(OS_ANDROID) || defined(OS_CHROMEOS)
+  // Android settings are handled through Java. Chrome OS settings are handled
+  // through the tray's setting panel.
+  NOTREACHED();
 #else
-  chrome::ScopedTabbedBrowserDisplayer browser_displayer(
-      Profile::FromBrowserContext(browser_context));
+  chrome::ScopedTabbedBrowserDisplayer browser_displayer(profile);
   chrome::ShowContentSettingsExceptions(browser_displayer.browser(),
                                         CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
 #endif

chrome/browser/notifications/notification_common.h

@@ -9,10 +9,6 @@
 #include "chrome/browser/notifications/notification_handler.h"
 #include "url/gurl.h"
 
-namespace content {
-class BrowserContext;
-}  // namespace content
-
 class GURL;
 class Profile;
 
@@ -24,7 +20,8 @@ class NotificationCommon {
   enum Operation {
     CLICK = 0,
     CLOSE = 1,
-    SETTINGS = 2,
+    DISABLE_PERMISSION = 2,
+    SETTINGS = 3,
     OPERATION_MAX = SETTINGS
   };
 
@@ -37,10 +34,7 @@ class NotificationCommon {
   };
 
   // Open the Notification settings screen when clicking the right button.
-  // TODO(miguelg) have it take a Profile instead once NotificationObjectProxy
-  // is updated.
-  static void OpenNotificationSettings(
-      content::BrowserContext* browser_context);
+  static void OpenNotificationSettings(Profile* profile, const GURL& origin);
 };
 
 // Metadata for PERSISTENT notifications.

chrome/browser/notifications/notification_display_service.cc

@@ -104,8 +104,11 @@ void NotificationDisplayService::ProcessNotificationOperation(
       handler->OnClose(profile_, origin, notification_id, by_user.value(),
                        std::move(completed_closure));
       break;
+    case NotificationCommon::DISABLE_PERMISSION:
+      handler->DisableNotifications(profile_, origin);
+      break;
     case NotificationCommon::SETTINGS:
-      handler->OpenSettings(profile_);
+      handler->OpenSettings(profile_, origin);
       break;
   }
 }

chrome/browser/notifications/notification_handler.cc

@@ -28,7 +28,12 @@ void NotificationHandler::OnClick(Profile* profile,
   std::move(completed_closure).Run();
 }
 
-void NotificationHandler::OpenSettings(Profile* profile) {
+void NotificationHandler::DisableNotifications(Profile* profile,
+                                               const GURL& origin) {
+  NOTREACHED();
+}
+
+void NotificationHandler::OpenSettings(Profile* profile, const GURL& origin) {
   // Notification types that display a settings button must override this method
   // to handle user interaction with it.
   NOTREACHED();

chrome/browser/notifications/notification_handler.h

@@ -55,8 +55,11 @@ class NotificationHandler {
                        const base::Optional<base::string16>& reply,
                        base::OnceClosure completed_closure);
 
-  // Open notification settings.
-  virtual void OpenSettings(Profile* profile);
+  // Called when notifications of the given origin have to be disabled.
+  virtual void DisableNotifications(Profile* profile, const GURL& origin);
+
+  // Called when the settings page for the given origin has to be opened.
+  virtual void OpenSettings(Profile* profile, const GURL& origin);
 };
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_HANDLER_H_

chrome/browser/notifications/notification_interactive_uitest_support.cc

@@ -8,7 +8,6 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
-#include "chrome/browser/notifications/web_notification_delegate.h"
 #include "chrome/browser/permissions/permission_request_manager.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"

chrome/browser/notifications/persistent_notification_handler.cc

@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "base/logging.h"
+#include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/profiles/profile.h"
 
@@ -43,6 +44,12 @@ void PersistentNotificationHandler::OnClick(
       std::move(completed_closure));
 }
 
-void PersistentNotificationHandler::OpenSettings(Profile* profile) {
-  NotificationCommon::OpenNotificationSettings(profile);
+void PersistentNotificationHandler::DisableNotifications(Profile* profile,
+                                                         const GURL& origin) {
+  DesktopNotificationProfileUtil::DenyPermission(profile, origin);
+}
+
+void PersistentNotificationHandler::OpenSettings(Profile* profile,
+                                                 const GURL& origin) {
+  NotificationCommon::OpenNotificationSettings(profile, origin);
 }

chrome/browser/notifications/persistent_notification_handler.h

@@ -27,7 +27,8 @@ class PersistentNotificationHandler : public NotificationHandler {
                const base::Optional<int>& action_index,
                const base::Optional<base::string16>& reply,
                base::OnceClosure completed_closure) override;
-  void OpenSettings(Profile* profile) override;
+  void DisableNotifications(Profile* profile, const GURL& origin) override;
+  void OpenSettings(Profile* profile, const GURL& origin) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PersistentNotificationHandler);

chrome/browser/notifications/platform_notification_service_impl.cc

@@ -19,7 +19,6 @@
 #include "chrome/browser/notifications/metrics/notification_metrics_logger_factory.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
-#include "chrome/browser/notifications/web_notification_delegate.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker.h"
 #include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/permissions/permission_result.h"
@@ -358,9 +357,7 @@ void PlatformNotificationServiceImpl::DisplayNotification(
 
   message_center::Notification notification = CreateNotificationFromData(
       profile, origin, notification_id, notification_data,
-      notification_resources,
-      new WebNotificationDelegate(NotificationHandler::Type::WEB_NON_PERSISTENT,
-                                  profile, notification_id, origin));
+      notification_resources, nullptr /* delegate */);
 
   NotificationDisplayServiceFactory::GetForProfile(profile)->Display(
       NotificationHandler::Type::WEB_NON_PERSISTENT, notification);
@@ -386,9 +383,7 @@ void PlatformNotificationServiceImpl::DisplayPersistentNotification(
 
   message_center::Notification notification = CreateNotificationFromData(
       profile, origin, notification_id, notification_data,
-      notification_resources,
-      new WebNotificationDelegate(NotificationHandler::Type::WEB_PERSISTENT,
-                                  profile, notification_id, origin));
+      notification_resources, nullptr /* delegate */);
   auto metadata = std::make_unique<PersistentNotificationMetadata>();
   metadata->service_worker_scope = service_worker_scope;
 

chrome/browser/notifications/platform_notification_service_unittest.cc

@@ -18,7 +18,6 @@
 #include "chrome/browser/notifications/metrics/notification_metrics_logger_factory.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
-#include "chrome/browser/notifications/web_notification_delegate.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -340,8 +339,7 @@ TEST_F(PlatformNotificationServiceTest, CreateNotificationFromData) {
 
   Notification notification = service()->CreateNotificationFromData(
       profile_, origin, "id", notification_data, NotificationResources(),
-      new WebNotificationDelegate(NotificationHandler::Type::WEB_PERSISTENT,
-                                  profile_, "id", origin));
+      nullptr /* delegate */);
   EXPECT_TRUE(notification.context_message().empty());
 
   // Create a mocked extension.
@@ -363,9 +361,7 @@ TEST_F(PlatformNotificationServiceTest, CreateNotificationFromData) {
   notification = service()->CreateNotificationFromData(
       profile_,
       GURL("chrome-extension://honijodknafkokifofgiaalefdiedpko/main.html"),
-      "id", notification_data, NotificationResources(),
-      new WebNotificationDelegate(NotificationHandler::Type::EXTENSION,
-                                  profile_, "id", origin));
+      "id", notification_data, NotificationResources(), nullptr /* delegate */);
   EXPECT_EQ("NotificationTest",
             base::UTF16ToUTF8(notification.context_message()));
 }

chrome/browser/notifications/stub_notification_display_service.cc

@@ -116,7 +116,7 @@ void StubNotificationDisplayService::SimulateSettingsClick(
     iter->notification.delegate()->SettingsClick();
   } else {
     DCHECK(handler);
-    handler->OpenSettings(profile_);
+    handler->OpenSettings(profile_, iter->notification.origin_url());
   }
 }
 

chrome/browser/notifications/stub_notification_display_service.h

@@ -16,6 +16,10 @@
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "ui/message_center/notification.h"
 
+namespace content {
+class BrowserContext;
+}
+
 class Profile;
 
 // Implementation of the NotificationDisplayService interface that can be used

chrome/browser/notifications/web_notification_delegate.cc

-// Copyright 2016 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/web_notification_delegate.h"
-
-#include "base/feature_list.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/strings/nullable_string16.h"
-#include "chrome/browser/notifications/desktop_notification_profile_util.h"
-#include "chrome/browser/notifications/notification_common.h"
-#include "chrome/browser/notifications/notification_display_service.h"
-#include "chrome/browser/notifications/notification_display_service_factory.h"
-#include "chrome/browser/profiles/profile.h"
-
-WebNotificationDelegate::WebNotificationDelegate(
-    NotificationHandler::Type notification_type,
-    Profile* profile,
-    const std::string& notification_id,
-    const GURL& origin)
-    : notification_type_(notification_type),
-      profile_(profile),
-      notification_id_(notification_id),
-      origin_(origin) {}
-
-WebNotificationDelegate::~WebNotificationDelegate() {}
-
-void WebNotificationDelegate::SettingsClick() {
-#if defined(OS_CHROMEOS)
-  NOTREACHED();
-#else
-  NotificationCommon::OpenNotificationSettings(profile_);
-#endif
-}
-
-void WebNotificationDelegate::DisableNotification() {
-  DCHECK_NE(notification_type_, NotificationHandler::Type::EXTENSION);
-  DesktopNotificationProfileUtil::DenyPermission(profile_, origin_);
-}
-
-void WebNotificationDelegate::Close(bool by_user) {
-  auto* display_service =
-      NotificationDisplayServiceFactory::GetForProfile(profile_);
-  display_service->ProcessNotificationOperation(
-      NotificationCommon::CLOSE, notification_type_, origin(), notification_id_,
-      base::nullopt /* action_index */, base::nullopt /* reply */, by_user);
-}
-
-void WebNotificationDelegate::Click() {
-  auto* display_service =
-      NotificationDisplayServiceFactory::GetForProfile(profile_);
-  display_service->ProcessNotificationOperation(
-      NotificationCommon::CLICK, notification_type_, origin(), notification_id_,
-      base::nullopt /* action_index */, base::nullopt /* reply */,
-      base::nullopt /* by_user */);
-}
-
-void WebNotificationDelegate::ButtonClick(int action_index) {
-  DCHECK_GE(action_index, 0);
-  auto* display_service =
-      NotificationDisplayServiceFactory::GetForProfile(profile_);
-  display_service->ProcessNotificationOperation(
-      NotificationCommon::CLICK, notification_type_, origin(), notification_id_,
-      action_index, base::nullopt /* reply */, base::nullopt /* by_user */);
-}
-
-void WebNotificationDelegate::ButtonClickWithReply(
-    int action_index,
-    const base::string16& reply) {
-  auto* display_service =
-      NotificationDisplayServiceFactory::GetForProfile(profile_);
-  display_service->ProcessNotificationOperation(
-      NotificationCommon::CLICK, notification_type_, origin(), notification_id_,
-      action_index, reply, base::nullopt /* by_user */);
-}

chrome/browser/notifications/web_notification_delegate.h

-// Copyright 2016 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_WEB_NOTIFICATION_DELEGATE_H_
-#define CHROME_BROWSER_NOTIFICATIONS_WEB_NOTIFICATION_DELEGATE_H_
-
-#include <string>
-
-#include "base/feature_list.h"
-#include "base/macros.h"
-#include "chrome/browser/notifications/notification_common.h"
-#include "ui/message_center/notification_delegate.h"
-#include "url/gurl.h"
-
-class Profile;
-
-namespace features {
-
-extern const base::Feature kAllowFullscreenWebNotificationsFeature;
-
-} // namespace features
-
-// Delegate class for Web Notifications.
-class WebNotificationDelegate : public message_center::NotificationDelegate {
- public:
-  WebNotificationDelegate(NotificationHandler::Type notification_type,
-                          Profile* profile,
-                          const std::string& notification_id,
-                          const GURL& origin);
-
-  // NotificationDelegate implementation.
-  void SettingsClick() override;
-  void DisableNotification() override;
-  void Close(bool by_user) override;
-  void Click() override;
-  void ButtonClick(int action_index) override;
-  void ButtonClickWithReply(int action_index,
-                            const base::string16& reply) override;
-
- protected:
-  ~WebNotificationDelegate() override;
-  const GURL& origin() { return origin_; }
-
- private:
-  NotificationHandler::Type notification_type_;
-  Profile* profile_;
-  std::string notification_id_;
-  GURL origin_;
-
-  DISALLOW_COPY_AND_ASSIGN(WebNotificationDelegate);
-};
-
-#endif  // CHROME_BROWSER_NOTIFICATIONS_WEB_NOTIFICATION_DELEGATE_H_