Reset the GCM store to recover from it when it is busted

components/gcm_driver/gcm_client_impl.cc

Index: components/gcm_driver/gcm_client_impl.cc
diff --git a/components/gcm_driver/gcm_client_impl.cc b/components/gcm_driver/gcm_client_impl.cc
index 6b1c980f00ff8441153b58afde58881516509f0c..735469f65ce65096a59d9526a56b8f06767d0586 100644
--- a/components/gcm_driver/gcm_client_impl.cc
+++ b/components/gcm_driver/gcm_client_impl.cc
@@ -58,6 +58,15 @@ enum OutgoingMessageTTLCategory {
   TTL_CATEGORY_COUNT
 };
 
+enum ResetStoreError {
+  DESTROYING_STORE_FAILED,
+  INFINITE_STORE_RESET,
+  // NOTE: always keep this entry at the end. Add new value only immediately
+  // above this line. Make sure to update the corresponding histogram enum
+  // accordingly.
+  RESET_STORE_ERROR_COUNT
+};
+
 const int kMaxRegistrationRetries = 5;
 const char kMessageTypeDataMessage[] = "gcm";
 const char kMessageTypeDeletedMessagesKey[] = "deleted_messages";
@@ -245,6 +254,7 @@ GCMClientImpl::GCMClientImpl(scoped_ptr<GCMInternalsBuilder> internals_builder)
       delegate_(NULL),
       start_mode_(DELAYED_START),
       clock_(internals_builder_->BuildClock()),
+      gcm_store_reset_(false),
       url_request_context_getter_(NULL),
       pending_registration_requests_deleter_(&pending_registration_requests_),
       pending_unregistration_requests_deleter_(
@@ -316,9 +326,10 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr<GCMStore::LoadResult> result) {
   DCHECK_EQ(LOADING, state_);
 
   if (!result->success) {
-    ResetState();
+    ResetStore();
     return;
   }
+  gcm_store_reset_ = false;
 
   registrations_ = result->registrations;
   device_checkin_info_.android_id = result->device_android_id;
@@ -426,9 +437,22 @@ void GCMClientImpl::StartMCSLogin() {
                      device_checkin_info_.secret);
 }
 
-void GCMClientImpl::ResetState() {
-  state_ = UNINITIALIZED;
-  // TODO(fgorski): reset all of the necessart objects and start over.
+void GCMClientImpl::ResetStore() {
+  DCHECK_EQ(LOADING, state_);
+
+  // If already being reset, don't do it again. We want to prevent from
+  // resetting and loading from the store again and again.
+  if (gcm_store_reset_) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "GCM.ResetStore", INFINITE_STORE_RESET, RESET_STORE_ERROR_COUNT);

[Alexei Svitkine (slow), 2015/02/10 13:47:06]

Nit: Make a function in the anon namespace for this and pass in the enum. (Each
macro expands to a big boilerplate, so ensuring there's only one per histogram
saves bloat).

[jianli, 2015/02/10 19:45:14]

Done.

+    state_ = UNINITIALIZED;
+    return;
+  }
+  gcm_store_reset_ = true;
+
+  // Destroy the GCM store to start over.
+  gcm_store_->Destroy(base::Bind(&GCMClientImpl::ResetStoreCallback,
+                      weak_ptr_factory_.GetWeakPtr()));
 }
 
 void GCMClientImpl::SetAccountTokens(
@@ -627,6 +651,20 @@ void GCMClientImpl::IgnoreWriteResultCallback(bool success) {
   // sync_intergration_tests are not broken.
 }
 
+void GCMClientImpl::ResetStoreCallback(bool success) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "GCM.ResetStore", DESTROYING_STORE_FAILED, RESET_STORE_ERROR_COUNT);

[Nicolas Zea, 2015/02/10 18:59:46]

Why is this saying we failed? Shouldn't that be dependent on success?

[jianli, 2015/02/10 19:45:14]

My bad. It should be placed within if block.

+
+  if (!success) {
+    LOG(ERROR) << "Failed to reset GCM store";
+    state_ = UNINITIALIZED;
+    return;
+  }
+
+  state_ = INITIALIZED;
+  Start(start_mode_);
+}
+
 void GCMClientImpl::Stop() {
   // TODO(fgorski): Perhaps we should make a distinction between a Stop and a
   // Shutdown.