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

chrome/browser/services/gcm/gcm_profile_service.cc

 // Copyright (c) 2013 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 "chrome/browser/services/gcm/gcm_profile_service.h"
 
 #include <string>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 218 matching lines @@
   for (size_t i = 0; i < task_queue->tasks.size(); ++i)
     task_queue->tasks[i].Run();
   task_queue->tasks.clear();
 }
 
 class GCMProfileService::IOWorker
     : public GCMClient::Delegate,
       public base::RefCountedThreadSafe<GCMProfileService::IOWorker>{
  public:
   // Called on UI thread.
-  explicit IOWorker(const base::WeakPtr<GCMProfileService>& service);
+  IOWorker();
 
   // Overridden from GCMClient::Delegate:
   // Called on IO thread.
   virtual void OnRegisterFinished(const std::string& app_id,
                                   const std::string& registration_id,
                                   GCMClient::Result result) OVERRIDE;
   virtual void OnUnregisterFinished(const std::string& app_id,
                                     bool success) OVERRIDE;
   virtual void OnSendFinished(const std::string& app_id,
                               const std::string& message_id,
                               GCMClient::Result result) OVERRIDE;
   virtual void OnMessageReceived(
       const std::string& app_id,
       const GCMClient::IncomingMessage& message) OVERRIDE;
   virtual void OnMessagesDeleted(const std::string& app_id) OVERRIDE;
   virtual void OnMessageSendError(const std::string& app_id,
                                   const std::string& message_id,
                                   GCMClient::Result result) OVERRIDE;
   virtual void OnGCMReady() OVERRIDE;
 
   // Called on IO thread.
-  void Initialize(GCMClientFactory* gcm_client_factory,
+  void Initialize(scoped_ptr<GCMClientFactory> gcm_client_factory,
                   const base::FilePath& store_path,
                   const std::vector<std::string>& account_ids,
                   const scoped_refptr<net::URLRequestContextGetter>&
                       url_request_context_getter);
   void Reset();
+  void Load(const base::WeakPtr<GCMProfileService>& service);
   void CheckOut();
   void Register(const std::string& app_id,
                 const std::vector<std::string>& sender_ids,
                 const std::string& cert);
   void Unregister(const std::string& app_id);
   void Send(const std::string& app_id,
             const std::string& receiver_id,
             const GCMClient::OutgoingMessage& message);
 
+  // For testing purpose. Can be called from UI thread. Use with care.
+  GCMClient* gcm_client_for_testing() const { return gcm_client_.get(); }
+
  private:
   friend class base::RefCountedThreadSafe<IOWorker>;
   virtual ~IOWorker();
 
-  const base::WeakPtr<GCMProfileService> service_;
+  base::WeakPtr<GCMProfileService> service_;
 
   scoped_ptr<GCMClient> gcm_client_;
 };
 
-GCMProfileService::IOWorker::IOWorker(
-    const base::WeakPtr<GCMProfileService>& service)
-    : service_(service) {
+GCMProfileService::IOWorker::IOWorker() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 }
 
 GCMProfileService::IOWorker::~IOWorker() {
 }
 
 void GCMProfileService::IOWorker::Initialize(
-    GCMClientFactory* gcm_client_factory,
+    scoped_ptr<GCMClientFactory> gcm_client_factory,
     const base::FilePath& store_path,
     const std::vector<std::string>& account_ids,
     const scoped_refptr<net::URLRequestContextGetter>&
         url_request_context_getter) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   gcm_client_ = gcm_client_factory->BuildInstance().Pass();
 
   checkin_proto::ChromeBuildProto chrome_build_proto;
   chrome_build_proto.set_platform(GetPlatform());
   chrome_build_proto.set_chrome_version(GetVersion());
   chrome_build_proto.set_channel(GetChannel());
 
   scoped_refptr<base::SequencedWorkerPool> worker_pool(
       content::BrowserThread::GetBlockingPool());
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner(
       worker_pool->GetSequencedTaskRunnerWithShutdownBehavior(
           worker_pool->GetSequenceToken(),
           base::SequencedWorkerPool::SKIP_ON_SHUTDOWN));
 
   gcm_client_->Initialize(chrome_build_proto,
                           store_path,
                           account_ids,
                           blocking_task_runner,
                           url_request_context_getter,
                           this);
-
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&GCMProfileService::FinishInitializationOnUI,
-                 service_,
-                 gcm_client_->IsReady()));
 }
 
 void GCMProfileService::IOWorker::Reset() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   // GCMClient instance must be destroyed from the same thread where it was
   // created.
   gcm_client_.reset();
 }
 
@@ skipping 77 matching lines @@
 }
 
 void GCMProfileService::IOWorker::OnGCMReady() {
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&GCMProfileService::GCMClientReady,
                  service_));
 }
 
