Fix a few bugs when closing mac notifications

chrome/browser/notifications/platform_notification_service_impl.cc

 // Copyright 2014 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/platform_notification_service_impl.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 392 matching lines @@
   DCHECK(profile);
 
   closed_notifications_.insert(persistent_notification_id);
 
 #if defined(OS_ANDROID)
   bool cancel_by_persistent_id = true;
 #else
   bool cancel_by_persistent_id =
       GetNotificationDisplayService(profile)->SupportsNotificationCenter();
 #endif
-
+  auto iter = persistent_notifications_.find(persistent_notification_id);
+  if (iter == persistent_notifications_.end())
+    return;

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

This will break closing persistent notifications shown during a previous session
of the browser. (I.e. before a restart.)

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

True, changed it a bit to reflect that case, if instead of deleting the element
through the iterator we delete it through the key it becomes easier .

   if (cancel_by_persistent_id) {
     // TODO(peter): Remove this conversion when the notification ids are being
     // generated by the caller of this method.
     GetNotificationDisplayService(profile)->Close(
         base::Int64ToString(persistent_notification_id));
+  } else {
+    GetNotificationDisplayService(profile)->Close(iter->second);

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

Is invoking Close() twice with different values a problem for Mac?

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

It isn't but in a world with an XPC service it will have to go try to find the
notification id both in the local notification center and the XPC one which
seemed like a waste.

   }
-
-  auto iter = persistent_notifications_.find(persistent_notification_id);
-  if (iter == persistent_notifications_.end())
-    return;
-
-  GetNotificationDisplayService(profile)->Close(iter->second);
-
   persistent_notifications_.erase(iter);
 }
 
 bool PlatformNotificationServiceImpl::GetDisplayedPersistentNotifications(
     BrowserContext* browser_context,
     std::set<std::string>* displayed_notifications) {
   DCHECK(displayed_notifications);
 
   Profile* profile = Profile::FromBrowserContext(browser_context);
   if (!profile || profile->AsTestingProfile())
@@ skipping 127 matching lines @@
   }
 #endif
 
   return base::string16();
 }
 
 void PlatformNotificationServiceImpl::SetNotificationDisplayServiceForTesting(
     NotificationDisplayService* display_service) {
   test_display_service_ = display_service;
 }