Fix a few bugs when closing mac notifications

chrome/browser/notifications/notification_platform_bridge_mac.mm

 // Copyright 2015 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.
 
 #include "chrome/browser/notifications/notification_platform_bridge_mac.h"
 
 #include <utility>
 
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
@@ skipping 104 matching lines @@
     // from the notification center before displaying this one.
     // TODO(miguelg): This will need to work for alerts as well via XPC
     // once supported.
     if (notification.renotify()) {
       NSUserNotificationCenter* notification_center =
           [NSUserNotificationCenter defaultUserNotificationCenter];
       for (NSUserNotification* existing_notification in
            [notification_center deliveredNotifications]) {
         NSString* identifier =
             [existing_notification valueForKey:@"identifier"];
-        if ([identifier isEqual:base::SysUTF8ToNSString(notification.tag())]) {
+        if ([identifier
+                isEqualToString:base::SysUTF8ToNSString(notification.tag())]) {
           [notification_center
               removeDeliveredNotification:existing_notification];
           break;
         }
       }
     }
   }
 
   [builder setOrigin:base::SysUTF8ToNSString(notification.origin_url().spec())];
   [builder setNotificationId:base::SysUTF8ToNSString(notification_id)];
   [builder setProfileId:base::SysUTF8ToNSString(profile_id)];
   [builder setIncognito:incognito];
 
   NSUserNotification* toast = [builder buildUserNotification];
   [notification_center_ deliverNotification:toast];
 }
 
 void NotificationPlatformBridgeMac::Close(const std::string& profile_id,
                                           const std::string& notification_id) {
   NSString* candidate_id = base::SysUTF8ToNSString(notification_id);
 
   NSString* current_profile_id = base::SysUTF8ToNSString(profile_id);
   for (NSUserNotification* toast in
        [notification_center_ deliveredNotifications]) {
     NSString* toast_id =
         [toast.userInfo objectForKey:notification_constants::kNotificationId];
 
     NSString* persistent_profile_id = [toast.userInfo
         objectForKey:notification_constants::kNotificationProfileId];
-
-    if (toast_id == candidate_id &&
-        persistent_profile_id == current_profile_id) {
+    if ([toast_id isEqualToString:candidate_id] &&
+        [persistent_profile_id isEqualToString:current_profile_id]) {

[Peter Beverloo, 2016/06/26 20:46:26]

This seems like the kind of bug that would easily be catchable by unit tests. Do
you still plan to add some?

[Miguel Garcia, 2016/06/28 08:17:20]

You'd thing but in the tests all strings would be static so they'd share the
same pointer and it would not be caught :(

In any case yes, as you've seen in previews reviews as I refactor all the paths
(send, click..) to provide support for XPC I am adding unit tests. Close has not
been through that process yet.

       [notification_center_ removeDeliveredNotification:toast];
     }
   }
 }
 
 bool NotificationPlatformBridgeMac::GetDisplayed(
     const std::string& profile_id,
     bool incognito,
     std::set<std::string>* notifications) const {
   DCHECK(notifications);
@@ skipping 55 matching lines @@
           buttonIndex.intValue);
 }
 
 - (BOOL)userNotificationCenter:(NSUserNotificationCenter*)center
      shouldPresentNotification:(NSUserNotification*)nsNotification {
   // Always display notifications, regardless of whether the app is foreground.
   return YES;
 }
 
 @end