Make GCMProfileService own GCMDriver, instead of deriving from it

Also remove several tests related to testing on neutral channel signals.
Replacement tests will be added when we switch to starting and stopping
GCM on demand in the future patch.

BUG=356716
TEST=tests updated
TBR=kalman@chromium.org,pavely@chromium.org,arv@chromium.org

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

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

chrome/browser/extensions/api/gcm/gcm_api.cc

@@ -13,9 +13,11 @@
 #include "base/strings/string_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/services/gcm/gcm_driver.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/common/extensions/api/gcm.h"
+#include "extensions/browser/event_router.h"
 #include "extensions/common/extension.h"
 
 namespace {
@@ -28,6 +30,7 @@ const char kGoogleRestrictedPrefix[] = "google";
 // Error messages.
 const char kInvalidParameter[] =
     "Function was called with invalid parameters.";
+const char kGCMDisabled[] = "GCM is currently disabled.";
 const char kNotSignedIn[] = "Profile was not signed in.";
 const char kAsyncOperationPending[] =
     "Asynchronous operation is pending.";
@@ -42,6 +45,8 @@ const char* GcmResultToError(gcm::GCMClient::Result result) {
       return "";
     case gcm::GCMClient::INVALID_PARAMETER:
       return kInvalidParameter;
+    case gcm::GCMClient::GCM_DISABLED:
+      return kGCMDisabled;
     case gcm::GCMClient::NOT_SIGNED_IN:
       return kNotSignedIn;
     case gcm::GCMClient::ASYNC_OPERATION_PENDING:
@@ -97,9 +102,9 @@ bool GcmApiFunction::IsGcmApiEnabled() const {
       gcm::GCMProfileService::ALWAYS_DISABLED;
 }
 
-gcm::GCMProfileService* GcmApiFunction::GCMProfileService() const {
+gcm::GCMDriver* GcmApiFunction::GetGCMDriver() const {
   return gcm::GCMProfileServiceFactory::GetForProfile(
-      Profile::FromBrowserContext(browser_context()));
+      Profile::FromBrowserContext(browser_context()))->driver();
 }
 
 GcmRegisterFunction::GcmRegisterFunction() {}
@@ -111,7 +116,7 @@ bool GcmRegisterFunction::DoWork() {
       api::gcm::Register::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  GCMProfileService()->Register(
+  GetGCMDriver()->Register(
       GetExtension()->id(),
       params->sender_ids,
       base::Bind(&GcmRegisterFunction::CompleteFunctionWithResult, this));
@@ -134,7 +139,7 @@ GcmUnregisterFunction::~GcmUnregisterFunction() {}
 bool GcmUnregisterFunction::DoWork() {
   UMA_HISTOGRAM_BOOLEAN("GCM.APICallUnregister", true);
 
-  GCMProfileService()->Unregister(
+  GetGCMDriver()->Unregister(
       GetExtension()->id(),
       base::Bind(&GcmUnregisterFunction::CompleteFunctionWithResult, this));
 
@@ -164,7 +169,7 @@ bool GcmSendFunction::DoWork() {
   if (params->message.time_to_live.get())
     outgoing_message.time_to_live = *params->message.time_to_live;
 
-  GCMProfileService()->Send(
+  GetGCMDriver()->Send(
       GetExtension()->id(),
       params->message.destination_id,
       outgoing_message,
@@ -199,19 +204,9 @@ bool GcmSendFunction::ValidateMessageData(
 }
 
 GcmJsEventRouter::GcmJsEventRouter(Profile* profile) : profile_(profile) {
-  EventRouter* event_router = EventRouter::Get(profile_);
-  if (!event_router)
-    return;
-
-  event_router->RegisterObserver(this, api::gcm::OnMessage::kEventName);
-  event_router->RegisterObserver(this, api::gcm::OnMessagesDeleted::kEventName);
-  event_router->RegisterObserver(this, api::gcm::OnSendError::kEventName);
 }
 
 GcmJsEventRouter::~GcmJsEventRouter() {
-  EventRouter* event_router = EventRouter::Get(profile_);
-  if (event_router)
-    event_router->UnregisterObserver(this);
 }
 
 void GcmJsEventRouter::OnMessage(
@@ -252,12 +247,4 @@ void GcmJsEventRouter::OnSendError(
   EventRouter::Get(profile_)->DispatchEventToExtension(app_id, event.Pass());
 }
 
-void GcmJsEventRouter::OnListenerAdded(const EventListenerInfo& details) {
-  if (gcm::GCMProfileService::GetGCMEnabledState(profile_) ==
-      gcm::GCMProfileService::ALWAYS_DISABLED) {
-    return;
-  }
-  gcm::GCMProfileServiceFactory::GetForProfile(profile_)->Start();
-}
-
 }  // namespace extensions

chrome/browser/extensions/api/gcm/gcm_api.h

@@ -6,11 +6,11 @@
 #define CHROME_BROWSER_EXTENSIONS_API_GCM_GCM_API_H_
 
 #include "chrome/common/extensions/api/gcm.h"
-#include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_function.h"
 #include "google_apis/gcm/gcm_client.h"
 
 namespace gcm {
+class GCMDriver;
 class GCMProfileService;
 }  // namespace gcm
 
@@ -34,7 +34,7 @@ class GcmApiFunction : public AsyncExtensionFunction {
   // Checks that the GCM API is enabled.
   bool IsGcmApiEnabled() const;
 
-  gcm::GCMProfileService* GCMProfileService() const;
+  gcm::GCMDriver* GetGCMDriver() const;
 };
 
 class GcmRegisterFunction : public GcmApiFunction {
@@ -91,7 +91,7 @@ class GcmSendFunction : public GcmApiFunction {
   bool ValidateMessageData(const gcm::GCMClient::MessageData& data) const;
 };
 
-class GcmJsEventRouter : public EventRouter::Observer {
+class GcmJsEventRouter {
  public:
   explicit GcmJsEventRouter(Profile* profile);
 
@@ -103,9 +103,6 @@ class GcmJsEventRouter : public EventRouter::Observer {
   void OnSendError(const std::string& app_id,
                    const gcm::GCMClient::SendErrorDetails& send_error_details);
 
-  // EventRouter::Observer:
-  virtual void OnListenerAdded(const EventListenerInfo& details) OVERRIDE;
-
  private:
   // The application we route the event to is running in context of the
   // |profile_| and the latter outlives the event router.

chrome/browser/extensions/api/gcm/gcm_apitest.cc

@@ -13,7 +13,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "components/gcm_driver/gcm_client_factory.h"
 
 namespace {
 

chrome/browser/extensions/extension_gcm_app_handler.cc

@@ -9,6 +9,7 @@
 #include "base/location.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/services/gcm/gcm_driver.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "content/public/browser/notification_details.h"
@@ -63,7 +64,7 @@ ExtensionGCMAppHandler::~ExtensionGCMAppHandler() {
        extension != enabled_extensions.end();
        ++extension) {
     if (IsGCMPermissionEnabled(extension->get()))
-      GetGCMProfileService()->RemoveAppHandler((*extension)->id());
+      GetGCMDriver()->RemoveAppHandler((*extension)->id());
   }
 }
 
@@ -99,15 +100,15 @@ void ExtensionGCMAppHandler::OnExtensionLoaded(
     content::BrowserContext* browser_context,
     const Extension* extension) {
   if (IsGCMPermissionEnabled(extension))
-    GetGCMProfileService()->AddAppHandler(extension->id(), this);
+    GetGCMDriver()->AddAppHandler(extension->id(), this);
 }
 
 void ExtensionGCMAppHandler::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
     const Extension* extension,
     UnloadedExtensionInfo::Reason reason) {
   if (IsGCMPermissionEnabled(extension))
-    GetGCMProfileService()->RemoveAppHandler(extension->id());
+    GetGCMDriver()->RemoveAppHandler(extension->id());
 }
 
 void ExtensionGCMAppHandler::Observe(
@@ -117,17 +118,17 @@ void ExtensionGCMAppHandler::Observe(
   DCHECK_EQ(chrome::NOTIFICATION_EXTENSION_UNINSTALLED, type);
   const Extension* extension = content::Details<Extension>(details).ptr();
   if (IsGCMPermissionEnabled(extension)) {
-    GetGCMProfileService()->Unregister(
+    GetGCMDriver()->Unregister(
         extension->id(),
         base::Bind(&ExtensionGCMAppHandler::OnUnregisterCompleted,
                    weak_factory_.GetWeakPtr(),
                    extension->id()));
-    GetGCMProfileService()->RemoveAppHandler(extension->id());
+    GetGCMDriver()->RemoveAppHandler(extension->id());
   }
 }
 
-gcm::GCMProfileService* ExtensionGCMAppHandler::GetGCMProfileService() const {
-  return gcm::GCMProfileServiceFactory::GetForProfile(profile_);
+gcm::GCMDriver* ExtensionGCMAppHandler::GetGCMDriver() const {
+  return gcm::GCMProfileServiceFactory::GetForProfile(profile_)->driver();
 }
 
 void ExtensionGCMAppHandler::OnUnregisterCompleted(

chrome/browser/extensions/extension_gcm_app_handler.h

@@ -26,6 +26,7 @@ class BrowserContext;
 }
 
 namespace gcm {
+class GCMDriver;
 class GCMProfileService;
 }
 
@@ -77,7 +78,7 @@ class ExtensionGCMAppHandler : public gcm::GCMAppHandler,
       const Extension* extension,
       UnloadedExtensionInfo::Reason reason) OVERRIDE;
 
-  gcm::GCMProfileService* GetGCMProfileService() const;
+  gcm::GCMDriver* GetGCMDriver() const;
 
   // BrowserContextKeyedAPI implementation.
   static const char* service_name() { return "ExtensionGCMAppHandler"; }

chrome/browser/extensions/extension_gcm_app_handler_unittest.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/services/gcm/fake_gcm_client.h"
 #include "chrome/browser/services/gcm/fake_gcm_client_factory.h"
 #include "chrome/browser/services/gcm/fake_signin_manager.h"
+#include "chrome/browser/services/gcm/gcm_driver.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -167,7 +168,10 @@ class ExtensionGCMAppHandlerTest : public testing::Test {
  public:
   static KeyedService* BuildGCMProfileService(
       content::BrowserContext* context) {
-    return new gcm::GCMProfileService(static_cast<Profile*>(context));
+    return new gcm::GCMProfileService(
+        Profile::FromBrowserContext(context),
+        scoped_ptr<gcm::GCMClientFactory>(
+            new gcm::FakeGCMClientFactory(gcm::FakeGCMClient::NO_DELAY_START)));
   }
 
   ExtensionGCMAppHandlerTest()
@@ -209,15 +213,8 @@ class ExtensionGCMAppHandlerTest : public testing::Test {
     profile()->GetPrefs()->SetBoolean(prefs::kGCMChannelEnabled, true);
 
     // Create GCMProfileService that talks with fake GCMClient.
-    gcm::GCMProfileService* gcm_profile_service =
-        static_cast<gcm::GCMProfileService*>(
-            gcm::GCMProfileServiceFactory::GetInstance()->
-                SetTestingFactoryAndUse(
-                    profile(),
-                    &ExtensionGCMAppHandlerTest::BuildGCMProfileService));
-    scoped_ptr<gcm::GCMClientFactory> gcm_client_factory(
-        new gcm::FakeGCMClientFactory(gcm::FakeGCMClient::NO_DELAY_START));
-    gcm_profile_service->Initialize(gcm_client_factory.Pass());
+    gcm::GCMProfileServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+        profile(), &ExtensionGCMAppHandlerTest::BuildGCMProfileService);
 
     // Create a fake version of ExtensionGCMAppHandler.
     gcm_app_handler_.reset(new FakeExtensionGCMAppHandler(profile(), &waiter_));
@@ -288,7 +285,7 @@ class ExtensionGCMAppHandlerTest : public testing::Test {
 
   void Register(const std::string& app_id,
                 const std::vector<std::string>& sender_ids) {
-    GetGCMProfileService()->Register(
+    GetGCMDriver()->Register(
         app_id,
         sender_ids,
         base::Bind(&ExtensionGCMAppHandlerTest::RegisterCompleted,
@@ -301,12 +298,12 @@ class ExtensionGCMAppHandlerTest : public testing::Test {
     waiter_.SignalCompleted();
   }
 
-  gcm::GCMProfileService* GetGCMProfileService() const {
-    return gcm::GCMProfileServiceFactory::GetForProfile(profile());
+  gcm::GCMDriver* GetGCMDriver() const {
+    return gcm::GCMProfileServiceFactory::GetForProfile(profile())->driver();
   }
 
   bool HasAppHandlers(const std::string& app_id) const {
-    return GetGCMProfileService()->app_handlers().count(app_id);
+    return GetGCMDriver()->app_handlers().count(app_id);
   }
 
   Profile* profile() const { return profile_.get(); }

chrome/browser/invalidation/gcm_invalidation_bridge.h

@@ -37,7 +37,7 @@ class GCMInvalidationBridge : public gcm::GCMAppHandler,
  public:
   class Core;
 
-  GCMInvalidationBridge(gcm::GCMDriver* gcm_service,
+  GCMInvalidationBridge(gcm::GCMDriver* gcm_driver,
                         IdentityProvider* identity_provider);
   virtual ~GCMInvalidationBridge();
 

chrome/browser/invalidation/gcm_invalidation_bridge_unittest.cc

@@ -4,8 +4,7 @@
 
 #include "base/run_loop.h"
 #include "chrome/browser/invalidation/gcm_invalidation_bridge.h"
-#include "chrome/browser/services/gcm/gcm_profile_service.h"
-#include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
+#include "chrome/browser/services/gcm/gcm_driver.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service_builder.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
@@ -18,17 +17,12 @@
 namespace invalidation {
 namespace {
 
-// Implementation of GCMProfileService::Register that always succeeds with the
-// same registrationId.
-class FakeGCMProfileService : public gcm::GCMProfileService {
+// Implementation of GCMDriver::Register that always succeeds with the same
+// registrationId.
+class FakeGCMDriver : public gcm::GCMDriver {
  public:
-  static KeyedService* Build(content::BrowserContext* context) {
-    Profile* profile = static_cast<Profile*>(context);
-    return new FakeGCMProfileService(profile);
-  }
-
-  explicit FakeGCMProfileService(Profile* profile)
-      : gcm::GCMProfileService(profile) {}
+  FakeGCMDriver() {}
+  virtual ~FakeGCMDriver() {}
 
   virtual void Register(const std::string& app_id,
                         const std::vector<std::string>& sender_ids,
@@ -40,7 +34,7 @@ class FakeGCMProfileService : public gcm::GCMProfileService {
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(FakeGCMProfileService);
+  DISALLOW_COPY_AND_ASSIGN(FakeGCMDriver);
 };
 
 class GCMInvalidationBridgeTest : public ::testing::Test {
@@ -53,20 +47,16 @@ class GCMInvalidationBridgeTest : public ::testing::Test {
     TestingProfile::Builder builder;
     builder.AddTestingFactory(ProfileOAuth2TokenServiceFactory::GetInstance(),
                               &BuildAutoIssuingFakeProfileOAuth2TokenService);
-    builder.AddTestingFactory(gcm::GCMProfileServiceFactory::GetInstance(),
-                              &FakeGCMProfileService::Build);
     profile_ = builder.Build();
 
     FakeProfileOAuth2TokenService* token_service =
         (FakeProfileOAuth2TokenService*)
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile_.get());
     token_service->IssueRefreshTokenForUser("", "fake_refresh_token");
-    gcm_profile_service_ =
-        (FakeGCMProfileService*)gcm::GCMProfileServiceFactory::GetForProfile(
-            profile_.get());
+    gcm_driver_.reset(new FakeGCMDriver());
 
     identity_provider_.reset(new FakeIdentityProvider(token_service));
-    bridge_.reset(new GCMInvalidationBridge(gcm_profile_service_,
+    bridge_.reset(new GCMInvalidationBridge(gcm_driver_.get(),
                                             identity_provider_.get()));
 
     delegate_ = bridge_->CreateDelegate();
@@ -89,7 +79,7 @@ class GCMInvalidationBridgeTest : public ::testing::Test {
 
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<Profile> profile_;
-  FakeGCMProfileService* gcm_profile_service_;
+  scoped_ptr<gcm::GCMDriver> gcm_driver_;
   scoped_ptr<FakeIdentityProvider> identity_provider_;
 
   std::vector<std::string> issued_tokens_;

chrome/browser/invalidation/invalidation_service_factory.cc

@@ -125,7 +125,7 @@ KeyedService* InvalidationServiceFactory::BuildServiceInstanceFor(
       identity_provider.Pass(),
       scoped_ptr<TiclSettingsProvider>(
           new TiclProfileSettingsProvider(profile)),
-      gcm::GCMProfileServiceFactory::GetForProfile(profile),
+      gcm::GCMProfileServiceFactory::GetForProfile(profile)->driver(),
       profile->GetRequestContext());
   service->Init(scoped_ptr<syncer::InvalidationStateTracker>(
       new InvalidatorStorage(profile->GetPrefs())));

chrome/browser/invalidation/ticl_invalidation_service.h

@@ -62,7 +62,7 @@ class TiclInvalidationService : public base::NonThreadSafe,
   TiclInvalidationService(
       scoped_ptr<IdentityProvider> identity_provider,
       scoped_ptr<TiclSettingsProvider> settings_provider,
-      gcm::GCMDriver* gcm_service,
+      gcm::GCMDriver* gcm_driver,
       const scoped_refptr<net::URLRequestContextGetter>& request_context);
   virtual ~TiclInvalidationService();