+void GCMProfileService::IOWorker::Load(
+    const base::WeakPtr<GCMProfileService>& service) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  service_ = service;
+  gcm_client_->Load();
+}
+
 void GCMProfileService::IOWorker::CheckOut() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   gcm_client_->CheckOut();
-  gcm_client_.reset();
+
+  // Note that we still need to keep GCMClient instance alive since the profile
+  // might be signed in again.
 }
 
 void GCMProfileService::IOWorker::Register(
     const std::string& app_id,
     const std::vector<std::string>& sender_ids,
     const std::string& cert) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   gcm_client_->Register(app_id, cert, sender_ids);
 }
@@ skipping 16 matching lines @@
 GCMProfileService::RegistrationInfo::RegistrationInfo() {
 }
 
 GCMProfileService::RegistrationInfo::~RegistrationInfo() {
 }
 
 bool GCMProfileService::RegistrationInfo::IsValid() const {
   return !sender_ids.empty() && !registration_id.empty();
 }
 
-bool GCMProfileService::enable_gcm_for_testing_ = false;
+// static
+GCMProfileService::GCMEnabledState GCMProfileService::GetGCMEnabledState(
+    Profile* profile) {
+  const base::Value* gcm_enabled_value =
+      profile->GetPrefs()->GetUserPrefValue(prefs::kGCMChannelEnabled);
+  if (!gcm_enabled_value)
+    return ENABLED_FOR_APPS;
 
-// static
-bool GCMProfileService::IsGCMEnabled(Profile* profile) {
-  // GCM is not enabled in incognito mode.
-  if (profile->IsOffTheRecord())
-    return false;
+  bool gcm_enabled = false;
+  if (!gcm_enabled_value->GetAsBoolean(&gcm_enabled))
+    return ENABLED_FOR_APPS;
 
-  if (enable_gcm_for_testing_)
-    return true;
-
-  return profile->GetPrefs()->GetBoolean(prefs::kGCMChannelEnabled);
+  return gcm_enabled ? ALWAYS_ENABLED : ALWAYS_DISABLED;
 }
 
 // static
 void GCMProfileService::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   // GCM support is only enabled by default for Canary/Dev/Custom builds.
   chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
   bool on_by_default = false;
   if (channel == chrome::VersionInfo::CHANNEL_UNKNOWN ||
       channel == chrome::VersionInfo::CHANNEL_CANARY ||
@@ skipping 15 matching lines @@
       testing_delegate_(NULL),
       weak_ptr_factory_(this) {
   DCHECK(!profile->IsOffTheRecord());
 }
 
 GCMProfileService::~GCMProfileService() {
 }
 
 void GCMProfileService::Initialize(
     scoped_ptr<GCMClientFactory> gcm_client_factory) {
-  gcm_client_factory_ = gcm_client_factory.Pass();
-
-  // This has to be done first since CheckIn depends on it.
-  io_worker_ = new IOWorker(weak_ptr_factory_.GetWeakPtr());
-
-#if !defined(OS_ANDROID)
-  js_event_router_.reset(new extensions::GcmJsEventRouter(profile_));
-#endif
-
   registrar_.Add(this,
                  chrome::NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL,
                  content::Source<Profile>(profile_));
   registrar_.Add(this,
                  chrome::NOTIFICATION_GOOGLE_SIGNED_OUT,
                  content::Source<Profile>(profile_));
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::Source<Profile>(profile_));
   // TODO(jianli): move extension specific logic out of GCMProfileService.
   registrar_.Add(this,
                  chrome:: NOTIFICATION_EXTENSION_UNINSTALLED,
                  content::Source<Profile>(profile_));
 
-  // In case that the profile has been signed in before GCMProfileService is
-  // created.
-  SigninManagerBase* manager = SigninManagerFactory::GetForProfile(profile_);
-  if (manager) {
-    std::string username = manager->GetAuthenticatedUsername();
-    if (!username.empty())
-      CheckIn(username);
-  }
+  // Get the list of available accounts.
+  std::vector<std::string> account_ids;
+#if !defined(OS_ANDROID)
+  account_ids =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->GetAccounts();
+#endif
+
+  // Create and initialize the GCMClient. Note that this does not initiate the
+  // GCM check-in.
+  io_worker_ = new IOWorker();
+  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter =

[SeRya, 2014/03/04 08:35:26]

I tried to use ProfileSyncService on startup and got the following failure:

[10007:10007:0304/122239:FATAL:profile_impl.cc(991)] Check failed:
ssl_config_service_manager_. SSLConfigServiceManager is not initialized yet
 [0x7f0417dda49e] base::debug::StackTrace::StackTrace()
 [0x7f0417e320a5] logging::LogMessage::~LogMessage()
 [0x7f041bc2ae94] ProfileImpl::GetSSLConfigService()
 [0x7f041b6c452e] ProfileIOData::InitializeOnUIThread()
 [0x7f041bc300dc] ProfileImplIOData::Handle::LazyInitialize()
 [0x7f041bc2fefd] ProfileImplIOData::Handle::GetResourceContext()
 [0x7f041bc2acfc] ProfileImpl::GetResourceContext()
 [0x7f041d1da911] content::StoragePartitionImplMap::Get()
 [0x7f041cde3ccf] content::(anonymous
namespace)::GetStoragePartitionFromConfig()
 [0x7f041cde3c3a] content::BrowserContext::GetStoragePartition()
 [0x7f041cde3e1f] content::BrowserContext::GetDefaultStoragePartition()
 [0x7f041bc2a945] ProfileImpl::GetRequestContext()
 [0x7f041c24c375] gcm::GCMProfileService::Initialize()
 [0x7f041bc76236] gcm::GCMProfileServiceFactory::BuildServiceInstanceFor()
 [0x7f041e33dd45]
BrowserContextKeyedServiceFactory::GetServiceForBrowserContext()
 [0x7f041bc760c7] gcm::GCMProfileServiceFactory::GetForProfile()
 [0x7f041b7efd21] ProfileSyncServiceFactory::BuildServiceInstanceFor()
 [0x7f041e33dd45]
BrowserContextKeyedServiceFactory::GetServiceForBrowserContext()
 [0x7f041b7efa8f] ProfileSyncServiceFactory::GetForProfile()

There is a comment at the DCHEK:

  // If ssl_config_service_manager_ is null, this typically means that some
  // BrowserContextKeyedService is trying to create a RequestContext at startup,
  // but SSLConfigServiceManager is not initialized until DoFinalInit() which is
  // invoked after all BrowserContextKeyedServices have been initialized (see
  // http://crbug.com/171406).

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pro...

Doesn't it make sense to avoid using profile_->GetRequestContext() in service
initialization?

[jianli, 2014/03/05 01:37:34]

I can't repro this. Could you please share how you get this? OS? Version?

+      profile_->GetRequestContext();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&GCMProfileService::IOWorker::Initialize,
+                 io_worker_,
+                 base::Passed(&gcm_client_factory),
+                 profile_->GetPath().Append(chrome::kGCMStoreDirname),
+                 account_ids,
+                 url_request_context_getter));
+
+  // Load from the GCM store and initiate the GCM check-in if the rollout signal
+  // indicates yes.
+  if (GetGCMEnabledState(profile_) == ALWAYS_ENABLED)
+    EnsureLoaded();
 }
 
 void GCMProfileService::Register(const std::string& app_id,
                                  const std::vector<std::string>& sender_ids,
                                  const std::string& cert,
                                  RegisterCallback callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(!app_id.empty() && !sender_ids.empty() && !callback.is_null());
 
+  // Ensure that check-in has been done.
+  EnsureLoaded();
+
   // If the profile was not signed in, bail out.
   if (username_.empty()) {
     callback.Run(std::string(), GCMClient::NOT_SIGNED_IN);
     return;
   }
 
   // If previous register operation is still in progress, bail out.
   if (register_callbacks_.find(app_id) != register_callbacks_.end()) {
     callback.Run(std::string(), GCMClient::ASYNC_OPERATION_PENDING);
     return;
@@ skipping 65 matching lines @@
                  cert));
 }
 
 void GCMProfileService::Send(const std::string& app_id,
                              const std::string& receiver_id,
                              const GCMClient::OutgoingMessage& message,
                              SendCallback callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(!app_id.empty() && !receiver_id.empty() && !callback.is_null());
 
+  // Ensure that check-in has been done.
+  EnsureLoaded();
+
   // If the profile was not signed in, bail out.
   if (username_.empty()) {
     callback.Run(std::string(), GCMClient::NOT_SIGNED_IN);
     return;
   }
 
   // If the message with send ID is still in progress, bail out.
   std::pair<std::string, std::string> key(app_id, message.id);
   if (send_callbacks_.find(key) != send_callbacks_.end()) {
     callback.Run(message.id, GCMClient::INVALID_PARAMETER);
@@ skipping 24 matching lines @@
   content::BrowserThread::PostTask(
       content::BrowserThread::IO,
       FROM_HERE,
       base::Bind(&GCMProfileService::IOWorker::Send,
                  io_worker_,
                  app_id,
                  receiver_id,
                  message));
 }
 
+GCMClient* GCMProfileService::GetGCMClientForTesting() const {
+  return io_worker_ ? io_worker_->gcm_client_for_testing() : NULL;
+}
+
 void GCMProfileService::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   switch (type) {
     case chrome::NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL: {
-      const GoogleServiceSigninSuccessDetails* signin_details =
-          content::Details<GoogleServiceSigninSuccessDetails>(details).ptr();
-      // This could be called multiple times when the password changed.
-      if (username_ != signin_details->username)
-        CheckIn(signin_details->username);
+      if (GetGCMEnabledState(profile_) == ALWAYS_ENABLED)
+        EnsureLoaded();
       break;
     }
     case chrome::NOTIFICATION_GOOGLE_SIGNED_OUT:
       CheckOut();
       break;
     case chrome::NOTIFICATION_PROFILE_DESTROYED:
       ResetGCMClient();
       break;
-    case chrome:: NOTIFICATION_EXTENSION_UNINSTALLED: {
-      extensions::Extension* extension =
-          content::Details<extensions::Extension>(details).ptr();
-      Unregister(extension->id());
+    case chrome:: NOTIFICATION_EXTENSION_UNINSTALLED:
+      if (!username_.empty()) {
+        extensions::Extension* extension =
+            content::Details<extensions::Extension>(details).ptr();
+        Unregister(extension->id());
+      }
       break;
-    }
     default:
       NOTREACHED();
   }
 }
 
