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

components/gcm_driver/gcm_client_impl.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 "components/gcm_driver/gcm_client_impl.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"

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

Nit: Change this to histogram_macros.h instead.

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

Done.

 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/default_clock.h"
 #include "base/timer/timer.h"
 #include "components/gcm_driver/gcm_account_mapper.h"
 #include "components/gcm_driver/gcm_backoff_policy.h"
 #include "google_apis/gcm/base/encryptor.h"
 #include "google_apis/gcm/base/mcs_message.h"
@@ skipping 28 matching lines @@
   TTL_LESS_THAN_OR_EQUAL_TO_ONE_DAY,
   TTL_LESS_THAN_OR_EQUAL_TO_ONE_WEEK,
   TTL_MORE_THAN_ONE_WEEK,
   TTL_MAXIMUM,
   // NOTE: always keep this entry at the end. Add new TTL category only
   // immediately above this line. Make sure to update the corresponding
   // histogram enum accordingly.
   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";
 const char kMessageTypeKey[] = "message_type";
 const char kMessageTypeSendErrorKey[] = "send_error";
 const char kSendErrorMessageIdKey[] = "google.message_id";
 const char kSendMessageFromValue[] = "gcm@chrome.com";
 const int64 kDefaultUserSerialNumber = 0LL;
 
 GCMClient::Result ToGCMClientResult(MCSClient::MessageSendStatus status) {
@@ skipping 167 matching lines @@
   account_tokens.clear();
   last_checkin_accounts.clear();
 }
 
 GCMClientImpl::GCMClientImpl(scoped_ptr<GCMInternalsBuilder> internals_builder)
     : internals_builder_(internals_builder.Pass()),
       state_(UNINITIALIZED),
       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_(
           &pending_unregistration_requests_),
       periodic_checkin_ptr_factory_(this),
       weak_ptr_factory_(this) {
 }
 
 GCMClientImpl::~GCMClientImpl() {
 }
@@ skipping 51 matching lines @@
   // Once the loading is completed, the check-in will be initiated.
   gcm_store_->Load(base::Bind(&GCMClientImpl::OnLoadCompleted,
                               weak_ptr_factory_.GetWeakPtr()));
   state_ = LOADING;
 }
 
 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;
   device_checkin_info_.secret = result->device_security_token;
   device_checkin_info_.last_checkin_accounts = result->last_checkin_accounts;
   // A case where there were previously no accounts reported with checkin is
   // considered to be the same as when the list of accounts is empty. It enables
   // scheduling a periodic checkin for devices with no signed in users
   // immediately after restart, while keeping |accounts_set == false| delays the
   // checkin until the list of accounts is set explicitly.
@@ skipping 87 matching lines @@
   delegate_->OnGCMReady(account_mappings, last_token_fetch_time);
 }
 
 void GCMClientImpl::StartMCSLogin() {
   DCHECK_EQ(READY, state_);
   DCHECK(device_checkin_info_.IsValid());
   mcs_client_->Login(device_checkin_info_.android_id,
                      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(
     const std::vector<AccountTokenInfo>& account_tokens) {
   device_checkin_info_.account_tokens.clear();
   for (std::vector<AccountTokenInfo>::const_iterator iter =
            account_tokens.begin();
        iter != account_tokens.end();
        ++iter) {
     device_checkin_info_.account_tokens[iter->email] = iter->access_token;
@@ skipping 178 matching lines @@
 
 void GCMClientImpl::DefaultStoreCallback(bool success) {
   DCHECK(success);
 }
 
 void GCMClientImpl::IgnoreWriteResultCallback(bool success) {
   // TODO(fgorski): Ignoring the write result for now to make sure
   // 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.
   DVLOG(1) << "Stopping the GCM Client";
   weak_ptr_factory_.InvalidateWeakPtrs();
   periodic_checkin_ptr_factory_.InvalidateWeakPtrs();
   device_checkin_info_.Reset();
   connection_factory_.reset();
   delegate_->OnDisconnected();
   mcs_client_.reset();
@@ skipping 380 matching lines @@
 bool GCMClientImpl::HasStandaloneRegisteredApp() const {
   if (registrations_.empty())
     return false;
   // Note that account mapper is not counted as a standalone app since it is
   // automatically started when other app uses GCM.
   return registrations_.size() > 1 ||
          !registrations_.count(kGCMAccountMapperAppId);
 }
 
 }  // namespace gcm