[GCM] Make sure GCM checkout logic is invoked when the profile is signed out

chrome/browser/services/gcm/gcm_profile_service_unittest.cc

Index: chrome/browser/services/gcm/gcm_profile_service_unittest.cc
diff --git a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc
index 241674e023aefd06c27f371efdc34acca3486441..bfd3d0801ae730fc7134eef12fce31361d41390f 100644
--- a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc
+++ b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc
@@ -98,7 +98,14 @@ class Waiter {
   void OnIOLoopPump() {
     DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
-    base::MessageLoop::current()->RunUntilIdle();
+    content::BrowserThread::PostTask(

[Nicolas Zea, 2014/02/26 02:26:37]

Doesn't RunUntilIdle accomplish the same thing?

[jianli, 2014/02/26 19:04:25]

No. It seems that occasionally some posted tasks are not seen by RunUntilIdle. I
have not figured out why.

Changing to this will make sure all IO tasks posted before this point get
executed. This is indeed what we want.

+        content::BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&Waiter::OnIOLoopPumpCompleted, base::Unretained(this)));
+  }
+
+  void OnIOLoopPumpCompleted() {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
     content::BrowserThread::PostTask(
         content::BrowserThread::UI,
@@ -214,9 +221,9 @@ class FakeGCMEventRouter : public GCMEventRouter {
 class FakeGCMClientFactory : public GCMClientFactory {
  public:
   FakeGCMClientFactory(
-      GCMClientMock::Status gcm_client_initial_status,
+      GCMClientMock::LoadingDelay gcm_client_loading_delay,
       GCMClientMock::ErrorSimulation gcm_client_error_simulation)
-      : gcm_client_initial_status_(gcm_client_initial_status),
+      : gcm_client_loading_delay_(gcm_client_loading_delay),
         gcm_client_error_simulation_(gcm_client_error_simulation),
         gcm_client_(NULL) {
   }
@@ -225,7 +232,7 @@ class FakeGCMClientFactory : public GCMClientFactory {
   }
 
   virtual scoped_ptr<GCMClient> BuildInstance() OVERRIDE {
-    gcm_client_ = new GCMClientMock(gcm_client_initial_status_,
+    gcm_client_ = new GCMClientMock(gcm_client_loading_delay_,
                                     gcm_client_error_simulation_);
     return scoped_ptr<GCMClient>(gcm_client_);
   }
@@ -233,7 +240,7 @@ class FakeGCMClientFactory : public GCMClientFactory {
   GCMClientMock* gcm_client() const { return gcm_client_; }
 
  private:
-  GCMClientMock::Status gcm_client_initial_status_;
+  GCMClientMock::LoadingDelay gcm_client_loading_delay_;
   GCMClientMock::ErrorSimulation gcm_client_error_simulation_;
   GCMClientMock* gcm_client_;
 
@@ -256,7 +263,7 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
       : waiter_(waiter),
         extension_service_(NULL),
         signin_manager_(NULL),
-        gcm_client_initial_status_(GCMClientMock::READY),
+        gcm_client_loading_delay_(GCMClientMock::NO_DELAY_LOADING),
         gcm_client_error_simulation_(GCMClientMock::ALWAYS_SUCCEED),
         registration_result_(GCMClient::SUCCESS),
         has_persisted_registration_info_(false),
@@ -278,7 +285,7 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
     extension_service_ = extension_system->Get(profile())->extension_service();
 
     // Enable GCM such that tests could be run on all channels.
-    GCMProfileService::enable_gcm_for_testing_ = true;
+    profile()->GetPrefs()->SetBoolean(prefs::kGCMChannelEnabled, true);
 
     // Mock a GCMEventRouter.
     gcm_event_router_.reset(new FakeGCMEventRouter(waiter_));
@@ -334,7 +341,7 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
             profile(), &GCMProfileServiceTestConsumer::BuildGCMProfileService));
     gcm_profile_service->set_testing_delegate(this);
     scoped_ptr<GCMClientFactory> gcm_client_factory(
-        new FakeGCMClientFactory(gcm_client_initial_status_,
+        new FakeGCMClientFactory(gcm_client_loading_delay_,
                                  gcm_client_error_simulation_));
     gcm_profile_service->Initialize(gcm_client_factory.Pass());
     waiter_->PumpIOLoop();
@@ -411,8 +418,8 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
   }
 
   GCMClientMock* GetGCMClient() const {
-    return static_cast<FakeGCMClientFactory*>(
-        GetGCMProfileService()->gcm_client_factory_.get())->gcm_client();
+    return static_cast<GCMClientMock*>(
+        GetGCMProfileService()->GetGCMClientForTesting());
   }
 
   const std::string& GetUsername() const {
@@ -432,8 +439,8 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
     return gcm_event_router_.get();
   }
 
-  void set_gcm_client_initial_status(GCMClientMock::Status status) {
-    gcm_client_initial_status_ = status;
+  void set_gcm_client_loading_delay(GCMClientMock::LoadingDelay delay) {
+    gcm_client_loading_delay_ = delay;
   }
   void set_gcm_client_error_simulation(GCMClientMock::ErrorSimulation error) {
     gcm_client_error_simulation_ = error;
@@ -465,7 +472,7 @@ class GCMProfileServiceTestConsumer : public GCMProfileService::TestingDelegate{
   FakeSigninManager* signin_manager_;  // Not owned.
   scoped_ptr<FakeGCMEventRouter> gcm_event_router_;
 
-  GCMClientMock::Status gcm_client_initial_status_;
+  GCMClientMock::LoadingDelay gcm_client_loading_delay_;
   GCMClientMock::ErrorSimulation gcm_client_error_simulation_;
 
   std::string registration_id_;
@@ -584,7 +591,67 @@ TEST_F(GCMProfileServiceTest, CreateGCMProfileServiceAfterProfileSignIn) {
   EXPECT_FALSE(consumer()->GetUsername().empty());
 }
 
-TEST_F(GCMProfileServiceTest, RegsiterWhenNotSignedIn) {
+TEST_F(GCMProfileServiceTest, SignInAndSignOutUnderPositiveChannelSignal) {
+  // Positive channel signal is provided in SetUp.
+  consumer()->CreateGCMProfileServiceInstance();
+  consumer()->SignIn(kTestingUsername);
+
+  // GCMClient should be loaded.
+  EXPECT_TRUE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::LOADED, consumer()->GetGCMClient()->status());
+
+  consumer()->SignOut();
+
+  // GCMClient should be checked out.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::CHECKED_OUT, consumer()->GetGCMClient()->status());
+}
+
+TEST_F(GCMProfileServiceTest, SignInAndSignOutUnderNegativeChannelSignal) {
+  // Negative channel signal will prevent GCMClient from checking in when the
+  // profile is signed in.
+  profile()->GetPrefs()->SetBoolean(prefs::kGCMChannelEnabled, false);
+
+  consumer()->CreateGCMProfileServiceInstance();
+  consumer()->SignIn(kTestingUsername);
+
+  // GCMClient should not be loaded.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::UNINITIALIZED, consumer()->GetGCMClient()->status());
+
+  consumer()->SignOut();
+
+  // Check-out should still be performed.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::CHECKED_OUT, consumer()->GetGCMClient()->status());
+}
+
+TEST_F(GCMProfileServiceTest, SignOutAndThenSignIn) {
+  // Positive channel signal is provided in SetUp.
+  consumer()->CreateGCMProfileServiceInstance();
+  consumer()->SignIn(kTestingUsername);
+
+  // GCMClient should be loaded.
+  EXPECT_TRUE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::LOADED, consumer()->GetGCMClient()->status());
+
+  consumer()->SignOut();
+
+  // GCMClient should be checked out.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::CHECKED_OUT, consumer()->GetGCMClient()->status());
+
+  // Sign-in with a different username.
+  consumer()->SignIn(kTestingUsername2);
+
+  // GCMClient should be loaded again.
+  EXPECT_TRUE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::LOADED, consumer()->GetGCMClient()->status());
+}
+
+TEST_F(GCMProfileServiceTest, RegisterWhenNotSignedIn) {
+  consumer()->CreateGCMProfileServiceInstance();
+
   std::vector<std::string> sender_ids;
   sender_ids.push_back("sender1");
   consumer()->Register(kTestingAppId, sender_ids);
@@ -593,7 +660,38 @@ TEST_F(GCMProfileServiceTest, RegsiterWhenNotSignedIn) {
   EXPECT_EQ(GCMClient::NOT_SIGNED_IN, consumer()->registration_result());
 }
 
+TEST_F(GCMProfileServiceTest, RegisterUnderNeutralChannelSignal) {
+  // Neutral channel signal will prevent GCMClient from checking in when the
+  // profile is signed in.
+  profile()->GetPrefs()->ClearPref(prefs::kGCMChannelEnabled);
+
+  consumer()->CreateGCMProfileServiceInstance();
+  consumer()->SignIn(kTestingUsername);
+
+  // GCMClient should not be checked in.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::UNINITIALIZED, consumer()->GetGCMClient()->status());
+
+  // Invoking register will make GCMClient checked in.
+  std::vector<std::string> sender_ids;
+  sender_ids.push_back("sender1");
+  consumer()->Register(kTestingAppId, sender_ids);
+  WaitUntilCompleted();
+
+  // GCMClient should be checked in.
+  EXPECT_TRUE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::LOADED, consumer()->GetGCMClient()->status());
+
+  // Registration should succeed.
+  std::string expected_registration_id =
+      GCMClientMock::GetRegistrationIdFromSenderIds(sender_ids);
+  EXPECT_EQ(expected_registration_id, consumer()->registration_id());
+  EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
+}
+
 TEST_F(GCMProfileServiceTest, SendWhenNotSignedIn) {
+  consumer()->CreateGCMProfileServiceInstance();
+
   GCMClient::OutgoingMessage message;
   message.id = "1";
   message.data["key1"] = "value1";
@@ -603,6 +701,34 @@ TEST_F(GCMProfileServiceTest, SendWhenNotSignedIn) {
   EXPECT_EQ(GCMClient::NOT_SIGNED_IN, consumer()->send_result());
 }
 
+TEST_F(GCMProfileServiceTest, SendUnderNeutralChannelSignal) {
+  // Neutral channel signal will prevent GCMClient from checking in when the
+  // profile is signed in.
+  profile()->GetPrefs()->ClearPref(prefs::kGCMChannelEnabled);
+
+  consumer()->CreateGCMProfileServiceInstance();
+  consumer()->SignIn(kTestingUsername);
+
+  // GCMClient should not be checked in.
+  EXPECT_FALSE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::UNINITIALIZED, consumer()->GetGCMClient()->status());
+
+  // Invoking send will make GCMClient checked in.
+  GCMClient::OutgoingMessage message;
+  message.id = "1";
+  message.data["key1"] = "value1";
+  consumer()->Send(kTestingAppId, kUserId, message);
+  WaitUntilCompleted();
+
+  // GCMClient should be checked in.
+  EXPECT_TRUE(consumer()->IsGCMClientReady());
+  EXPECT_EQ(GCMClientMock::LOADED, consumer()->GetGCMClient()->status());
+
+  // Sending should succeed.
+  EXPECT_EQ(consumer()->send_message_id(), message.id);
+  EXPECT_EQ(GCMClient::SUCCESS, consumer()->send_result());
+}
+
 // Tests single-profile.
 class GCMProfileServiceSingleProfileTest : public GCMProfileServiceTest {
  public:
@@ -620,29 +746,6 @@ class GCMProfileServiceSingleProfileTest : public GCMProfileServiceTest {
   }
 };
 
-TEST_F(GCMProfileServiceSingleProfileTest, SignOut) {
-  EXPECT_TRUE(consumer()->IsGCMClientReady());
-
-  // This will trigger check-out.
-  consumer()->SignOut();
-
-  EXPECT_FALSE(consumer()->IsGCMClientReady());
-}
-
-TEST_F(GCMProfileServiceSingleProfileTest, SignOutAndThenSignIn) {
-  EXPECT_TRUE(consumer()->IsGCMClientReady());
-
-  // This will trigger check-out.
-  consumer()->SignOut();
-
-  EXPECT_FALSE(consumer()->IsGCMClientReady());
-
-  // Sign-in with a different username.
-  consumer()->SignIn(kTestingUsername2);
-
-  EXPECT_TRUE(consumer()->IsGCMClientReady());
-}
-
 TEST_F(GCMProfileServiceSingleProfileTest, Register) {
   std::vector<std::string> sender_ids;
   sender_ids.push_back("sender1");
@@ -788,10 +891,8 @@ TEST_F(GCMProfileServiceSingleProfileTest,
   // preparation to call register 2nd time.
   consumer()->clear_registration_result();
 
-  // Needs to create a GCMClient instance that is not ready initiallly.
-  consumer()->set_gcm_client_initial_status(GCMClientMock::NOT_READY);
-
   // Simulate start-up by recreating GCMProfileService.
+  consumer()->set_gcm_client_loading_delay(GCMClientMock::DELAY_LOADING);
   consumer()->CreateGCMProfileServiceInstance();
 
   // Simulate start-up by reloading extension.
@@ -805,7 +906,7 @@ TEST_F(GCMProfileServiceSingleProfileTest,
   EXPECT_EQ(GCMClient::UNKNOWN_ERROR, consumer()->registration_result());
 
   // Register operation will be invoked after GCMClient becomes ready.
-  consumer()->GetGCMClient()->SetReady();
+  consumer()->GetGCMClient()->PerformDelayedLoading();
   WaitUntilCompleted();
   EXPECT_EQ(old_registration_id, consumer()->registration_id());
   EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());