Add support for native extension notifications

Add the ability to:
- Remove attribution
- Remove the settings button

BUG=571056

Review-Url: https://codereview.chromium.org/2799343003
Cr-Commit-Position: refs/heads/master@{#462906}

chrome/browser/notifications/notification_common.h

@@ -24,7 +24,8 @@ class NotificationCommon {
   enum Type {
     PERSISTENT = 0,
     NON_PERSISTENT = 1,
-    TYPE_MAX = NON_PERSISTENT
+    EXTENSION = 2,
+    TYPE_MAX = EXTENSION
   };
 
   // Open the Notification settings screen when clicking the right button.

chrome/browser/notifications/notification_platform_bridge_mac.mm

@@ -172,8 +172,12 @@ void NotificationPlatformBridgeMac::Display(
   [builder setTitle:base::SysUTF16ToNSString(notification.title())];
   [builder setContextMessage:base::SysUTF16ToNSString(notification.message())];
 
+  bool requires_attribution =
+      notification.context_message().empty() &&
+      notification_type != NotificationCommon::EXTENSION;
+
   base::string16 subtitle =
-      notification.context_message().empty()
+      requires_attribution
           ? url_formatter::FormatOriginForSecurityDisplay(
                 url::Origin(notification.origin_url()),
                 url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS)
@@ -184,6 +188,8 @@ void NotificationPlatformBridgeMac::Display(
     [builder setIcon:notification.icon().ToNSImage()];
   }
 
+  [builder setShowSettingsButton:(notification_type !=
+                                  NotificationCommon::EXTENSION)];
   std::vector<message_center::ButtonInfo> buttons = notification.buttons();
   if (!buttons.empty()) {
     DCHECK_LE(buttons.size(), blink::kWebNotificationMaxActions);

chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm

@@ -54,6 +54,7 @@ class NotificationPlatformBridgeMacTest : public testing::Test {
     [builder setProfileId:@"profile_id"];
     [builder setIncognito:false];
     [builder setNotificationType:@(NotificationCommon::PERSISTENT)];
+    [builder setShowSettingsButton:true];
 
     return [builder buildUserNotification];
   }

chrome/browser/ui/cocoa/notifications/notification_builder_mac.h

@@ -59,6 +59,7 @@
 - (void)setProfileId:(NSString*)profileId;
 - (void)setIncognito:(BOOL)incognito;
 - (void)setNotificationType:(NSNumber*)notificationType;
+- (void)setShowSettingsButton:(BOOL)showSettingsButton;
 
 // Returns a notification ready to be displayed out of the provided
 // |notificationData|.

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

@@ -26,6 +26,7 @@ NSString* const kNotificationTag = @"tag";
 NSString* const kNotificationCloseButtonTag = @"closeButton";
 NSString* const kNotificationOptionsButtonTag = @"optionsButton";
 NSString* const kNotificationSettingsButtonTag = @"settingsButton";
+
 }  // namespace
 
 @implementation NotificationBuilder {
@@ -122,6 +123,12 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
                         forKey:notification_constants::kNotificationType];
 }
 
+- (void)setShowSettingsButton:(BOOL)showSettingsButton {
+  [notificationData_
+      setObject:[NSNumber numberWithBool:showSettingsButton]
+         forKey:notification_constants::kNotificationHasSettingsButton];
+}
+
 - (NSUserNotification*)buildUserNotification {
   base::scoped_nsobject<NSUserNotification> toast(
       [[NSUserNotification alloc] init]);
@@ -142,11 +149,23 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
     }
   }
 
+  // Type (needed to define the buttons)
+  NSNumber* type = [notificationData_
+      objectForKey:notification_constants::kNotificationType];
+
+  // Extensions don't have a settings button.
+  NSNumber* showSettingsButton = [notificationData_
+      objectForKey:notification_constants::kNotificationHasSettingsButton];
+
   // Buttons
   if ([toast respondsToSelector:@selector(_showsButtons)]) {
     DCHECK([notificationData_ objectForKey:kNotificationCloseButtonTag]);
     DCHECK([notificationData_ objectForKey:kNotificationSettingsButtonTag]);
     DCHECK([notificationData_ objectForKey:kNotificationOptionsButtonTag]);
+    DCHECK([notificationData_
+        objectForKey:notification_constants::kNotificationHasSettingsButton]);
+
+    BOOL settingsButton = [showSettingsButton boolValue];
 
     [toast setValue:@YES forKey:@"_showsButtons"];
     // A default close button label is provided by the platform but we
@@ -158,15 +177,19 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
     // Display the Settings button as the action button if there are either no
     // developer-provided action buttons, or the alternate action menu is not
     // available on this Mac version. This avoids needlessly showing the menu.
-    // TODO(miguelg): Extensions should not have a settings button.
     if (![notificationData_ objectForKey:kNotificationButtonOne] ||
         ![toast respondsToSelector:@selector(_alwaysShowAlternateActionMenu)]) {
-      [toast
-          setActionButtonTitle:
-              [notificationData_ objectForKey:kNotificationSettingsButtonTag]];
+      if (settingsButton) {
+        [toast setActionButtonTitle:
+                   [notificationData_
+                       objectForKey:kNotificationSettingsButtonTag]];
+      } else {
+        [toast setHasActionButton:NO];
+      }
+
     } else {
       // Otherwise show the alternate menu, then show the developer actions and
-      // finally the settings one.
+      // finally the settings one if needed.
       DCHECK(
           [toast respondsToSelector:@selector(_alwaysShowAlternateActionMenu)]);
       DCHECK(
@@ -183,8 +206,11 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
         [buttons
             addObject:[notificationData_ objectForKey:kNotificationButtonTwo]];
       }
-      [buttons addObject:[notificationData_
-                             objectForKey:kNotificationSettingsButtonTag]];
+      if (settingsButton) {
+        [buttons addObject:[notificationData_
+                               objectForKey:kNotificationSettingsButtonTag]];
+      }
+
       [toast setValue:buttons forKey:@"_alternateActionButtonTitles"];
     }
   }
@@ -216,8 +242,6 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
       objectForKey:notification_constants::kNotificationIncognito]);
   NSNumber* incognito = [notificationData_
       objectForKey:notification_constants::kNotificationIncognito];
