Abstract notification clicks in its own dictionary

chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.mm

+// 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.
+
+#import "chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h"
+
+#import "chrome/browser/ui/cocoa/notifications/notification_constants_mac.h"
+#include "base/logging.h"
+
+namespace {
+
+// Make sure this Obj-C enum is kept in sync with the
+// PlatformNotificationServiceImpl NotificationOperation enum.
+// The latter cannot be reused because the XPC service is not aware of
+// PlatformNotificationCenter.
+typedef enum notificationOperations {
+  NOTIFICATION_CLICK,
+  NOTIFICATION_CLOSE,
+  NOTIFICATION_SETTINGS
+} NotificationOperation;
+}

[Peter Beverloo, 2016/06/16 17:02:43]

nits: types start with a capital, blank line above this one, and add a "//
namespace" comment

That also is a really old way to define an enum :-). We can't use an enum class
because you need the integral value, but you could write it like:

enum NotificationOperation {
  NOTIFICATION_CLICK = 0,
  NOTIFICATION_CLOSE = 1,
  NOTIFICATION_SETTINGS = 2
}

[Miguel Garcia, 2016/06/17 17:53:08]

Yeah I looked it up to see how people defined enums in Obj-C and this what it
came out (presumably because most of the code out there is Obj-C not Obj-C++).
Looking in chrome though it seems regular C++ enums are fine so changing this.

On 2016/06/16 17:02:43, Peter Beverloo - OOO June 8-16 wrote:

+
+@implementation NotificationResponseBuilder
+
++ (NSDictionary*)buildDictionary:(NSUserNotification*)notification {
+  NSString* origin =
+      [[notification userInfo]
+          objectForKey:notification_constants::kNotificationOrigin]
+          ? [[notification userInfo]
+                objectForKey:notification_constants::kNotificationOrigin]
+          : @"";
+  DCHECK([[notification userInfo]
+      objectForKey:notification_constants::kNotificationId]);
+  NSString* notificationId = [[notification userInfo]
+      objectForKey:notification_constants::kNotificationId];
+
+  DCHECK([[notification userInfo]
+      objectForKey:notification_constants::kNotificationProfileId]);
+  NSString* profileId = [[notification userInfo]
+      objectForKey:notification_constants::kNotificationProfileId];
+
+  DCHECK([[notification userInfo]
+      objectForKey:notification_constants::kNotificationIncognito]);
+  NSNumber* incognito = [[notification userInfo]
+      objectForKey:notification_constants::kNotificationIncognito];
+
+  // Initialize operation and button index for the case where the
+  // notification itself was clicked.
+  NotificationOperation operation = NOTIFICATION_CLICK;
+  int buttonIndex = -1;
+
+  // Determine whether the user clicked on a button, and if they did, whether it
+  // was a developer-provided button or the mandatory Settings button.
+  if (notification.activationType ==
+      NSUserNotificationActivationTypeActionButtonClicked) {
+    NSArray* alternateButtons = @[];
+    if ([notification
+            respondsToSelector:@selector(_alternateActionButtonTitles)]) {
+      alternateButtons =
+          [notification valueForKey:@"_alternateActionButtonTitles"];
+    }
+
+    bool multipleButtons = (alternateButtons.count > 0);
+
+    // No developer actions, just the settings button.
+    if (!multipleButtons) {
+      operation = NOTIFICATION_SETTINGS;
+      buttonIndex = -1;
+    } else {
+      // 0 based array containing.
+      // Button 1
+      // Button 2 (optional)
+      // Settings
+      NSNumber* actionIndex =
+          [notification valueForKey:@"_alternateActionIndex"];
+      operation = (actionIndex.unsignedLongValue == alternateButtons.count - 1)
+                      ? NOTIFICATION_SETTINGS
+                      : NOTIFICATION_CLICK;
+      buttonIndex =
+          (actionIndex.unsignedLongValue == alternateButtons.count - 1)
+              ? -1
+              : actionIndex.intValue;
+    }
+  }
+
+  return @{
+    notification_constants::kNotificationOrigin : origin,
+    notification_constants::kNotificationId : notificationId,
+    notification_constants::kNotificationProfileId : profileId,
+    notification_constants::kNotificationIncognito : incognito,
+    notification_constants::
+    kNotificationOperation : [NSNumber numberWithInt:operation],

[Peter Beverloo, 2016/06/16 17:02:43]

nit: could we wrap after the colon, avoiding breaking in the middle of the
constant's name?

[Miguel Garcia, 2016/06/17 17:53:08]

Done.

+    notification_constants::
+    kNotificationButtonIndex : [NSNumber numberWithInt:buttonIndex],
+  };
+}
+
+@end