Move IdentityProvider usage from GCMDriverDesktop to GCMProfileService

This is the 1st step towards removing sign-in enforcement for GCM.

We also remove the logic to pass account IDs to check-in request
since it seems to work fine without them and TokenService::GetAccounts
returns the empty list.

BUG=384041
TEST=existing and new tests
R=bartfab@chromium.org, fgorski@chromium.org, zea@chromium.org
TBR=jochen@chromium.org, rogerta@chromium.org

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

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

chrome/browser/browser_process_impl.cc

@@ -91,7 +91,6 @@
 #include "content/public/browser/storage_partition.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_l10n_util.h"
-#include "google_apis/gaia/identity_provider.h"
 #include "net/socket/client_socket_pool_manager.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -116,13 +115,6 @@
 #include "components/storage_monitor/storage_monitor.h"
 #endif
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/settings/device_identity_provider.h"
-#include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
-#elif !defined(OS_ANDROID)
-#include "google_apis/gaia/dummy_identity_provider.h"
-#endif
-
 #if defined(USE_AURA)
 #include "ui/aura/env.h"
 #endif
@@ -1026,14 +1018,13 @@ void BrowserProcessImpl::CreateGCMDriver() {
   CHECK(PathService::Get(chrome::DIR_GLOBAL_GCM_STORE, &store_path));
   gcm_driver_ = gcm::CreateGCMDriverDesktop(
       make_scoped_ptr(new gcm::GCMClientFactory),
-#if defined(OS_CHROMEOS)
-      scoped_ptr<IdentityProvider>(new chromeos::DeviceIdentityProvider(
-          chromeos::DeviceOAuth2TokenServiceFactory::Get())),
-#else
-      scoped_ptr<IdentityProvider>(new DummyIdentityProvider),
-#endif  // defined(OS_CHROMEOS)
       store_path,
       system_request_context());
+  // Sign-in is not required for device-level GCM usage. So we just call
+  // OnSignedIn to assume always signed-in. Note that GCM will not be started
+  // at this point since no one has asked for it yet.
+  // TODO(jianli): To be removed when sign-in enforcement is dropped.
+  gcm_driver_->OnSignedIn();
 #endif  // defined(OS_ANDROID)
 }
 

chrome/browser/services/gcm/gcm_desktop_utils.cc