-  NSNumber* type = [notificationData_
-      objectForKey:notification_constants::kNotificationType];
 
   toast.get().userInfo = @{
     notification_constants::kNotificationOrigin : origin,
@@ -225,6 +249,7 @@ NSString* const kNotificationSettingsButtonTag = @"settingsButton";
     notification_constants::kNotificationProfileId : profileId,
     notification_constants::kNotificationIncognito : incognito,
     notification_constants::kNotificationType : type,
+    notification_constants::kNotificationHasSettingsButton : showSettingsButton,
   };
 
   return toast.autorelease();

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

@@ -27,6 +27,7 @@ TEST(NotificationBuilderMacTest, TestNotificationNoButtons) {
   [builder setIncognito:false];
   [builder setNotificationType:
                [NSNumber numberWithInt:NotificationCommon::NON_PERSISTENT]];
+  [builder setShowSettingsButton:true];
 
   NSUserNotification* notification = [builder buildUserNotification];
   EXPECT_EQ("Title", base::SysNSStringToUTF8([notification title]));
@@ -57,6 +58,7 @@ TEST(NotificationBuilderMacTest, TestNotificationOneButton) {
   [builder
       setNotificationType:[NSNumber
                               numberWithInt:NotificationCommon::PERSISTENT]];
+  [builder setShowSettingsButton:true];
 
   NSUserNotification* notification = [builder buildUserNotification];
 
@@ -93,6 +95,7 @@ TEST(NotificationBuilderMacTest, TestNotificationTwoButtons) {
   [builder
       setNotificationType:[NSNumber
                               numberWithInt:NotificationCommon::PERSISTENT]];
+  [builder setShowSettingsButton:true];
 
   NSUserNotification* notification = [builder buildUserNotification];
 
@@ -115,6 +118,55 @@ TEST(NotificationBuilderMacTest, TestNotificationTwoButtons) {
   EXPECT_EQ("Settings", base::SysNSStringToUTF8([buttons objectAtIndex:2]));
 }
 
+TEST(NotificationBuilderMacTest, TestNotificationExtensionNoButtons) {
+  base::scoped_nsobject<NotificationBuilder> builder(
+      [[NotificationBuilder alloc] initWithCloseLabel:@"Close"
+                                         optionsLabel:@"Options"
+                                        settingsLabel:@"Settings"]);
+  [builder setTitle:@"Title"];
+  [builder setSubTitle:@"https://www.miguel.com"];
+  [builder setContextMessage:@"SubTitle"];
+  [builder setNotificationId:@"notificationId"];
+  [builder setProfileId:@"profileId"];
+  [builder setIncognito:false];
+  [builder
+      setNotificationType:[NSNumber
+                              numberWithInt:NotificationCommon::EXTENSION]];
+  [builder setShowSettingsButton:false];
+
+  NSUserNotification* notification = [builder buildUserNotification];
+
+  EXPECT_FALSE(notification.hasActionButton);
+  EXPECT_EQ("Close", base::SysNSStringToUTF8([notification otherButtonTitle]));
+}
+
+TEST(NotificationBuilderMacTest, TestNotificationExtensionButtons) {
+  base::scoped_nsobject<NotificationBuilder> builder(
+      [[NotificationBuilder alloc] initWithCloseLabel:@"Close"
+                                         optionsLabel:@"Options"
+                                        settingsLabel:@"Settings"]);
+  [builder setTitle:@"Title"];
+  [builder setSubTitle:@"https://www.miguel.com"];
+  [builder setContextMessage:@"SubTitle"];
+  [builder setButtons:@"Button1" secondaryButton:@"Button2"];
+  [builder setNotificationId:@"notificationId"];
+  [builder setProfileId:@"profileId"];
+  [builder setIncognito:false];
+  [builder
+      setNotificationType:[NSNumber
+                              numberWithInt:NotificationCommon::EXTENSION]];
+  [builder setShowSettingsButton:false];
+
+  NSUserNotification* notification = [builder buildUserNotification];
+
+  NSArray* buttons = [notification valueForKey:@"_alternateActionButtonTitles"];
+
+  // No settings button
+  ASSERT_EQ(2u, buttons.count);
+  EXPECT_EQ("Button1", base::SysNSStringToUTF8([buttons objectAtIndex:0]));
+  EXPECT_EQ("Button2", base::SysNSStringToUTF8([buttons objectAtIndex:1]));
+}
+
 TEST(NotificationBuilderMacTest, TestUserInfo) {
   base::scoped_nsobject<NotificationBuilder> builder(
       [[NotificationBuilder alloc] initWithCloseLabel:@"Close"
@@ -128,6 +180,7 @@ TEST(NotificationBuilderMacTest, TestUserInfo) {
   [builder
       setNotificationType:[NSNumber
                               numberWithInt:NotificationCommon::PERSISTENT]];
+  [builder setShowSettingsButton:true];
 
   NSUserNotification* notification = [builder buildUserNotification];
   EXPECT_EQ("Title", base::SysNSStringToUTF8([notification title]));
@@ -163,6 +216,7 @@ TEST(NotificationBuilderMacTest, TestBuildDictionary) {
     [sourceBuilder
         setNotificationType:
             [NSNumber numberWithInt:NotificationCommon::NON_PERSISTENT]];
+    [sourceBuilder setShowSettingsButton:true];
 
     notificationData = [sourceBuilder buildDictionary];
   }

chrome/browser/ui/cocoa/notifications/notification_constants_mac.h

@@ -16,6 +16,7 @@ extern NSString* const kNotificationIncognito;
 extern NSString* const kNotificationType;
 extern NSString* const kNotificationOperation;
 extern NSString* const kNotificationButtonIndex;
+extern NSString* const kNotificationHasSettingsButton;
 
 extern NSString* const kAlertXPCServiceName;
 

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

@@ -12,6 +12,8 @@ NSString* const kNotificationId = @"notificationId";
 NSString* const kNotificationProfileId = @"notificationProfileId";
 NSString* const kNotificationIncognito = @"notificationIncognito";
 NSString* const kNotificationType = @"notificationType";
+NSString* const kNotificationHasSettingsButton =
+    @"notificationHasSettingsButton";
 
 // Only applicable to the NotificationResponseBuilder
 NSString* const kNotificationOperation = @"notificationOperation";

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

@@ -45,6 +45,8 @@ enum NotificationOperation {
       objectForKey:notification_constants::kNotificationIncognito];
   NSNumber* notificationType = [[notification userInfo]
       objectForKey:notification_constants::kNotificationType];
+  NSNumber* hasSettingsButton = [[notification userInfo]
+      objectForKey:notification_constants::kNotificationHasSettingsButton];
 
   // Closed notifications are not activated.
   NotificationOperation operation =
@@ -54,7 +56,7 @@ enum NotificationOperation {
   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.
+  // was a developer-provided button or the  Settings button.
   if (notification.activationType ==
       NSUserNotificationActivationTypeActionButtonClicked) {
     NSArray* alternateButtons = @[];
@@ -64,26 +66,29 @@ enum NotificationOperation {
           [notification valueForKey:@"_alternateActionButtonTitles"];
     }
 
-    bool multipleButtons = (alternateButtons.count > 0);
+    BOOL settingsButtonRequired = [hasSettingsButton boolValue];
+    BOOL multipleButtons = (alternateButtons.count > 0);
 
     // No developer actions, just the settings button.
     if (!multipleButtons) {
+      DCHECK(settingsButtonRequired);
       operation = NOTIFICATION_SETTINGS;
       buttonIndex = -1;
     } else {
       // 0 based array containing.
       // Button 1
       // Button 2 (optional)
-      // Settings
+      // Settings (if required)
       NSNumber* actionIndex =
           [notification valueForKey:@"_alternateActionIndex"];
-      operation = (actionIndex.unsignedLongValue == alternateButtons.count - 1)
+      operation = settingsButtonRequired && (actionIndex.unsignedLongValue ==
+                                             alternateButtons.count - 1)
                       ? NOTIFICATION_SETTINGS
                       : NOTIFICATION_CLICK;
-      buttonIndex =
-          (actionIndex.unsignedLongValue == alternateButtons.count - 1)
-              ? -1
-              : actionIndex.intValue;
+      buttonIndex = settingsButtonRequired && (actionIndex.unsignedLongValue ==
+                                               alternateButtons.count - 1)
+                        ? -1
+                        : actionIndex.intValue;
     }
   }
 

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

@@ -13,7 +13,8 @@
 
 class NotificationResponseBuilderMacTest : public testing::Test {
  protected:
-  base::scoped_nsobject<NotificationBuilder> NewTestBuilder() {
+  base::scoped_nsobject<NotificationBuilder> NewTestBuilder(
+      NotificationCommon::Type type) {
     base::scoped_nsobject<NotificationBuilder> builder(
         [[NotificationBuilder alloc] initWithCloseLabel:@"Close"
                                            optionsLabel:@"Options"
@@ -26,13 +27,15 @@ class NotificationResponseBuilderMacTest : public testing::Test {
     [builder setNotificationId:@"notificationId"];
     [builder setProfileId:@"profileId"];
     [builder setIncognito:false];
-    [builder setNotificationType:@(NotificationCommon::PERSISTENT)];
+    [builder setNotificationType:@(type)];
+    [builder setShowSettingsButton:(type != NotificationCommon::EXTENSION)];
     return builder;
   }
 };
 
 TEST_F(NotificationResponseBuilderMacTest, TestNotificationClick) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   NSUserNotification* notification = [builder buildUserNotification];
   // This will be set by the notification center to indicate the notification
   // was clicked.
@@ -46,12 +49,14 @@ TEST_F(NotificationResponseBuilderMacTest, TestNotificationClick) {
       [response objectForKey:notification_constants::kNotificationOperation];
   NSNumber* buttonIndex =
       [response objectForKey:notification_constants::kNotificationButtonIndex];
+
   EXPECT_EQ(0 /* NOTIFICATION_CLICK */, operation.intValue);
   EXPECT_EQ(-1, buttonIndex.intValue);
 }
 
 TEST_F(NotificationResponseBuilderMacTest, TestNotificationSettingsClick) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   NSUserNotification* notification = [builder buildUserNotification];
 
   // This will be set by the notification center to indicate the only available
@@ -65,12 +70,14 @@ TEST_F(NotificationResponseBuilderMacTest, TestNotificationSettingsClick) {
       [response objectForKey:notification_constants::kNotificationOperation];
   NSNumber* buttonIndex =
       [response objectForKey:notification_constants::kNotificationButtonIndex];
+
   EXPECT_EQ(2 /* NOTIFICATION_SETTINGS */, operation.intValue);
   EXPECT_EQ(-1, buttonIndex.intValue);
 }
 
 TEST_F(NotificationResponseBuilderMacTest, TestNotificationOneActionClick) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   [builder setButtons:@"Button1" secondaryButton:@""];
 
   NSUserNotification* notification = [builder buildUserNotification];
@@ -93,7 +100,8 @@ TEST_F(NotificationResponseBuilderMacTest, TestNotificationOneActionClick) {
 }
 
 TEST_F(NotificationResponseBuilderMacTest, TestNotificationTwoActionClick) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   [builder setButtons:@"Button1" secondaryButton:@"Button2"];
 
   NSUserNotification* notification = [builder buildUserNotification];
@@ -118,7 +126,8 @@ TEST_F(NotificationResponseBuilderMacTest, TestNotificationTwoActionClick) {
 
 TEST_F(NotificationResponseBuilderMacTest,
        TestNotificationTwoActionSettingsClick) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   [builder setButtons:@"Button1" secondaryButton:@"Button2"];
   NSUserNotification* notification = [builder buildUserNotification];
 
@@ -144,7 +153,8 @@ TEST_F(NotificationResponseBuilderMacTest,
 }
 
 TEST_F(NotificationResponseBuilderMacTest, TestNotificationClose) {
-  base::scoped_nsobject<NotificationBuilder> builder = NewTestBuilder();
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::PERSISTENT);
   NSUserNotification* notification = [builder buildUserNotification];
 
   // None is what the NSUserNotification center emits when closing since it
@@ -162,3 +172,29 @@ TEST_F(NotificationResponseBuilderMacTest, TestNotificationClose) {
   EXPECT_EQ(1 /* NOTIFICATION_CLOSE */, operation.intValue);
   EXPECT_EQ(-1, buttonIndex.intValue);
 }
+
+TEST_F(NotificationResponseBuilderMacTest, TestNotificationExtension) {
+  base::scoped_nsobject<NotificationBuilder> builder =
+      NewTestBuilder(NotificationCommon::EXTENSION);
+  [builder setButtons:@"Button1" secondaryButton:@"Button2"];
+  NSUserNotification* notification = [builder buildUserNotification];
+  // These values will be set by the notification center to indicate that button
+  // 1 was clicked.
+  [notification
+      setValue:
+          [NSNumber
+              numberWithInt:NSUserNotificationActivationTypeActionButtonClicked]
+        forKey:@"_activationType"];
+  [notification setValue:[NSNumber numberWithInt:1]
+                  forKey:@"_alternateActionIndex"];
+
+  NSDictionary* response =
+      [NotificationResponseBuilder buildDictionary:notification];
+
+  NSNumber* operation =
+      [response objectForKey:notification_constants::kNotificationOperation];
+  NSNumber* buttonIndex =
+      [response objectForKey:notification_constants::kNotificationButtonIndex];
+  EXPECT_EQ(0 /* NOTIFICATION_CLICK */, operation.intValue);
+  EXPECT_EQ(1, buttonIndex.intValue);
+}