-void GCMProfileService::CheckIn(const std::string& username) {
-  DCHECK(!username.empty() && username_.empty());
+void GCMProfileService::EnsureLoaded() {
+  SigninManagerBase* manager = SigninManagerFactory::GetForProfile(profile_);
+  if (!manager)
+    return;
+  std::string username = manager->GetAuthenticatedUsername();
+  if (username.empty())
+    return;
+
+  // CheckIn could be called more than once when:
+  // 1) The password changes.
+  // 2) Register/send function calls it to ensure CheckIn is done.
+  if (username_ == username)
+    return;
   username_ = username;
 
+#if !defined(OS_ANDROID)
+  if (!js_event_router_)
+    js_event_router_.reset(new extensions::GcmJsEventRouter(profile_));
+#endif
+
   DCHECK(!delayed_task_controller_);
   delayed_task_controller_.reset(new DelayedTaskController);
 
-  // Load all register apps.
+  // Load all the registered apps.
   ReadRegisteredAppIDs();
 
-  // Get the list of available accounts.
-  std::vector<std::string> account_ids;
-#if !defined(OS_ANDROID)
-  account_ids =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->GetAccounts();
-#endif
-
-  // Let the IO thread create and initialize GCMClient.
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter =
-      profile_->GetRequestContext();
+  // This will load the data from the gcm store and trigger the check-in if
+  // the persisted check-in info is not found.
+  // Note that we need to pass weak pointer again since the existing weak
+  // pointer in IOWorker might have been invalidated when check-out occurs.
   content::BrowserThread::PostTask(
       content::BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&GCMProfileService::IOWorker::Initialize,
+      base::Bind(&GCMProfileService::IOWorker::Load,
                  io_worker_,
-                 gcm_client_factory_.get(),
-                 profile_->GetPath().Append(chrome::kGCMStoreDirname),
-                 account_ids,
-                 url_request_context_getter));
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void GCMProfileService::CheckOut() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  DCHECK(!username_.empty());
+  // We still proceed with the check-out logic even if the check-in is not
+  // initiated in the current session. This will make sure that all the
+  // persisted data written previously will get purged.
   username_.clear();
 
+  // Remove all the queued tasks since they no longer make sense after
+  // check-out.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   // Remove persisted data from app's state store.
   for (RegistrationInfoMap::const_iterator iter =
            registration_info_map_.begin();
        iter != registration_info_map_.end(); ++iter) {
     DeleteRegistrationInfo(iter->first);
   }
 
   // Remove persisted data from prefs store.
-  profile_->GetPrefs()->ClearPref(prefs::kGCMChannelEnabled);
   profile_->GetPrefs()->ClearPref(prefs::kGCMRegisteredAppIDs);
 
   gcm_client_ready_ = false;
   delayed_task_controller_.reset();
   register_callbacks_.clear();
   send_callbacks_.clear();
   registration_info_map_.clear();
 
   content::BrowserThread::PostTask(
       content::BrowserThread::IO,
@@ skipping 125 matching lines @@
                                          GCMClient::Result result) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // Drop the event if signed out.
   if (username_.empty())
     return;
 
   GetEventRouter(app_id)->OnSendError(app_id, message_id, result);
 }
 
-void GCMProfileService::FinishInitializationOnUI(bool ready) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  gcm_client_ready_ = ready;
-  if (gcm_client_ready_)
-    delayed_task_controller_->SetGCMReady();
-}
-
 void GCMProfileService::GCMClientReady() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   if (gcm_client_ready_)
     return;
   gcm_client_ready_ = true;
 
   delayed_task_controller_->SetGCMReady();
 }
 
@@ skipping 119 matching lines @@
 
   return true;
 }
 
 // static
 const char* GCMProfileService::GetPersistentRegisterKeyForTesting() {
   return kRegistrationKey;
 }
 
 }  // namespace gcm