Clean up the NotificationUIManagerAndroid.

When this class was introduced, it served the needs for non-persistent
Web Notifications (and other kinds of notifications), with the knowledge
that it wouldn't be able to reliably provide many of the API methods
defined in the NotificationUIManager interface. That was OK.

Today, we only ship persistent Web Notifications, which can outlive the
browser process, and don't use the NotificationUIManagerAndroid for any
other purpose. However, the code to support other kinds of notifications
is still more or less in place, with no tests or users. It's broken.

Therefore we remove it. This CL also removes a test that doesn't make
sense anymore, since this stops us from double-closing notifications.

BUG=

Review URL: https://codereview.chromium.org/1072043003

Cr-Commit-Position: refs/heads/master@{#325273}

chrome/browser/notifications/notification_ui_manager_android.h

@@ -7,19 +7,23 @@
 
 #include <jni.h>
 #include <map>
-#include <set>
 #include <string>
 
 #include "base/android/scoped_java_ref.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 
-class ProfileNotification;
-
 // Implementation of the Notification UI Manager for Android, which defers to
 // the Android framework for displaying notifications.
 //
-// The Android notification manager only operates reliably on notifications
-// that were opened or interacted with since the last restart of Chrome.
+// Android does not support getting the notifications currently shown by an
+// app without very intrusive permissions, which means that it's not possible
+// to provide reliable implementations for methods which have to iterate over
+// the existing notifications. Because of this, these have not been implemented.
+//
+// The UI manager implementation *is* reliable for adding and canceling single
+// notifications based on their delegate id. Finally, events for persistent Web
+// Notifications will be forwarded directly to the associated event handlers,
+// as such notifications may outlive the browser process on Android.
 class NotificationUIManagerAndroid : public NotificationUIManager {
  public:
   NotificationUIManagerAndroid();
@@ -39,7 +43,7 @@ class NotificationUIManagerAndroid : public NotificationUIManager {
                             jstring java_origin,
                             jstring java_tag);
 
-  // NotificationUIManager implementation;
+  // NotificationUIManager implementation.
   void Add(const Notification& notification, Profile* profile) override;
   bool Update(const Notification& notification,
               Profile* profile) override;
@@ -57,37 +61,15 @@ class NotificationUIManagerAndroid : public NotificationUIManager {
   static bool RegisterNotificationUIManager(JNIEnv* env);
 
  private:
-  // Closes the Notification as displayed on the Android system.
-  void PlatformCloseNotification(const std::string& notification_id);
-
-  // Adds |profile_notification| to a private local map. The map takes ownership
-  // of the notification object.
-  void AddProfileNotification(ProfileNotification* profile_notification);
-
-  // Erases |profile_notification| from the private local map and deletes it.
-  // If |close| is true, also closes the notification.
-  void RemoveProfileNotification(ProfileNotification* profile_notification,
-                                 bool close);
-
-  // Returns the ProfileNotification for the |id|, or NULL if no such
-  // notification is found.
-  ProfileNotification* FindProfileNotification(const std::string& id) const;
-
-  // Returns the ProfileNotification for |profile| that has the same origin and
-  // tag as |notification|. Returns NULL if no valid match is found.
-  ProfileNotification* FindNotificationToReplace(
-      const Notification& notification, Profile* profile) const;
-
-  // Map from a notification id to the associated ProfileNotification*.
-  std::map<std::string, ProfileNotification*> profile_notifications_;
-
   // Pair containing the information necessary in order to enable closing
   // notifications that were not created by this instance of the manager: the
   // notification's origin and tag. This list may not contain the notifications
   // that have not been interacted with since the last restart of Chrome.
   using RegeneratedNotificationInfo = std::pair<std::string, std::string>;
 
-  // Map from persistent notification id to RegeneratedNotificationInfo.
+  // Mapping of a persistent notification id to renegerated notification info.
+  // TODO(peter): Remove this map once notification delegate ids for Web
+  // notifications are created by the content/ layer.
   std::map<int64_t, RegeneratedNotificationInfo>
       regenerated_notification_infos_;
 

chrome/browser/notifications/persistent_notification_delegate.cc

@@ -9,14 +9,6 @@
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "content/public/common/persistent_notification_status.h"
 
-namespace {
-
-// Persistent notifications fired through the delegate do not care about the
-// lifetime of the Service Worker responsible for executing the event.
-void OnEventDispatchComplete(content::PersistentNotificationStatus status) {}
-
-}  // namespace
-
 PersistentNotificationDelegate::PersistentNotificationDelegate(
     content::BrowserContext* browser_context,
     int64_t persistent_notification_id,
@@ -36,8 +28,7 @@ void PersistentNotificationDelegate::Click() {
   PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick(
       browser_context_,
       persistent_notification_id_,
-      origin_,
-      base::Bind(&OnEventDispatchComplete));
+      origin_);
 }
 
 std::string PersistentNotificationDelegate::id() const {

chrome/browser/notifications/platform_notification_service_impl.cc

@@ -45,6 +45,13 @@ using message_center::NotifierId;
 
 namespace {
 
+// Persistent notifications fired through the delegate do not care about the
+// lifetime of the Service Worker responsible for executing the event.
+void OnEventDispatchComplete(content::PersistentNotificationStatus status) {
+  // TODO(peter): Record UMA statistics about the result status of running
+  // events for persistent Web Notifications.
+}
+
 void CancelNotification(const std::string& id, ProfileID profile_id) {
   PlatformNotificationServiceImpl::GetInstance()
       ->GetNotificationUIManager()->CancelById(id, profile_id);
@@ -66,16 +73,14 @@ PlatformNotificationServiceImpl::~PlatformNotificationServiceImpl() {}
 void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
     content::BrowserContext* browser_context,
     int64_t persistent_notification_id,
-    const GURL& origin,
-    const base::Callback<void(content::PersistentNotificationStatus)>&
-        callback) const {
+    const GURL& origin) const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
             browser_context,
             persistent_notification_id,
             origin,
-            callback);
+            base::Bind(&OnEventDispatchComplete));
 }
 
 blink::WebNotificationPermission

chrome/browser/notifications/platform_notification_service_impl.h

@@ -38,9 +38,7 @@ class PlatformNotificationServiceImpl
   void OnPersistentNotificationClick(
       content::BrowserContext* browser_context,
       int64_t persistent_notification_id,
-      const GURL& origin,
-      const base::Callback<void(content::PersistentNotificationStatus)>&
-          callback) const;
+      const GURL& origin) const;
 
   // Returns the Notification UI Manager through which notifications can be
   // displayed to the user. Can be overridden for testing.