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

We now always create GCMProfileService that listens to profile
notifications such that GCM checkout logic could be invoked when the
profile is signed out.

We now move the GCM check-in logic from GCMClient::Initialize to GCMClient::CheckIn.

BUG=344031
TEST=new tests added and existing tests updated

Review URL: https://codereview.chromium.org/165993005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253787 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 53 insertions, 43 deletions

diff --git a/google_apis/gcm/engine/mcs_client.cc b/google_apis/gcm/engine/mcs_client.cc
index 75fed0d..7cbd9ae 100644
--- a/google_apis/gcm/engine/mcs_client.cc
+++ b/google_apis/gcm/engine/mcs_client.cc
@@ -337,11 +337,6 @@ void MCSClient::SendMessage(const MCSMessage& message) {
   MaybeSendMessage();
 }
 
-void MCSClient::Destroy() {
-  gcm_store_->Destroy(base::Bind(&MCSClient::OnGCMUpdateFinished,
-                                 weak_ptr_factory_.GetWeakPtr()));
-}
-
 void MCSClient::ResetStateAndBuildLoginRequest(
     mcs_proto::LoginRequest* request) {
   DCHECK(android_id_);
diff --git a/google_apis/gcm/engine/mcs_client.h b/google_apis/gcm/engine/mcs_client.h
index a81643b..f254566 100644
--- a/google_apis/gcm/engine/mcs_client.h
+++ b/google_apis/gcm/engine/mcs_client.h
@@ -122,13 +122,6 @@ class GCM_EXPORT MCSClient {
   // |message_sent_callback_| is invoked with a TTL expiration error.
   virtual void SendMessage(const MCSMessage& message);
 
-  // Disconnects the client and permanently destroys the persistent GCM store.
-  // WARNING: This is permanent, and the client must be recreated with new
-  // credentials afterwards.
-  // TODO(jianli): destroying the persistent GCM store should be moved to
-  // GCMClient.
-  void Destroy();
-
   // Returns the current state of the client.
   State state() const { return state_; }
 
diff --git a/google_apis/gcm/gcm_client.h b/google_apis/gcm/gcm_client.h
index 9280c68..c57cee7 100644
--- a/google_apis/gcm/gcm_client.h
+++ b/google_apis/gcm/gcm_client.h
@@ -132,7 +132,8 @@ class GCM_EXPORT GCMClient {
   GCMClient();
   virtual ~GCMClient();
 
-  // Begins initialization of the GCM Client.
+  // Begins initialization of the GCM Client. This will not trigger a
+  // connection.
   // |chrome_build_proto|: chrome info, i.e., version, channel and etc.
   // |store_path|: path to the GCM store.
   // |account_ids|: account IDs to be related to the device when checking in.
@@ -149,6 +150,10 @@ class GCM_EXPORT GCMClient {
           url_request_context_getter,
       Delegate* delegate) = 0;
 
+  // Loads the data from the persistent store. This will automatically kick off
+  // the check-in if the check-in info is not found in the store.
+  virtual void Load() = 0;
+
   // Checks out of the GCM service. This will erase all the cached and persisted
   // data.
   virtual void CheckOut() = 0;
@@ -178,9 +183,6 @@ class GCM_EXPORT GCMClient {
   virtual void Send(const std::string& app_id,
                     const std::string& receiver_id,
                     const OutgoingMessage& message) = 0;
-
-  // Returns true if GCM becomes ready.
-  virtual bool IsReady() const = 0;
 };
 
 }  // namespace gcm
diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc
index 5c46c60..46b246d 100644
--- a/google_apis/gcm/gcm_client_impl.cc
+++ b/google_apis/gcm/gcm_client_impl.cc
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram.h"
 #include "base/sequenced_task_runner.h"
 #include "base/time/default_clock.h"
 #include "google_apis/gcm/base/mcs_message.h"
@@ -141,30 +142,18 @@ void GCMClientImpl::Initialize(
   account_ids_ = account_ids;
 
   gcm_store_.reset(new GCMStoreImpl(false, path, blocking_task_runner));
-  gcm_store_->Load(base::Bind(&GCMClientImpl::OnLoadCompleted,
-                              weak_ptr_factory_.GetWeakPtr()));
 
   delegate_ = delegate;
 
-  // |mcs_client_| might already be set for testing at this point. No need to
-  // create a |connection_factory_|.
-  if (!mcs_client_.get()) {
-    const net::HttpNetworkSession::Params* network_session_params =
-        url_request_context_getter->GetURLRequestContext()->
-            GetNetworkSessionParams();
-    DCHECK(network_session_params);
-    network_session_ = new net::HttpNetworkSession(*network_session_params);
-    connection_factory_.reset(new ConnectionFactoryImpl(
-        GURL(kMCSEndpoint),
-        kDefaultBackoffPolicy,
-        network_session_,
-        net_log_.net_log()));
-    mcs_client_.reset(new MCSClient(chrome_build_proto.chrome_version(),
-                                    clock_.get(),
-                                    connection_factory_.get(),
-                                    gcm_store_.get()));
-  }
+  state_ = INITIALIZED;
+}
+
+void GCMClientImpl::Load() {
+  DCHECK_EQ(INITIALIZED, state_);
 
+  // Once the loading is completed, the check-in will be initiated.
+  gcm_store_->Load(base::Bind(&GCMClientImpl::OnLoadCompleted,
+                              weak_ptr_factory_.GetWeakPtr()));
   state_ = LOADING;
 }
 
@@ -191,6 +180,25 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr<GCMStore::LoadResult> result) {
 
 void GCMClientImpl::InitializeMCSClient(
     scoped_ptr<GCMStore::LoadResult> result) {
+  // |mcs_client_| might already be set for testing at this point. No need to
+  // create a |connection_factory_|.
+  if (!mcs_client_.get()) {
+    const net::HttpNetworkSession::Params* network_session_params =
+        url_request_context_getter_->GetURLRequestContext()->
+            GetNetworkSessionParams();
+    DCHECK(network_session_params);
+    network_session_ = new net::HttpNetworkSession(*network_session_params);
+    connection_factory_.reset(new ConnectionFactoryImpl(
+        GURL(kMCSEndpoint),
+        kDefaultBackoffPolicy,
+        network_session_,
+        net_log_.net_log()));
+    mcs_client_.reset(new MCSClient(chrome_build_proto_.chrome_version(),
+                                    clock_.get(),
+                                    connection_factory_.get(),
+                                    gcm_store_.get()));
+  }
+
   mcs_client_->Initialize(
       base::Bind(&GCMClientImpl::OnMCSError, weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&GCMClientImpl::OnMessageReceivedFromMCS,
@@ -279,12 +287,13 @@ void GCMClientImpl::SetDeviceCredentialsCallback(bool success) {
 }
 
 void GCMClientImpl::CheckOut() {
-  delegate_ = NULL;
   device_checkin_info_.Reset();
-  mcs_client_->Destroy();  // This will also destroy GCM store.
   mcs_client_.reset();
+  gcm_store_->Destroy(base::Bind(&GCMClientImpl::OnGCMStoreDestroyed,
+                                 weak_ptr_factory_.GetWeakPtr()));
   checkin_request_.reset();
   pending_registrations_.clear();
+  state_ = INITIALIZED;
 }
 
 void GCMClientImpl::Register(const std::string& app_id,
@@ -372,6 +381,11 @@ void GCMClientImpl::OnUnregisterCompleted(const std::string& app_id,
   pending_unregistrations_.erase(iter);
 }
 
+void GCMClientImpl::OnGCMStoreDestroyed(bool success) {
+  DLOG_IF(ERROR, !success) << "GCM store failed to be destroyed!";
+  UMA_HISTOGRAM_BOOLEAN("GCM.StoreDestroySucceeded", success);
+}
+
 void GCMClientImpl::Send(const std::string& app_id,
                          const std::string& receiver_id,
                          const OutgoingMessage& message) {
@@ -399,10 +413,6 @@ void GCMClientImpl::Send(const std::string& app_id,
   mcs_client_->SendMessage(mcs_message);
 }
 
-bool GCMClientImpl::IsReady() const {
-  return state_ == READY;
-}
-
 void GCMClientImpl::OnMessageReceivedFromMCS(const gcm::MCSMessage& message) {
   switch (message.tag()) {
     case kLoginResponseTag:
diff --git a/google_apis/gcm/gcm_client_impl.h b/google_apis/gcm/gcm_client_impl.h
index 9299bd5..72b86ba 100644
--- a/google_apis/gcm/gcm_client_impl.h
+++ b/google_apis/gcm/gcm_client_impl.h
@@ -55,6 +55,7 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
       const scoped_refptr<net::URLRequestContextGetter>&
           url_request_context_getter,
       Delegate* delegate) OVERRIDE;
+  virtual void Load() OVERRIDE;
   virtual void CheckOut() OVERRIDE;
   virtual void Register(const std::string& app_id,
                         const std::string& cert,
@@ -63,13 +64,14 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
   virtual void Send(const std::string& app_id,
                     const std::string& receiver_id,
                     const OutgoingMessage& message) OVERRIDE;
-  virtual bool IsReady() const OVERRIDE;
 
  private:
   // State representation of the GCMClient.
   enum State {
     // Uninitialized.
     UNINITIALIZED,
+    // Initialized,
+    INITIALIZED,
     // GCM store loading is in progress.
     LOADING,
     // Initial device checkin is in progress.
@@ -150,6 +152,9 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
   // Completes the unregistration request.
   void OnUnregisterCompleted(const std::string& app_id, bool status);
 
+  // Completes the GCM store destroy request.
+  void OnGCMStoreDestroyed(bool success);
+
   // Handles incoming data message and dispatches it the a relevant user
   // delegate.
   void HandleIncomingMessage(const gcm::MCSMessage& message);
diff --git a/google_apis/gcm/gcm_client_impl_unittest.cc b/google_apis/gcm/gcm_client_impl_unittest.cc
index 42ecf86..cb8d217c 100644
--- a/google_apis/gcm/gcm_client_impl_unittest.cc
+++ b/google_apis/gcm/gcm_client_impl_unittest.cc
@@ -284,10 +284,15 @@ void GCMClientImplTest::InitializeGCMClient() {
                           message_loop_.message_loop_proxy(),
                           url_request_context_getter_,
                           this);
+
 #if defined(OS_MACOSX)
   // On OSX, prevent the Keychain permissions popup during unit tests.
   Encryptor::UseMockKeychain(true);  // Must be after Initialize.
 #endif
+
+  // Starting loading and check-in.
+  gcm_client_->Load();
+
   // Ensuring that mcs_client is using the same gcm_store as gcm_client.
   mcs_client()->set_gcm_store(gcm_client_->gcm_store_.get());
   PumpLoopUntilIdle();