Initial client side implementation of the XPC service

chrome/browser/notifications/notification_platform_bridge_mac.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/chrome/browser/notifications/notification_platform_bridge_mac.mm
index 08fefce8bcc7e130114c3ca7f3e03986004b5f58..a1ce41b2846c018e31182d68784f655180b7e40e 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -6,8 +6,12 @@
 
 #include <utility>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/mac/bundle_locations.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
+#include "base/mac/scoped_nsobject.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ -20,10 +24,14 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_builder_mac.h"
+#import "chrome/browser/ui/cocoa/notifications/notification_delivery.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_constants_mac.h"
 #import "chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h"
+#include "chrome/common/features.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/url_formatter/elide_url.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationConstants.h"
+#include "ui/base/l10n/l10n_util_mac.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -71,6 +79,23 @@ void ProfileLoadedCallback(NotificationCommon::Operation operation,
                                      notification_id, action_index);
 }
 
+// Loads the profile and process the Notification response
+void DoProcessNotificationResponse(NotificationCommon::Operation operation,
+                                   NotificationCommon::Type type,
+                                   const std::string& profile_id,
+                                   bool incognito,
+                                   const std::string& origin,
+                                   const std::string& notification_id,
+                                   int32_t button_index) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  ProfileManager* profileManager = g_browser_process->profile_manager();
+  DCHECK(profileManager);
+
+  profileManager->LoadProfile(
+      profile_id, incognito, base::Bind(&ProfileLoadedCallback, operation, type,
+                                        origin, notification_id, button_index));
+}
+
 }  // namespace
 
 // static
@@ -86,19 +111,30 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
 }
 @end
 
+// Interface to communicate with the Alert XPC service.
+@interface NotificationRemoteDispatcher : NSObject
+
+- (instancetype)init;

[Robert Sesek, 2016/09/12 15:33:47]

You don't have to re-declare init if there's no arguments.

[Miguel Garcia, 2016/09/12 16:57:59]

Done.

+- (void)dispatchNotification:(NSDictionary*)data;
+
+@end
+
 // /////////////////////////////////////////////////////////////////////////////
 
 NotificationPlatformBridgeMac::NotificationPlatformBridgeMac(
     NSUserNotificationCenter* notification_center)
     : delegate_([NotificationCenterDelegate alloc]),