@@ -73,7 +73,6 @@ GCMClient::ChromeBuildInfo GetChromeBuildInfo() {
 
 scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
     scoped_ptr<GCMClientFactory> gcm_client_factory,
-    scoped_ptr<IdentityProvider> identity_provider,
     const base::FilePath& store_path,
     const scoped_refptr<net::URLRequestContextGetter>& request_context) {
   scoped_refptr<base::SequencedWorkerPool> worker_pool(
@@ -84,7 +83,6 @@ scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
           base::SequencedWorkerPool::SKIP_ON_SHUTDOWN));
   return scoped_ptr<GCMDriver>(new GCMDriverDesktop(
       gcm_client_factory.Pass(),
-      identity_provider.Pass(),
       GetChromeBuildInfo(),
       store_path,
       request_context,

chrome/browser/services/gcm/gcm_desktop_utils.h

@@ -8,8 +8,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 
-class IdentityProvider;
-
 namespace base {
 class FilePath;
 }
@@ -25,7 +23,6 @@ class GCMClientFactory;
 
 scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
     scoped_ptr<GCMClientFactory> gcm_client_factory,
-    scoped_ptr<IdentityProvider> identity_provider,
     const base::FilePath& store_path,
     const scoped_refptr<net::URLRequestContextGetter>& request_context);
 

chrome/browser/services/gcm/gcm_profile_service.cc

@@ -28,6 +28,64 @@
 
 namespace gcm {
 
+#if !defined(OS_ANDROID)
+class GCMProfileService::IdentityObserver : public IdentityProvider::Observer {
+ public:
+  IdentityObserver(Profile* profile, GCMDriver* driver);
+  virtual ~IdentityObserver();
+
+  // IdentityProvider::Observer:
+  virtual void OnActiveAccountLogin() OVERRIDE;
+  virtual void OnActiveAccountLogout() OVERRIDE;
+
+  std::string SignedInUserName() const;
+
+ private:
+  GCMDriver* driver_;
+  scoped_ptr<IdentityProvider> identity_provider_;
+
+  // The account ID that this service is responsible for. Empty when the service
+  // is not running.
+  std::string account_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(IdentityObserver);
+};
+
+GCMProfileService::IdentityObserver::IdentityObserver(Profile* profile,
+                                                      GCMDriver* driver)
+    : driver_(driver) {
+  identity_provider_.reset(new ProfileIdentityProvider(
+      SigninManagerFactory::GetForProfile(profile),
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
+      LoginUIServiceFactory::GetForProfile(profile)));
+  identity_provider_->AddObserver(this);
+
+  OnActiveAccountLogin();
+}
+
+GCMProfileService::IdentityObserver::~IdentityObserver() {
+  identity_provider_->RemoveObserver(this);
+}
+
+void GCMProfileService::IdentityObserver::OnActiveAccountLogin() {
+  // This might be called multiple times when the password changes.
+  const std::string account_id = identity_provider_->GetActiveAccountId();
+  if (account_id == account_id_)
+    return;
+  account_id_ = account_id;
+
+  driver_->OnSignedIn();
+}
+
+void GCMProfileService::IdentityObserver::OnActiveAccountLogout() {
+  driver_->Purge();
+}
+
+std::string GCMProfileService::IdentityObserver::SignedInUserName() const {
+  return driver_->IsStarted() ? account_id_ : std::string();
+}
+#endif  // !defined(OS_ANDROID)
+
 // static
 bool GCMProfileService::IsGCMEnabled(Profile* profile) {
   return profile->GetPrefs()->GetBoolean(prefs::kGCMChannelEnabled);
@@ -60,12 +118,10 @@ GCMProfileService::GCMProfileService(
 
   driver_ = CreateGCMDriverDesktop(
       gcm_client_factory.Pass(),
-      scoped_ptr<IdentityProvider>(new ProfileIdentityProvider(
-          SigninManagerFactory::GetForProfile(profile_),
-          ProfileOAuth2TokenServiceFactory::GetForProfile(profile_),
-          LoginUIServiceFactory::GetForProfile(profile_))),
       profile_->GetPath().Append(chrome::kGCMStoreDirname),
       profile_->GetRequestContext());
+
+  identity_observer_.reset(new IdentityObserver(profile, driver_.get()));
 }
 #endif  // defined(OS_ANDROID)
 
@@ -96,12 +152,25 @@ void GCMProfileService::Register(const std::string& app_id,
 }
 
 void GCMProfileService::Shutdown() {
+#if !defined(OS_ANDROID)
+  identity_observer_.reset();
+#endif  // !defined(OS_ANDROID)
+
   if (driver_) {
     driver_->Shutdown();
     driver_.reset();
   }
 }
 
+std::string GCMProfileService::SignedInUserName() const {
+#if defined(OS_ANDROID)
+  return std::string();
+#else
+  return identity_observer_ ? identity_observer_->SignedInUserName()
+                            : std::string();
+#endif  // defined(OS_ANDROID)
+}
+
 void GCMProfileService::SetDriverForTesting(GCMDriver* driver) {
   driver_.reset(driver);
 }

chrome/browser/services/gcm/gcm_profile_service.h

@@ -54,6 +54,11 @@ class GCMProfileService : public KeyedService {
   // KeyedService:
   virtual void Shutdown() OVERRIDE;
 
+  // Returns the user name if the profile is signed in or an empty string
+  // otherwise.
+  // TODO(jianli): To be removed when sign-in enforcement is dropped.
+  std::string SignedInUserName() const;
+
   // For testing purpose.
   void SetDriverForTesting(GCMDriver* driver);
 
@@ -76,6 +81,12 @@ class GCMProfileService : public KeyedService {
   // Implementation of content::PushMessagingService using GCMProfileService.
   PushMessagingServiceImpl push_messaging_service_;
 
+  // TODO(jianli): To be removed when sign-in enforcement is dropped.
+#if !defined(OS_ANDROID)
+  class IdentityObserver;
+  scoped_ptr<IdentityObserver> identity_observer_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(GCMProfileService);
 };
 

chrome/browser/services/gcm/gcm_profile_service_unittest.cc

@@ -59,6 +59,9 @@ class GCMProfileServiceTest : public testing::Test {
 
   FakeGCMClient* GetGCMClient() const;
 
+  void CreateGCMProfileService();
+  void SignIn();
+
   void RegisterAndWaitForCompletion(const std::vector<std::string>& sender_ids);
   void UnregisterAndWaitForCompletion();
   void SendAndWaitForCompletion(const GCMClient::OutgoingMessage& message);
@@ -116,24 +119,28 @@ void GCMProfileServiceTest::SetUp() {
   builder.AddTestingFactory(SigninManagerFactory::GetInstance(),
                             FakeSigninManager::Build);
   profile_ = builder.Build();
+}
+
+void GCMProfileServiceTest::TearDown() {
+  gcm_profile_service_->driver()->RemoveAppHandler(kTestAppID);
+}
 
+void GCMProfileServiceTest::CreateGCMProfileService() {
   gcm_profile_service_ = static_cast<GCMProfileService*>(
       GCMProfileServiceFactory::GetInstance()->SetTestingFactoryAndUse(
           profile_.get(),
           &BuildGCMProfileService));
   gcm_profile_service_->driver()->AddAppHandler(
       kTestAppID, gcm_app_handler_.get());
+}
 
+void GCMProfileServiceTest::SignIn() {
   FakeSigninManager* signin_manager = static_cast<FakeSigninManager*>(
       SigninManagerFactory::GetInstance()->GetForProfile(profile_.get()));
   signin_manager->SignIn(kTestAccountID);
   base::RunLoop().RunUntilIdle();
 }
 
-void GCMProfileServiceTest::TearDown() {
-  gcm_profile_service_->driver()->RemoveAppHandler(kTestAppID);
-}
-
 void GCMProfileServiceTest::RegisterAndWaitForCompletion(
     const std::vector<std::string>& sender_ids) {
   base::RunLoop run_loop;
@@ -194,7 +201,27 @@ void GCMProfileServiceTest::SendCompleted(
   callback.Run();
 }
 
+TEST_F(GCMProfileServiceTest, CreateGCMProfileServiceBeforeSignIn) {
+  CreateGCMProfileService();
+  EXPECT_FALSE(driver()->IsStarted());
+
+  SignIn();
+  EXPECT_TRUE(driver()->IsStarted());
+}
+
+TEST_F(GCMProfileServiceTest, CreateGCMProfileServiceAfterSignIn) {
+  SignIn();
+  // Note that we can't check if GCM is started or not since the
+  // GCMProfileService that hosts the GCMDriver is not created yet.
+
+  CreateGCMProfileService();
+  EXPECT_TRUE(driver()->IsStarted());
+}
+
 TEST_F(GCMProfileServiceTest, RegisterAndUnregister) {
+  CreateGCMProfileService();
+  SignIn();
+
   std::vector<std::string> sender_ids;
   sender_ids.push_back("sender");
   RegisterAndWaitForCompletion(sender_ids);
@@ -209,6 +236,9 @@ TEST_F(GCMProfileServiceTest, RegisterAndUnregister) {
 }
 
 TEST_F(GCMProfileServiceTest, Send) {
+  CreateGCMProfileService();
+  SignIn();
+
   GCMClient::OutgoingMessage message;
   message.id = "1";
   message.data["key1"] = "value1";

chrome/browser/ui/webui/gcm_internals_ui.cc

@@ -156,7 +156,7 @@ void GcmInternalsUIMessageHandler::ReturnResults(
                           gcm::GCMProfileService::IsGCMEnabled(profile));
   if (profile_service) {
     device_info->SetString("signedInUserName",
-                           profile_service->driver()->SignedInUserName());
+                           profile_service->SignedInUserName());
     device_info->SetBoolean("gcmClientReady",
                             profile_service->driver()->IsGCMClientReady());
   }
@@ -229,7 +229,7 @@ void GcmInternalsUIMessageHandler::RequestAllInfo(
 
   if (!profile_service) {
     ReturnResults(profile, NULL, NULL);
-  } else if (profile_service->driver()->SignedInUserName().empty()) {
+  } else if (profile_service->SignedInUserName().empty()) {
     ReturnResults(profile, profile_service, NULL);
   } else {
     profile_service->driver()->GetGCMStatistics(
@@ -258,7 +258,7 @@ void GcmInternalsUIMessageHandler::SetRecording(const base::ListValue* args) {
     ReturnResults(profile, NULL, NULL);
     return;
   }
-  if (profile_service->driver()->SignedInUserName().empty()) {
+  if (profile_service->SignedInUserName().empty()) {
     ReturnResults(profile, profile_service, NULL);
     return;
   }

components/gcm_driver/fake_gcm_client.cc

@@ -33,7 +33,6 @@ FakeGCMClient::~FakeGCMClient() {
 void FakeGCMClient::Initialize(
     const ChromeBuildInfo& chrome_build_info,
     const base::FilePath& store_path,
-    const std::vector<std::string>& account_ids,
     const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner,
     const scoped_refptr<net::URLRequestContextGetter>&
         url_request_context_getter,

components/gcm_driver/fake_gcm_client.h

@@ -39,7 +39,6 @@ class FakeGCMClient : public GCMClient {
   virtual void Initialize(
       const ChromeBuildInfo& chrome_build_info,
       const base::FilePath& store_path,
-      const std::vector<std::string>& account_ids,
       const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner,
       const scoped_refptr<net::URLRequestContextGetter>&
           url_request_context_getter,

components/gcm_driver/fake_gcm_driver.cc

@@ -22,6 +22,12 @@ void FakeGCMDriver::AddAppHandler(
 void FakeGCMDriver::RemoveAppHandler(const std::string& app_id) {
 }
 
+void FakeGCMDriver::OnSignedIn() {
+}
+
+void FakeGCMDriver::Purge() {
+}
+
 void FakeGCMDriver::Enable() {
 }
 
@@ -52,10 +58,6 @@ void FakeGCMDriver::SetGCMRecording(const GetGCMStatisticsCallback& callback,
                                     bool recording) {
 }
 
-std::string FakeGCMDriver::SignedInUserName() const {
-  return std::string();
-}
-
 GCMClient::Result FakeGCMDriver::EnsureStarted() {
   return GCMClient::SUCCESS;
 }

components/gcm_driver/fake_gcm_driver.h

@@ -23,6 +23,8 @@ class FakeGCMDriver : public GCMDriver {
   virtual void RemoveAppHandler(const std::string& app_id) OVERRIDE;
 
   // GCMDriver implementation:
+  virtual void OnSignedIn() OVERRIDE;
+  virtual void Purge() OVERRIDE;
   virtual void Enable() OVERRIDE;
   virtual void Disable() OVERRIDE;
   virtual GCMClient* GetGCMClientForTesting() const OVERRIDE;
@@ -33,7 +35,6 @@ class FakeGCMDriver : public GCMDriver {
                                 bool clear_logs) OVERRIDE;
   virtual void SetGCMRecording(const GetGCMStatisticsCallback& callback,
                                bool recording) OVERRIDE;
-  virtual std::string SignedInUserName() const OVERRIDE;
 
  protected:
   // GCMDriver implementation:

components/gcm_driver/gcm_client.h

@@ -208,15 +208,13 @@ class GCMClient {
   // connection.
   // |chrome_build_info|: 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.
   // |blocking_task_runner|: for running blocking file tasks.
   // |url_request_context_getter|: for url requests.
   // |delegate|: the delegate whose methods will be called asynchronously in
   //             response to events and messages.
   virtual void Initialize(
       const ChromeBuildInfo& chrome_build_info,
       const base::FilePath& store_path,
-      const std::vector<std::string>& account_ids,
       const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner,
       const scoped_refptr<net::URLRequestContextGetter>&
           url_request_context_getter,