Deflake PushMessagingBrowserTest.DenyPushPermissionUnregisters & friends

chrome/browser/push_messaging/push_messaging_browsertest.cc

Index: chrome/browser/push_messaging/push_messaging_browsertest.cc
diff --git a/chrome/browser/push_messaging/push_messaging_browsertest.cc b/chrome/browser/push_messaging/push_messaging_browsertest.cc
index 6960f4b65ae4cf41914af7b9885f6fac7c6d4cfa..6768fc3f23b327ce14033602b014a09b71482885 100644
--- a/chrome/browser/push_messaging/push_messaging_browsertest.cc
+++ b/chrome/browser/push_messaging/push_messaging_browsertest.cc
@@ -5,9 +5,11 @@
 #include <map>
 #include <string>
 
+#include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/notifications/notification_test_util.h"
@@ -89,22 +91,13 @@ class InfoBarResponder : public infobars::InfoBarManager::Observer {
 // FakeGCMProfileService::UnregisterCallback.
 class UnregistrationCallback {
  public:
-  UnregistrationCallback() : done_(false), waiting_(false) {}
-
   void Run(const std::string& app_id) {
     app_id_ = app_id;
-    done_ = true;
-    if (waiting_)
-      base::MessageLoop::current()->Quit();
+    run_loop_.Quit();
   }
 
   void WaitUntilSatisfied() {
-    if (done_)
-      return;
-
-    waiting_ = true;
-    while (!done_)
-      content::RunMessageLoop();
+    run_loop_.Run();
   }
 
   const std::string& app_id() {
@@ -112,38 +105,10 @@ class UnregistrationCallback {
   }
 
  private:
-  bool done_;
-  bool waiting_;
+  base::RunLoop run_loop_;

[Avi (use Gerrit), 2015/03/13 13:58:04]

MessageLoopRunner.

[johnme, 2015/03/13 14:21:29]

Done.

   std::string app_id_;
 };
 
-// Class to instantiate on the stack that is meant to be used with
-// StubNotificationUIManager::SetNotificationAddedCallback. Mind that Run()
-// might be invoked prior to WaitUntilSatisfied() being called.
-class NotificationAddedCallback {
- public:
-  NotificationAddedCallback() : done_(false), waiting_(false) {}
-
-  void Run() {
-    done_ = true;
-    if (waiting_)
-      base::MessageLoop::current()->Quit();
-  }
-
-  void WaitUntilSatisfied() {
-    if (done_)
-      return;
-
-    waiting_ = true;
-    while (!done_)
-      content::RunMessageLoop();
-  }
-
- private:
-  bool done_;
-  bool waiting_;
-};
-
 // The Push API depends on Web Notifications, which is only available on Android
 // Jelly Bean and later.
 bool IsPushSupported() {
@@ -639,9 +604,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("isControlled()", &script_result));
   ASSERT_EQ("true - is controlled", script_result);
 
-  NotificationAddedCallback callback;
-  notification_manager()->SetNotificationAddedCallback(
-      base::Bind(&NotificationAddedCallback::Run, base::Unretained(&callback)));
+  base::RunLoop run_loop;
+  notification_manager()->SetNotificationAddedCallback(run_loop.QuitClosure());
 
   gcm::GCMClient::IncomingMessage message;
   message.sender_id = "1234567890";
@@ -650,7 +614,7 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
   EXPECT_EQ("immediate:shownotification-without-waituntil", script_result);
 
-  callback.WaitUntilSatisfied();
+  run_loop.Run();
 
   ASSERT_EQ(1u, notification_manager()->GetNotificationCount());
   EXPECT_EQ("push_test_tag",
@@ -786,9 +750,15 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   browser()->profile()->GetHostContentSettingsMap()->
       ClearSettingsForOneType(CONTENT_SETTINGS_TYPE_PUSH_MESSAGING);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
@@ -808,6 +778,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
@@ -816,6 +790,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_DEFAULT);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
@@ -835,6 +811,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
@@ -843,6 +823,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_BLOCK);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - denied", script_result);
 
@@ -862,9 +844,15 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   browser()->profile()->GetHostContentSettingsMap()->
       ClearSettingsForOneType(CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
@@ -884,6 +872,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
@@ -892,6 +884,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_DEFAULT);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
@@ -911,6 +905,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      run_loop.QuitClosure());
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
@@ -919,6 +917,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_BLOCK);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - denied", script_result);
 
@@ -938,6 +938,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      base::BarrierClosure(2, run_loop.QuitClosure()));
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
@@ -952,6 +956,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_ALLOW);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
@@ -975,6 +981,10 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
+  base::RunLoop run_loop;
+  push_service()->SetContentSettingChangedCallbackForTesting(
+      base::BarrierClosure(4, run_loop.QuitClosure()));
+
   GURL origin = https_server()->GetURL(std::string()).GetOrigin();
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::Wildcard(),
@@ -1001,6 +1011,8 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
       std::string(),
       CONTENT_SETTING_DEFAULT);
 
+  run_loop.Run();
+
   // The two first rules should give |origin| the permission to use Push even
   // if the rules it used to have have been reset.
   // The Push service should not unsubcribe |origin| because at no point it was