Introduce a new API to handle native notification clicks

chrome/browser/notifications/native_notification_display_service.h

Index: chrome/browser/notifications/native_notification_display_service.h
diff --git a/chrome/browser/notifications/native_notification_display_service.h b/chrome/browser/notifications/native_notification_display_service.h
index 37989383a1d846ec7f50254d7cc3ece8e25acc9b..4277e0a39145e10eb192b97f8803de2f304941e4 100644
--- a/chrome/browser/notifications/native_notification_display_service.h
+++ b/chrome/browser/notifications/native_notification_display_service.h
@@ -5,12 +5,17 @@
 #ifndef CHROME_BROWSER_NOTIFICATIONS_NATIVE_NOTIFICATION_DISPLAY_SERVICE_H_
 #define CHROME_BROWSER_NOTIFICATIONS_NATIVE_NOTIFICATION_DISPLAY_SERVICE_H_
 
+#include <map>
+#include <memory>
 #include <set>
 #include <string>
+
 #include "base/macros.h"
+#include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 
 class Notification;
+class NotificationHandler;
 class NotificationPlatformBridge;
 class Profile;
 
@@ -24,15 +29,34 @@ class NativeNotificationDisplayService : public NotificationDisplayService {
   ~NativeNotificationDisplayService() override;
 
   // NotificationDisplayService implementation.
-  void Display(const std::string& notification_id,
+  void Display(NotificationCommon::Type notification_type,
+               const std::string& notification_id,
                const Notification& notification) override;
-  void Close(const std::string& notification_id) override;
+  void Close(NotificationCommon::Type notification_type,
+             const std::string& notification_id) override;
   bool GetDisplayed(std::set<std::string>* notifications) const override;
+
+  // Used by the notification bridge to propagate back events (click, close...)
+  void ProcessNotificationOperation(NotificationCommon::Operation operation,
+                                    NotificationCommon::Type notification_type,
+                                    const std::string& origin,
+                                    const std::string& notification_id,
+                                    int action_index);
+
+  void AddNotificationHandler(NotificationCommon::Type notification_type,

[Peter Beverloo, 2016/07/05 14:25:34]

Please document public APIs.

[Miguel Garcia, 2016/07/05 17:12:52]

Done.

+                              std::unique_ptr<NotificationHandler> handler);
+  void RemoveNotificationHandler(NotificationCommon::Type notification_type);
+
   bool SupportsNotificationCenter() const override;

[Peter Beverloo, 2016/07/05 14:25:34]

Move this to line 38.

[Miguel Garcia, 2016/07/05 17:12:52]

Done.

 
  private:
+  NotificationHandler* GetNotificationHandler(
+      NotificationCommon::Type notification_type);

[Peter Beverloo, 2016/07/05 14:25:34]

nit: const?

[Miguel Garcia, 2016/07/05 17:12:52]

I know if I add it will compile but it feels like const should really mean that
you cannot change the state of the object and here you can (and we do) change
the state of the handler itself (for example when adding/removing delegates).

+
   Profile* profile_;
   NotificationPlatformBridge* notification_bridge_;
+  std::map<NotificationCommon::Type, std::unique_ptr<NotificationHandler>>

[Peter Beverloo, 2016/07/05 14:25:34]

nit: unordered_map

[Miguel Garcia, 2016/07/05 17:12:52]

C++ 11 does not easily allow unordered maps keyed by enums unfortunately.  There
are some work around to this but for a map of two, eventually three, elements is
probably not worth it.

Once we move to C++14 it should be possible.

+      notification_handlers_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeNotificationDisplayService);
 };