Revert "Pass incognito flag when closing macOS alerts"

This reverts commit e48554ec.

Reason for revert: breaks NotificationPlatformBridgeMacTest.TestIncognitoProfile
 on Mac10.12 Tests

See https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.12%20Tests/41203/overview

Original change's description:
> Pass incognito flag when closing macOS alerts
>
> A macOS notification is identified with the tuple
> {notificationId, profileId, incognito} and we have to pass these down to
> the XPC service to close the correct notification. The same applies to
> the notification id used in the UNNotification API where we need to
> include the incognito flag in the id. Also updates some method names to
> be more consistent.
>
> Bug: None
> Change-Id: Ie45cb74eab182e0b495560110d8e6e32367c4bc7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2642275
> Reviewed-by: Rayan Kanso <rayankans@chromium.org>
> Commit-Queue: Richard Knoll <knollr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#846183}

TBR=rayankans@chromium.org,knollr@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I0d1344c1f61a8dc04a156a94810a8059ffa55e6a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2645156Reviewed-by: Richard Knoll <knollr@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846345}

chrome/browser/notifications/alert_dispatcher_mac.h

@@ -15,17 +15,14 @@
 // Deliver a notification to the XPC service to be displayed as an alert.
 - (void)dispatchNotification:(NSDictionary*)data;
 
-// Close a notification for a given |notificationId|, |profileId| and
-// |incognito|.
+// Close a notification for a given |notificationId| and |profileId|.
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito;
+                  withProfileId:(NSString*)profileId;
 
 // Close all notifications.
 - (void)closeAllNotifications;
 
-// Get currently displayed notifications for |profileId| and |incognito|. The
-// returned ids are scoped to the passed profile and are not globally unique.
+// Get currently displayed notifications.
 - (void)
 getDisplayedAlertsForProfileId:(NSString*)profileId
                      incognito:(BOOL)incognito

chrome/browser/notifications/notification_platform_bridge_mac.mm

@@ -221,32 +221,32 @@ void NotificationPlatformBridgeMac::Display(
 
 void NotificationPlatformBridgeMac::Close(Profile* profile,
                                           const std::string& notification_id) {
-  NSString* notificationId = base::SysUTF8ToNSString(notification_id);
-  NSString* profileId = base::SysUTF8ToNSString(GetProfileId(profile));
-  bool incognito = profile->IsOffTheRecord();
+  NSString* candidate_id = base::SysUTF8ToNSString(notification_id);
+  NSString* current_profile_id = base::SysUTF8ToNSString(GetProfileId(profile));
 
+  bool notification_removed = false;
   for (NSUserNotification* toast in
        [notification_center_ deliveredNotifications]) {
-    NSString* toastId =
+    NSString* toast_id =
         [toast.userInfo objectForKey:notification_constants::kNotificationId];
-    NSString* toastProfileId = [toast.userInfo
+
+    NSString* persistent_profile_id = [toast.userInfo
         objectForKey:notification_constants::kNotificationProfileId];
-    BOOL toastIncognito = [[toast.userInfo
-        objectForKey:notification_constants::kNotificationIncognito] boolValue];
 
-    if ([notificationId isEqualToString:toastId] &&
-        [profileId isEqualToString:toastProfileId] &&
-        incognito == toastIncognito) {
+    if ([toast_id isEqualToString:candidate_id] &&
+        [persistent_profile_id isEqualToString:current_profile_id]) {
       [notification_center_ removeDeliveredNotification:toast];
-      return;
+      notification_removed = true;
+      break;
     }
   }
 
   // If no banner existed with that ID try to see if there is an alert
   // in the xpc server.
-  [alert_dispatcher_ closeNotificationWithId:notificationId
-                                   profileId:profileId
-                                   incognito:incognito];
+  if (!notification_removed) {
+    [alert_dispatcher_ closeNotificationWithId:candidate_id
+                                 withProfileId:current_profile_id];
+  }
 }
 
 void NotificationPlatformBridgeMac::GetDisplayed(
@@ -280,8 +280,9 @@ void NotificationPlatformBridgeMac::GetDisplayed(
       },
       std::move(callback), std::move(banners));
 
-  [alert_dispatcher_ getDisplayedAlertsForProfileId:profileId
-                                          incognito:incognito
+  [alert_dispatcher_ getDisplayedAlertsForProfileId:base::SysUTF8ToNSString(
+                                                        GetProfileId(profile))
+                                          incognito:profile->IsOffTheRecord()
                                            callback:std::move(alerts_callback)];
 }
 
@@ -386,11 +387,9 @@ void NotificationPlatformBridgeMac::DisplayServiceShutDown(Profile* profile) {}
 }
 
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito {
+                  withProfileId:(NSString*)profileId {
   [[self serviceProxy] closeNotificationWithId:notificationId
-                                     profileId:profileId
-                                     incognito:incognito];
+                                 withProfileId:profileId];
 }
 
 - (void)closeAllNotifications {
@@ -413,12 +412,12 @@ getDisplayedAlertsForProfileId:(NSString*)profileId
     content::GetUIThreadTaskRunner({})->PostTask(
         FROM_HERE,
         base::BindOnce(copyable_callback, std::move(displayedNotifications),
-                       /*supports_synchronization=*/true));
+                       true /* supports_synchronization */));
   };
 
   [[self serviceProxy] getDisplayedAlertsForProfileId:profileId
-                                            incognito:incognito
-                                                reply:reply];
+                                         andIncognito:incognito
+                                            withReply:reply];
 }
 
 // NotificationReply:

chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm

@@ -144,42 +144,6 @@ TEST_F(NotificationPlatformBridgeMacTest, TestDisplayNoButtons) {
     EXPECT_NSEQ(@"Close", [delivered_notification otherButtonTitle]);
 }
 
-TEST_F(NotificationPlatformBridgeMacTest, TestIncognitoProfile) {
-  std::unique_ptr<NotificationPlatformBridgeMac> bridge(
-      new NotificationPlatformBridgeMac(notification_center(),
-                                        alert_dispatcher()));
-  std::unique_ptr<Notification> notification =
-      CreateBanner("Title", "Context", "https://gmail.com", nullptr, nullptr);
-
-  TestingProfile::Builder profile_builder;
-  profile_builder.SetPath(profile()->GetPath());
-  profile_builder.SetProfileName(profile()->GetProfileUserName());
-  Profile* incogito_profile = profile_builder.BuildIncognito(profile());
-
-  // Show two notifications with the same id from different profiles.
-  bridge->Display(NotificationHandler::Type::WEB_PERSISTENT, profile(),
-                  *notification, /*metadata=*/nullptr);
-  bridge->Display(NotificationHandler::Type::WEB_PERSISTENT, incogito_profile,
-                  *notification, /*metadata=*/nullptr);
-  EXPECT_EQ(2u, [[notification_center() deliveredNotifications] count]);
-
-  // Close the one for the incognito profile.
-  bridge->Close(incogito_profile, "id1");
-  NSArray* notifications = [notification_center() deliveredNotifications];
-  ASSERT_EQ(1u, [notifications count]);
-
-  // Expect that the remaining notification is for the regular profile.
-  NSUserNotification* remaining_notification = [notifications objectAtIndex:0];
-  EXPECT_EQ(false,
-            [[[remaining_notification userInfo]
-                objectForKey:notification_constants::kNotificationIncognito]
-                boolValue]);
-
-  // Close the one for the regular profile.
-  bridge->Close(profile(), "id1");
-  EXPECT_EQ(0u, [[notification_center() deliveredNotifications] count]);
-}
-
 TEST_F(NotificationPlatformBridgeMacTest, TestDisplayNoSettings) {
   std::unique_ptr<Notification> notification = CreateNotification(
       "Title", "Context", "https://gmail.com", nullptr, nullptr,

chrome/browser/notifications/notification_platform_bridge_mac_unnotification.mm

@@ -154,8 +154,8 @@ void NotificationPlatformBridgeMacUNNotification::Display(
   // Create a new category from the desired action buttons.
   UNNotificationCategory* category = [builder buildCategory];
 
-  std::string system_notification_id = DeriveMacNotificationId(
-      profile->IsOffTheRecord(), GetProfileId(profile), notification.id());
+  std::string system_notification_id =
+      DeriveMacNotificationId(GetProfileId(profile), notification.id());
   NSString* notification_id = base::SysUTF8ToNSString(system_notification_id);
 
   // Check if this notification had an already existing category from a previous
@@ -218,8 +218,8 @@ void NotificationPlatformBridgeMacUNNotification::Display(
 void NotificationPlatformBridgeMacUNNotification::Close(
     Profile* profile,
     const std::string& notification_id) {
-  NSString* notificationId = base::SysUTF8ToNSString(DeriveMacNotificationId(
-      profile->IsOffTheRecord(), GetProfileId(profile), notification_id));
+  NSString* notificationId = base::SysUTF8ToNSString(
+      DeriveMacNotificationId(GetProfileId(profile), notification_id));
   base::WeakPtr<NotificationPlatformBridgeMacUNNotification> weak_ptr =
       weak_factory_.GetWeakPtr();
 

chrome/browser/notifications/notification_platform_bridge_mac_unnotification_unittest.mm

@@ -254,56 +254,6 @@ TEST_F(UNNotificationPlatformBridgeMacTest, TestDisplayMultipleProfiles) {
   }
 }
 
-TEST_F(UNNotificationPlatformBridgeMacTest, TestIncognitoProfile) {
-  if (@available(macOS 10.14, *)) {
-    Notification notification = CreateNotification();
-
-    TestingProfile::Builder profile_builder;
-    profile_builder.SetPath(profile_->GetPath());
-    profile_builder.SetProfileName(profile_->GetProfileUserName());
-    Profile* incogito_profile = profile_builder.BuildIncognito(profile_);
-
-    // Show two notifications with the same id from different profiles.
-    bridge_->Display(NotificationHandler::Type::WEB_PERSISTENT, profile_,
-                     notification, /*metadata=*/nullptr);
-    bridge_->Display(NotificationHandler::Type::WEB_PERSISTENT,
-                     incogito_profile, notification,
-                     /*metadata=*/nullptr);
-    [center_ getDeliveredNotificationsWithCompletionHandler:^(
-                 NSArray<UNNotification*>* _Nonnull notifications) {
-      EXPECT_EQ(2u, [notifications count]);
-    }];
-
-    // Close the one for the incognito profile.
-    bridge_->Close(incogito_profile, "id1");
-    // Close runs async code that we can't observe, make sure all tasks run.
-    base::RunLoop().RunUntilIdle();
-
-    __block UNNotification* remaining = nullptr;
-    [center_ getDeliveredNotificationsWithCompletionHandler:^(
-                 NSArray<UNNotification*>* _Nonnull notifications) {
-      ASSERT_EQ(1u, [notifications count]);
-      remaining = [notifications objectAtIndex:0];
-    }];
-
-    // Expect that the remaining notification is for the regular profile.
-    EXPECT_EQ(false,
-              [[[[[remaining request] content] userInfo]
-                  objectForKey:notification_constants::kNotificationIncognito]
-                  boolValue]);
-
-    // Close the one for the regular profile.
-    bridge_->Close(profile_, "id1");
-    // Close runs async code that we can't observe, make sure all tasks run.
-    base::RunLoop().RunUntilIdle();
-
-    [center_ getDeliveredNotificationsWithCompletionHandler:^(
-                 NSArray<UNNotification*>* _Nonnull notifications) {
-      EXPECT_EQ(0u, [notifications count]);
-    }];
-  }
-}
-
 TEST_F(UNNotificationPlatformBridgeMacTest, TestNotificationHasIcon) {
   if (@available(macOS 10.14, *)) {
     Notification notification = CreateNotification();

chrome/browser/notifications/notification_platform_bridge_mac_utils.h

@@ -28,8 +28,7 @@ base::string16 CreateMacNotificationContext(
 
 // Derives a unique notification identifier to be used by the macOS system
 // notification center to uniquely identify a notification.
-std::string DeriveMacNotificationId(bool incognito,
-                                    const std::string& profile_id,
+std::string DeriveMacNotificationId(const std::string& profile_id,
                                     const std::string& notification_id);
 
 // Validates contents of the |response| dictionary as received from the system

chrome/browser/notifications/notification_platform_bridge_mac_utils.mm

@@ -125,12 +125,9 @@ base::string16 CreateMacNotificationContext(
   return etldplusone;
 }
 
-std::string DeriveMacNotificationId(bool incognito,
-                                    const std::string& profile_id,
+std::string DeriveMacNotificationId(const std::string& profile_id,
                                     const std::string& notification_id) {
-  // i: incognito, r: regular profile
-  return base::StrCat(
-      {incognito ? "i" : "r", "|", profile_id, "|", notification_id});
+  return base::StrCat({profile_id, "|", notification_id});
 }
 
 bool VerifyMacNotificationData(NSDictionary* response) {

chrome/browser/notifications/stub_alert_dispatcher_mac.mm

@@ -29,21 +29,16 @@
 }
 
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito {
+                  withProfileId:(NSString*)profileId {
   DCHECK(profileId);
   DCHECK(notificationId);
   for (NSDictionary* toast in _alerts.get()) {
     NSString* toastId =
         [toast objectForKey:notification_constants::kNotificationId];
-    NSString* toastProfileId =
+    NSString* persistentProfileId =
         [toast objectForKey:notification_constants::kNotificationProfileId];
-    BOOL toastIncognito = [[toast
-        objectForKey:notification_constants::kNotificationIncognito] boolValue];
-
-    if ([notificationId isEqualToString:toastId] &&
-        [profileId isEqualToString:toastProfileId] &&
-        incognito == toastIncognito) {
+    if ([toastId isEqualToString:notificationId] &&
+        [persistentProfileId isEqualToString:profileId]) {
       [_alerts removeObject:toast];
       break;
     }

chrome/browser/notifications/stub_notification_center_mac.mm

@@ -41,20 +41,15 @@
       objectForKey:notification_constants::kNotificationId];
   NSString* profileId = [notification.userInfo
       objectForKey:notification_constants::kNotificationProfileId];
-  BOOL incognito = [[notification.userInfo
-      objectForKey:notification_constants::kNotificationIncognito] boolValue];
   DCHECK(profileId);
   DCHECK(notificationId);
   for (NSUserNotification* toast in _banners.get()) {
     NSString* toastId =
         [toast.userInfo objectForKey:notification_constants::kNotificationId];
-    NSString* toastProfileId = [toast.userInfo
+    NSString* persistentProfileId = [toast.userInfo
         objectForKey:notification_constants::kNotificationProfileId];
-    BOOL toastIncognito = [[toast.userInfo
-        objectForKey:notification_constants::kNotificationIncognito] boolValue];
-    if ([notificationId isEqualToString:toastId] &&
-        [profileId isEqualToString:toastProfileId] &&
-        incognito == toastIncognito) {
+    if ([toastId isEqualToString:notificationId] &&
+        [persistentProfileId isEqualToString:profileId]) {
       [_banners removeObject:toast];
       break;
     }

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

@@ -84,14 +84,12 @@ crashpad::SimpleStringDictionary* GetCrashpadAnnotations() {
 }
 
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito {
+                  withProfileId:(NSString*)profileId {
   DCHECK(_didSetExceptionPort);
   DCHECK(_notificationDelivery);
 
   [_notificationDelivery closeNotificationWithId:notificationId
-                                       profileId:profileId
-                                       incognito:incognito];
+                                   withProfileId:profileId];
 }
 
 - (void)closeAllNotifications {
@@ -102,13 +100,13 @@ crashpad::SimpleStringDictionary* GetCrashpadAnnotations() {
 }
 
 - (void)getDisplayedAlertsForProfileId:(NSString*)profileId
-                             incognito:(BOOL)incognito
-                                 reply:(void (^)(NSArray*))reply {
+                          andIncognito:(BOOL)incognito
+                             withReply:(void (^)(NSArray*))reply {
   DCHECK(_notificationDelivery);
 
   [_notificationDelivery getDisplayedAlertsForProfileId:profileId
-                                              incognito:incognito
-                                                  reply:reply];
+                                           andIncognito:incognito
+                                              withReply:reply];
 }
 
 @end

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

@@ -49,23 +49,20 @@
 }
 
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito {
+                  withProfileId:(NSString*)profileId {
   NSUserNotificationCenter* notificationCenter =
       [NSUserNotificationCenter defaultUserNotificationCenter];
-  NSArray* deliveredNotifications = [notificationCenter deliveredNotifications];
-  for (NSUserNotification* toast in deliveredNotifications) {
-    NSString* toastId =
-        [toast.userInfo objectForKey:notification_constants::kNotificationId];
-    NSString* toastProfileId = [toast.userInfo
+  for (NSUserNotification* candidate in
+       [notificationCenter deliveredNotifications]) {
+    NSString* candidateId = [candidate.userInfo
+        objectForKey:notification_constants::kNotificationId];
+
+    NSString* candidateProfileId = [candidate.userInfo
         objectForKey:notification_constants::kNotificationProfileId];
-    BOOL toastIncognito = [[toast.userInfo
-        objectForKey:notification_constants::kNotificationIncognito] boolValue];
 
-    if ([notificationId isEqualToString:toastId] &&
-        [profileId isEqualToString:toastProfileId] &&
-        incognito == toastIncognito) {
-      [notificationCenter removeDeliveredNotification:toast];
+    if ([candidateId isEqualToString:notificationId] &&
+        [profileId isEqualToString:candidateProfileId]) {
+      [notificationCenter removeDeliveredNotification:candidate];
       [_transactionHandler closeTransactionIfNeeded];
       break;
     }
@@ -79,21 +76,20 @@
 }
 
 - (void)getDisplayedAlertsForProfileId:(NSString*)profileId
-                             incognito:(BOOL)incognito
-                                 reply:(void (^)(NSArray*))reply {
+                          andIncognito:(BOOL)incognito
+                             withReply:(void (^)(NSArray*))reply {
   NSUserNotificationCenter* notificationCenter =
       [NSUserNotificationCenter defaultUserNotificationCenter];
   NSArray* deliveredNotifications = [notificationCenter deliveredNotifications];
   NSMutableArray* notificationIds =
       [NSMutableArray arrayWithCapacity:[deliveredNotifications count]];
   for (NSUserNotification* toast in deliveredNotifications) {
-    NSString* toastProfileId = [toast.userInfo
+    NSString* candidateProfileId = [toast.userInfo
         objectForKey:notification_constants::kNotificationProfileId];
-    BOOL toastIncognito = [[toast.userInfo
+    BOOL incognitoNotification = [[toast.userInfo
         objectForKey:notification_constants::kNotificationIncognito] boolValue];
-
-    if ([profileId isEqualToString:toastProfileId] &&
-        incognito == toastIncognito) {
+    if ([candidateProfileId isEqualToString:profileId] &&
+        incognito == incognitoNotification) {
       [notificationIds
           addObject:[toast.userInfo
                         objectForKey:notification_constants::kNotificationId]];

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

@@ -24,10 +24,9 @@
 // |notificationData| is generated using a NofiticationBuilder object.
 - (void)deliverNotification:(NSDictionary*)notificationData;
 
-// Closes an alert with the given |notificationId|, |profileId| and |incognito|.
+// Closes an alert with the given |notificationId| and |profileId|.
 - (void)closeNotificationWithId:(NSString*)notificationId
-                      profileId:(NSString*)profileId
-                      incognito:(BOOL)incognito;
+                  withProfileId:(NSString*)profileId;
 
 // Closes all the alerts being displayed.
 - (void)closeAllNotifications;
@@ -35,9 +34,8 @@
 // Will invoke |reply| with an array of NSString notification IDs for all alerts
 // for |profileId| and |incognito| value currently displayed.
 - (void)getDisplayedAlertsForProfileId:(NSString*)profileId
-                             incognito:(BOOL)incognito
-                                 reply:(void (^)(NSArray*))reply;
-
+                          andIncognito:(BOOL)incognito
+                             withReply:(void (^)(NSArray*))reply;
 @end
 
 // Response protocol for the XPC notification service to notify Chrome of