[GCM] Reset store on checkin rejection and add checkin state to internals

components/gcm_driver/gcm_client_impl_unittest.cc

Index: components/gcm_driver/gcm_client_impl_unittest.cc
diff --git a/components/gcm_driver/gcm_client_impl_unittest.cc b/components/gcm_driver/gcm_client_impl_unittest.cc
index f4e040d86f18901c38ba4a898b91f1c08ef6dde0..6c7ad444653e03fa4cf850d1b100178e90c5276f 100644
--- a/components/gcm_driver/gcm_client_impl_unittest.cc
+++ b/components/gcm_driver/gcm_client_impl_unittest.cc
@@ -280,10 +280,16 @@ class GCMClientImplTest : public testing::Test,
       const std::string& app_id,
       const std::string& message_id,
       const MCSClient::MessageSendStatus status);
+  void FailCheckin(net::HttpStatusCode response_code);
   void CompleteCheckin(uint64_t android_id,
                        uint64_t security_token,
                        const std::string& digest,
                        const std::map<std::string, std::string>& settings);
+  void CompleteCheckinImpl(uint64_t android_id,
+                           uint64_t security_token,
+                           const std::string& digest,
+                           const std::map<std::string, std::string>& settings,
+                           net::HttpStatusCode response_code);
   void CompleteRegistration(const std::string& registration_id);
   void CompleteUnregistration(const std::string& app_id);
   void VerifyPendingRequestFetcherDeleted();
@@ -457,11 +463,27 @@ void GCMClientImplTest::BuildGCMClient(base::TimeDelta clock_step) {
       new FakeGCMInternalsBuilder(clock_step))));
 }
 
+void GCMClientImplTest::FailCheckin(net::HttpStatusCode response_code) {
+  std::map<std::string, std::string> settings;
+  CompleteCheckinImpl(0, 0, GServicesSettings::CalculateDigest(settings),
+                      settings, response_code);
+}
+
 void GCMClientImplTest::CompleteCheckin(
     uint64_t android_id,
     uint64_t security_token,
     const std::string& digest,
     const std::map<std::string, std::string>& settings) {
+  CompleteCheckinImpl(android_id, security_token, digest, settings,
+                      net::HTTP_OK);
+}
+
+void GCMClientImplTest::CompleteCheckinImpl(
+    uint64_t android_id,
+    uint64_t security_token,
+    const std::string& digest,
+    const std::map<std::string, std::string>& settings,
+    net::HttpStatusCode response_code) {
   checkin_proto::AndroidCheckinResponse response;
   response.set_stats_ok(true);
   response.set_android_id(android_id);
@@ -486,7 +508,7 @@ void GCMClientImplTest::CompleteCheckin(
 
   net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
   ASSERT_TRUE(fetcher);
-  fetcher->set_response_code(net::HTTP_OK);
+  fetcher->set_response_code(response_code);
   fetcher->SetResponseString(response_string);
   fetcher->delegate()->OnURLFetchComplete(fetcher);
   // Give a chance for GCMStoreImpl::Backend to finish persisting data.
@@ -1214,6 +1236,25 @@ TEST_F(GCMClientImplCheckinTest, CheckinWhenAccountReplaced) {
             device_checkin_info().account_tokens);
 }
 
+TEST_F(GCMClientImplCheckinTest, ResetStoreWhenCheckinRejected) {
+  std::map<std::string, std::string> settings;
+  ASSERT_NO_FATAL_FAILURE(FailCheckin(net::HTTP_UNAUTHORIZED));
+  PumpLoopUntilIdle();
+
+  // Store should have been destroyed. Restart client and verify the initial
+  // checkin response is persisted.

[Peter Beverloo, 2016/10/28 15:52:30]

fwiw, I've found using the base::HistogramTester an excellent tool in verifying
such "should"s, since we record them as UMAs. See the GCMKeyStoreTest, for
example.

[Nicolas Zea, 2016/11/01 00:18:16]

In this scenario there aren't any new histograms being recorded though. The
checkin has its own histogram for the response, but I'm not sure that's
something we should be verifying here.

Or do you think that we should record a different histogram in this scenario?

[Peter Beverloo, 2016/11/01 15:19:14]

Resetting the store would record to the GCM.ResetStore histogram, right? That
means you'd be able to test it with something like

ASSERT_TRUE(histogram_tester_.ExpectTotalCount("GCM.ResetStore", 1));

Anyway, neither important nor blocking.

+  BuildGCMClient(base::TimeDelta());
+  InitializeGCMClient();
+  StartGCMClient();
+  ASSERT_NO_FATAL_FAILURE(
+      CompleteCheckin(kDeviceAndroidId2, kDeviceSecurityToken2,
+                      GServicesSettings::CalculateDigest(settings), settings));
+
+  EXPECT_EQ(LOADING_COMPLETED, last_event());
+  EXPECT_EQ(kDeviceAndroidId2, mcs_client()->last_android_id());
+  EXPECT_EQ(kDeviceSecurityToken2, mcs_client()->last_security_token());
+}
+
 class GCMClientImplStartAndStopTest : public GCMClientImplTest {
  public:
   GCMClientImplStartAndStopTest();