Deflake PushMessagingBrowserTest.DenyPushPermissionUnregisters & friends

chrome/browser/push_messaging/push_messaging_browsertest.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 <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"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/push_messaging/push_messaging_application_id.h"
 #include "chrome/browser/push_messaging/push_messaging_constants.h"
 #include "chrome/browser/push_messaging/push_messaging_service_factory.h"
 #include "chrome/browser/push_messaging/push_messaging_service_impl.h"
 #include "chrome/browser/services/gcm/fake_gcm_profile_service.h"
@@ skipping 61 matching lines @@
   InfoBarService* infobar_service_;
   bool accept_;
   bool has_observed_;
 };
 
 // Class to instantiate on the stack that is meant to be used with
 // FakeGCMProfileService. The ::Run() method follows the signature of
 // 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() {
     return app_id_;
   }
 
  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() {
 #if defined(OS_ANDROID)
   if (base::android::BuildInfo::GetInstance()->sdk_int() <
       base::android::SDK_VERSION_JELLY_BEAN) {
     DVLOG(0) << "The Push API is only supported in Android 4.1 and later.";
     return false;
   }
 #endif
@@ skipping 475 matching lines @@
   EXPECT_EQ("1234567890", gcm_service()->last_registered_sender_ids()[0]);
 
   ASSERT_TRUE(RunScript("isControlled()", &script_result));
   ASSERT_EQ("false - is not controlled", script_result);
 
   LoadTestPage();  // Reload to become controlled.
 
   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";
   message.data["data"] = "shownotification-without-waituntil";
   push_service()->OnMessage(app_id.app_id_guid(), message);
   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",
             notification_manager()->GetNotificationAt(0).tag());
 
   // Verify that the renderer process hasn't crashed.
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 }
 #endif
@@ skipping 115 matching lines @@
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        LocalResetPushPermissionUnregisters) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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),
       ContentSettingsPattern::FromURLNoWildcard(origin),
       CONTENT_SETTINGS_TYPE_PUSH_MESSAGING,
       std::string(),
       CONTENT_SETTING_DEFAULT);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        DenyPushPermissionUnregisters) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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),
       ContentSettingsPattern::FromURLNoWildcard(origin),
       CONTENT_SETTINGS_TYPE_PUSH_MESSAGING,
       std::string(),
       CONTENT_SETTING_BLOCK);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - denied", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        GlobalResetNotificationsPermissionUnregisters) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        LocalResetNotificationsPermissionUnregisters) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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),
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       std::string(),
       CONTENT_SETTING_DEFAULT);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - default", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        DenyNotificationsPermissionUnregisters) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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),
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       std::string(),
       CONTENT_SETTING_BLOCK);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - denied", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("false - not registered", script_result);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        GrantAlreadyGrantedPermissionDoesNotUnregister) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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),
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       std::string(),
       CONTENT_SETTING_ALLOW);
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
       ContentSettingsPattern::FromURLNoWildcard(origin),
       CONTENT_SETTINGS_TYPE_PUSH_MESSAGING,
       std::string(),
       CONTENT_SETTING_ALLOW);
 
+  run_loop.Run();
+
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 }
 
 // This test is testing some non-trivial content settings rules and make sure
 // that they are respected with regards to automatic unregistration. In other
 // words, it checks that the push service does not end up unregistering origins
 // that have push permission with some non-common rules.
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
                        AutomaticUnregistrationFollowsContentSettingRules) {
   std::string script_result;
 
   TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 
   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(),
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       std::string(),
       CONTENT_SETTING_ALLOW);
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromString("https://*"),
       ContentSettingsPattern::FromString("https://*"),
       CONTENT_SETTINGS_TYPE_PUSH_MESSAGING,
       std::string(),
       CONTENT_SETTING_ALLOW);
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       std::string(),
       CONTENT_SETTING_DEFAULT);
   browser()->profile()->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(origin),
       ContentSettingsPattern::FromURLNoWildcard(origin),
       CONTENT_SETTINGS_TYPE_PUSH_MESSAGING,
       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
   // left without permission to use Push.
 
   ASSERT_TRUE(RunScript("hasPermission()", &script_result));
   EXPECT_EQ("permission status - granted", script_result);
 
   ASSERT_TRUE(RunScript("hasRegistration()", &script_result));
   EXPECT_EQ("true - registered", script_result);
 }