[GCM] Reland: Introduce GCMClientFactory to create GCMClient for GCMProfileService

chrome/browser/services/gcm/gcm_profile_service.cc

Index: chrome/browser/services/gcm/gcm_profile_service.cc
diff --git a/chrome/browser/services/gcm/gcm_profile_service.cc b/chrome/browser/services/gcm/gcm_profile_service.cc
index 3afea2173d8d888f726db7c801a655b191af6703..0e1b45d7af7798f727c08341310b7342b46480b3 100644
--- a/chrome/browser/services/gcm/gcm_profile_service.cc
+++ b/chrome/browser/services/gcm/gcm_profile_service.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/state_store.h"
+#include "chrome/browser/services/gcm/gcm_client_factory.h"
 #include "chrome/browser/services/gcm/gcm_event_router.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -179,10 +180,12 @@ class GCMProfileService::IOWorker
     : public GCMClient::Delegate,
       public base::RefCountedThreadSafe<GCMProfileService::IOWorker>{
  public:
+  // Called on UI thread.
   explicit IOWorker(const base::WeakPtr<GCMProfileService>& service);
+  void Initialize();
 
   // Overridden from GCMClient::Delegate:
-  // Called from IO thread.
+  // Called on IO thread.
   virtual void OnCheckInFinished(const GCMClient::CheckinInfo& checkin_info,
                                  GCMClient::Result result) OVERRIDE;
   virtual void OnRegisterFinished(const std::string& app_id,
@@ -200,7 +203,6 @@ class GCMProfileService::IOWorker
                                   GCMClient::Result result) OVERRIDE;
   virtual GCMClient::CheckinInfo GetCheckinInfo() const OVERRIDE;
   virtual void OnLoadingCompleted() OVERRIDE;
-  virtual base::TaskRunner* GetFileTaskRunner() OVERRIDE;
 
   void CheckGCMClientLoading();
   void SetUser(const std::string& username);
@@ -221,8 +223,13 @@ class GCMProfileService::IOWorker
   friend class base::RefCountedThreadSafe<IOWorker>;
   virtual ~IOWorker();
 
+  void InitializeGCMClient();
+
   const base::WeakPtr<GCMProfileService> service_;
 
+  // Not owned.
+  GCMClient* gcm_client_;
+
   // The username (email address) of the signed-in user.
   std::string username_;
 
@@ -233,12 +240,29 @@ class GCMProfileService::IOWorker
 
 GCMProfileService::IOWorker::IOWorker(
     const base::WeakPtr<GCMProfileService>& service)
-    : service_(service) {
+    : service_(service),
+      gcm_client_(NULL) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 }
 
 GCMProfileService::IOWorker::~IOWorker() {
 }
 
+void GCMProfileService::IOWorker::Initialize() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&GCMProfileService::IOWorker::InitializeGCMClient, this));
+}
+
+void GCMProfileService::IOWorker::InitializeGCMClient() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  gcm_client_ = GCMClientFactory::GetClient();
+}
+
 void GCMProfileService::IOWorker::OnCheckInFinished(
     const GCMClient::CheckinInfo& checkin_info,
     GCMClient::Result result) {
@@ -340,18 +364,13 @@ void GCMProfileService::IOWorker::OnLoadingCompleted() {
                  service_));
 }
 
-base::TaskRunner* GCMProfileService::IOWorker::GetFileTaskRunner() {
-  // TODO(jianli): to be implemented.
-  return NULL;
-}
-
 void GCMProfileService::IOWorker::CheckGCMClientLoading() {
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&GCMProfileService::CheckGCMClientLoadingFinished,
                  service_,
-                 GCMClient::Get()->IsLoading()));
+                 gcm_client_->IsLoading()));
 }
 
 void GCMProfileService::IOWorker::SetUser(const std::string& username) {
@@ -359,7 +378,7 @@ void GCMProfileService::IOWorker::SetUser(const std::string& username) {
   DCHECK(username_.empty() && !username.empty());
 
   username_ = username;
-  GCMClient::Get()->SetUserDelegate(username_, this);
+  gcm_client_->SetUserDelegate(username_, this);
 }
 
 void GCMProfileService::IOWorker::RemoveUser(const std::string& username) {
@@ -369,13 +388,13 @@ void GCMProfileService::IOWorker::RemoveUser(const std::string& username) {
   if (username_.empty())
     return;
   username_.clear();
-  GCMClient::Get()->SetUserDelegate(username_, NULL);
+  gcm_client_->SetUserDelegate(username_, NULL);
 }
 
 void GCMProfileService::IOWorker::CheckIn() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
-  GCMClient::Get()->CheckIn(username_);
+  gcm_client_->CheckIn(username_);
 }
 
 void GCMProfileService::IOWorker::SetCheckinInfo(
@@ -399,14 +418,14 @@ void GCMProfileService::IOWorker::Register(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK(!username_.empty() && checkin_info_.IsValid());
 
-  GCMClient::Get()->Register(username_, app_id, cert, sender_ids);
+  gcm_client_->Register(username_, app_id, cert, sender_ids);
 }
 
 void GCMProfileService::IOWorker::Unregister(const std::string& app_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK(!username_.empty() && checkin_info_.IsValid());
 
-  GCMClient::Get()->Unregister(username_, app_id);
+  gcm_client_->Unregister(username_, app_id);
 }
 
 void GCMProfileService::IOWorker::Send(
@@ -416,7 +435,7 @@ void GCMProfileService::IOWorker::Send(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK(!username_.empty() && checkin_info_.IsValid());
 
-  GCMClient::Get()->Send(username_, app_id, receiver_id, message);
+  gcm_client_->Send(username_, app_id, receiver_id, message);
 }
 
 GCMProfileService::RegistrationInfo::RegistrationInfo() {
@@ -465,6 +484,7 @@ GCMProfileService::GCMProfileService(Profile* profile)
       testing_delegate_(NULL),
       weak_ptr_factory_(this) {
   DCHECK(!profile->IsOffTheRecord());
+
   Init();
 }
 
@@ -492,6 +512,7 @@ void GCMProfileService::Init() {
 
   // This has to be done first since CheckIn depends on it.
   io_worker_ = new IOWorker(weak_ptr_factory_.GetWeakPtr());
+  io_worker_->Initialize();

[Nicolas Zea, 2014/01/21 23:04:45]

I think it's cleaner to have the GCMProfileService do:
PostTask(IO, ..., base::Bind(..IOWoker::Initialize, io_worker_))

and make replace Initialize's implementation with InitializeGCMClient. That way
the IOWorker truly is an IO worker, including the initialize code.

[jianli, 2014/01/21 23:16:16]

Good idea. Also moved the logic defined in CheckGCMClientLoading to
IOWorker::Initialize and remove the following task posting.

 
   // Checks if GCMClient has finished the loading.
   content::BrowserThread::PostTask(