-      notification_center_(notification_center) {
+      notification_center_(notification_center),
+      notification_remote_dispatcher_(
+          [[NotificationRemoteDispatcher alloc] init]) {
   [notification_center_ setDelegate:delegate_.get()];
 }
 
 NotificationPlatformBridgeMac::~NotificationPlatformBridgeMac() {
   [notification_center_ setDelegate:nil];
 
-  // TODO(miguelg) lift this restriction if possible.
+  // TODO(miguelg) remove only alerts shown by the XPC service.
+  // TODO(miguelg) do not remove banners if possible.
   [notification_center_ removeAllDeliveredNotifications];
 }
 
@@ -139,6 +175,7 @@ void NotificationPlatformBridgeMac::Display(
   // Tag
   if (!notification.tag().empty()) {
     [builder setTag:base::SysUTF8ToNSString(notification.tag())];
+
     // If renotify is needed, delete the notification with the same tag
     // from the notification center before displaying this one.
     // TODO(miguelg): This will need to work for alerts as well via XPC
@@ -166,8 +203,18 @@ void NotificationPlatformBridgeMac::Display(
   [builder setIncognito:incognito];
   [builder setNotificationType:[NSNumber numberWithInteger:notification_type]];
 
+#if BUILDFLAG(ENABLE_XPC_NOTIFICATIONS)
+  if (notification.never_timeout()) {

[Robert Sesek, 2016/09/12 15:33:47]

This could use some commentary.

[Miguel Garcia, 2016/09/12 16:57:59]

Done.

+    NSDictionary* dict = [builder buildDictionary];
+    [notification_remote_dispatcher_ dispatchNotification:dict];
+  } else {
+    NSUserNotification* toast = [builder buildUserNotification];
+    [notification_center_ deliverNotification:toast];
+  }
+#else
   NSUserNotification* toast = [builder buildUserNotification];
   [notification_center_ deliverNotification:toast];
+#endif  // ENABLE_XPC_NOTIFICATIONS
 }
 
 void NotificationPlatformBridgeMac::Close(const std::string& profile_id,
@@ -214,6 +261,39 @@ bool NotificationPlatformBridgeMac::SupportsNotificationCenter() const {
 }
 
 // static
+void NotificationPlatformBridgeMac::ProcessNotificationResponse(
+    NSDictionary* response) {
+  if (!NotificationPlatformBridgeMac::VerifyNotificationData(response))
+    return;
+
+  NSNumber* buttonIndex =

[Robert Sesek, 2016/09/12 15:33:47]

under_score all these names

[Miguel Garcia, 2016/09/12 16:57:59]

Done.

+      [response objectForKey:notification_constants::kNotificationButtonIndex];
+  NSNumber* operation =
+      [response objectForKey:notification_constants::kNotificationOperation];
+
+  std::string notificationOrigin = base::SysNSStringToUTF8(
+      [response objectForKey:notification_constants::kNotificationOrigin]);
+  std::string notificationId = base::SysNSStringToUTF8(
+      [response objectForKey:notification_constants::kNotificationId]);
+  std::string profileId = base::SysNSStringToUTF8(
+      [response objectForKey:notification_constants::kNotificationProfileId]);
+  NSNumber* isIncognito =
+      [response objectForKey:notification_constants::kNotificationIncognito];
+  NSNumber* notificationType =
+      [response objectForKey:notification_constants::kNotificationType];
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(DoProcessNotificationResponse,
+                 static_cast<NotificationCommon::Operation>(
+                     operation.unsignedIntValue),
+                 static_cast<NotificationCommon::Type>(
+                     notificationType.unsignedIntValue),
+                 profileId, [isIncognito boolValue], notificationOrigin,
+                 notificationId, buttonIndex.intValue));
+}
+
+// static
 bool NotificationPlatformBridgeMac::VerifyNotificationData(
     NSDictionary* response) {
   if (![response
@@ -282,44 +362,13 @@ bool NotificationPlatformBridgeMac::VerifyNotificationData(
 }
 
 // /////////////////////////////////////////////////////////////////////////////
-
 @implementation NotificationCenterDelegate
 - (void)userNotificationCenter:(NSUserNotificationCenter*)center
        didActivateNotification:(NSUserNotification*)notification {
-  NSDictionary* response =
+  NSDictionary* notification_response =

[Robert Sesek, 2016/09/12 15:33:47]

but camelCase here ;-)

[Miguel Garcia, 2016/09/12 16:57:59]

Done.

       [NotificationResponseBuilder buildDictionary:notification];
-  if (!NotificationPlatformBridgeMac::VerifyNotificationData(response))
-    return;
-
-  NSNumber* buttonIndex =
-      [response objectForKey:notification_constants::kNotificationButtonIndex];
-  NSNumber* operation =
-      [response objectForKey:notification_constants::kNotificationOperation];
-
-  std::string notificationOrigin = base::SysNSStringToUTF8(
-      [response objectForKey:notification_constants::kNotificationOrigin]);
-  NSString* notificationId =
-      [response objectForKey:notification_constants::kNotificationId];
-  std::string persistentNotificationId =
-      base::SysNSStringToUTF8(notificationId);
-  std::string profileId = base::SysNSStringToUTF8(
-      [response objectForKey:notification_constants::kNotificationProfileId]);
-  NSNumber* isIncognito =
-      [response objectForKey:notification_constants::kNotificationIncognito];
-  NSNumber* notificationType =
-      [response objectForKey:notification_constants::kNotificationType];
-
-  ProfileManager* profileManager = g_browser_process->profile_manager();
-  DCHECK(profileManager);
-
-  profileManager->LoadProfile(
-      profileId, [isIncognito boolValue],
-      base::Bind(
-          &ProfileLoadedCallback, static_cast<NotificationCommon::Operation>(
-                                      operation.unsignedIntValue),
-          static_cast<NotificationCommon::Type>(
-              notificationType.unsignedIntValue),
-          notificationOrigin, persistentNotificationId, buttonIndex.intValue));
+  NotificationPlatformBridgeMac::ProcessNotificationResponse(
+      notification_response);
 }
 
 - (BOOL)userNotificationCenter:(NSUserNotificationCenter*)center
@@ -329,3 +378,54 @@ bool NotificationPlatformBridgeMac::VerifyNotificationData(
 }
 
 @end
+
+@implementation NotificationRemoteDispatcher {
+  // The connection to the XPC server in charge of delivering alerts.
+  base::scoped_nsobject<NSXPCConnection> xpcConnection_;
+}
+
+- (instancetype)init {
+  if ((self = [super init])) {
+#if BUILDFLAG(ENABLE_XPC_NOTIFICATIONS)

[Robert Sesek, 2016/09/12 15:33:47]

Why not move this #if to be around the instantiation instead? That way this code
will get compile coverage.

[Miguel Garcia, 2016/09/12 16:57:59]

I shied away from that because the instantiation was in the middle of the
argument list in the constructor of the bridge but ok :)

+    xpcConnection_.reset([[NSXPCConnection alloc]
+        initWithServiceName:
+            [NSString
+                stringWithFormat:notification_constants::kAlertXPCServiceName,
+                                 [base::mac::OuterBundle() bundleIdentifier]]]);
+    xpcConnection_.get().remoteObjectInterface =
+        [NSXPCInterface interfaceWithProtocol:@protocol(NotificationDelivery)];
+
+    xpcConnection_.get().interruptionHandler = ^{
+      LOG(WARNING) << "connection interrupted: interruptionHandler: ";
+      // TODO(miguelg): perhaps add some UMA here.
+      // We will be getting this handler both when the XPC server crashes or
+      // when it decides to close the connection.
+    };
+    xpcConnection_.get().invalidationHandler = ^{
+      LOG(WARNING) << "connection invalidationHandler received";
+      // This means that the connection should be recreated if it needs
+      // to be used again. It should not really happen.
+      DCHECK(false) << "XPC Connection invalidated";
+    };
+
+    xpcConnection_.get().exportedInterface =
+        [NSXPCInterface interfaceWithProtocol:@protocol(NotificationReply)];
+    xpcConnection_.get().exportedObject = self;
+    [xpcConnection_ resume];
+#endif  // ENABLE_XPC_NOTIFICATIONS
+  }
+
+  return self;
+}
+
+- (void)dispatchNotification:(NSDictionary*)data {
+  [[xpcConnection_ remoteObjectProxy] deliverNotification:data];
+}
+
+// NotificationReply implementation
+- (void)notificationClick:(NSDictionary*)notificationResponseData {
+  NotificationPlatformBridgeMac::ProcessNotificationResponse(
+      notificationResponseData);
+}
+
+@end