Clean up the NotificationUIManagerAndroid.

chrome/browser/notifications/notification_ui_manager_android.h

Index: chrome/browser/notifications/notification_ui_manager_android.h
diff --git a/chrome/browser/notifications/notification_ui_manager_android.h b/chrome/browser/notifications/notification_ui_manager_android.h
index 2a9c44484d057455e50be05aac4ffe7f69529b8e..a1bc513e1700822b171df944bf349cfb10388240 100644
--- a/chrome/browser/notifications/notification_ui_manager_android.h
+++ b/chrome/browser/notifications/notification_ui_manager_android.h
@@ -7,16 +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.
+//
+// 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 {

[johnme, 2015/04/09 17:00:01]

With all the NOTIMPLEMENTEDs, the only methods of NotificationUIManager you
actually implement are Add and CancelById (and you misinterpret the id for
CancelById ;) ).

I suggested below that you add implementations for at least CancelAll* backed by
the notifications database. But alternatively, maybe you should create a new
PersistentNotificationUIManager class, and have this class only implement that,
while MessageCenterNotificationManager would implement both that and
NotificationUIManager?

[Peter Beverloo, 2015/04/09 17:34:42]

That could work out, but requires some additional thinking. I like the idea.

Filed https://crbug.com/475599 and added it to the spreadsheet.

  public:
   NotificationUIManagerAndroid();
@@ -36,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;
@@ -54,34 +61,14 @@ 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 a regenerated notification's origin and tag.
+  // Pair containing the origin and tag of a notification event received from
+  // Java. This information is required in order to support closing such
+  // notifications again.
   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_;