Make GetDisplayedNotifications asynchronous.

chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm b/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
index fc3a98afd8fa2fb559439423d2ae6da96ecb5e97..555388ce505bb3df5af0bccb856750762946b7b9 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
@@ -5,7 +5,9 @@
 #import <AppKit/AppKit.h>
 #import <objc/runtime.h>
 
+#include "base/bind.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_platform_bridge_mac.h"
@@ -16,12 +18,15 @@
 #include "chrome/browser/ui/cocoa/notifications/notification_constants_mac.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h"
 #include "chrome/common/features.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gtest_mac.h"
 #include "url/gurl.h"
 
 class NotificationPlatformBridgeMacTest : public testing::Test {
  public:
+  NotificationPlatformBridgeMacTest()
+      : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}

[Peter Beverloo, 2017/03/15 18:07:50]

Huh? Where does this test require the IO thread?

[Miguel Garcia, 2017/03/16 14:57:42]

duh, that was needed when I was replying in the IO thread, changed to DEFAULT
now.

   void SetUp() override {
     notification_center_.reset([[StubNotificationCenter alloc] init]);
     alert_dispatcher_.reset([[StubAlertDispatcher alloc] init]);
@@ -53,6 +58,13 @@ class NotificationPlatformBridgeMacTest : public testing::Test {
     return [builder buildUserNotification];
   }
 
+  static void CheckNotificationCount(
+      size_t expected,
+      std::unique_ptr<std::set<std::string>> notifications,
+      bool supports_sync) {
+    EXPECT_EQ(expected, notifications->size());
+  }
+
   std::unique_ptr<Notification> CreateBanner(const char* title,
                                              const char* subtitle,
                                              const char* origin,
@@ -116,6 +128,7 @@ class NotificationPlatformBridgeMacTest : public testing::Test {
  private:
   base::scoped_nsobject<StubNotificationCenter> notification_center_;
   base::scoped_nsobject<StubAlertDispatcher> alert_dispatcher_;
+  content::TestBrowserThreadBundle thread_bundle_;
 };
 
 TEST_F(NotificationPlatformBridgeMacTest, TestNotificationVerifyValidResponse) {
@@ -263,9 +276,9 @@ TEST_F(NotificationPlatformBridgeMacTest, TestGetDisplayed) {
                   "profile_id", false, *notification);
   EXPECT_EQ(1u, [[notification_center() deliveredNotifications] count]);
 
-  std::set<std::string> notifications;
-  EXPECT_TRUE(bridge->GetDisplayed("profile_id", false, &notifications));
-  EXPECT_EQ(1u, notifications.size());
+  bridge->GetDisplayed("profile_id", false /* incognito */,
+                       base::Bind(&CheckNotificationCount, 1u));
+  base::RunLoop().RunUntilIdle();

[Peter Beverloo, 2017/03/15 18:07:50]

nit: another paradigm you can consider is:

void ReturnNotificationCount(
    size_t* out_notification_count,
    base::Closure quit_closure,
    std::unique_ptr<std::set<std::string>> notifications,
    bool supports_sync) {
  DCHECK(out_notification_count);
  *out_notification_count = notification.size();
  quit_closure.Run();
}

---

{
  size_t notification_count;

  base::RunLoop run_loop;
  bridge->GetDisplayed("profile_id", false /* incognito */,
      base::Bind(&ReturnNotificationCount, &notification_count,
                 run_loop.QuitClosure()));
  run_loop.Run();

  EXPECT_EQ(notification_count, 1u);
}

[Miguel Garcia, 2017/03/16 14:57:42]

I rather leave it as is unless you disagree strongly :)


On 2017/03/15 18:07:50, Peter Beverloo wrote:

 }
 
 TEST_F(NotificationPlatformBridgeMacTest, TestGetDisplayedUnknownProfile) {
@@ -277,11 +290,9 @@ TEST_F(NotificationPlatformBridgeMacTest, TestGetDisplayedUnknownProfile) {
   bridge->Display(NotificationCommon::PERSISTENT, "notification_id",
                   "profile_id", false, *notification);
   EXPECT_EQ(1u, [[notification_center() deliveredNotifications] count]);
-
-  std::set<std::string> notifications;
-  EXPECT_TRUE(
-      bridge->GetDisplayed("unknown_profile_id", false, &notifications));
-  EXPECT_EQ(0u, notifications.size());
+  bridge->GetDisplayed("unknown_profile_id", false /* incognito */,
+                       base::Bind(&CheckNotificationCount, 0u));
+  base::RunLoop().RunUntilIdle();
 }
 
 TEST_F(NotificationPlatformBridgeMacTest, TestQuitRemovesNotifications) {