Refactor GCMNetworkChannelDelegateImpl

Receiving messages from GCM needed separate object with lifetime tied to
TiclInvalidationService on UI thread. This change is to replace
GCMNetworkChannelDelegateImpl with pair of objects:
GCMInvalidationBridge/Core.
Here is object diagram for this refactoring: http://www.plantuml.com:80/plantuml/png/VPBRQiCm343VynKYZuNs0xgoXsLZ17PZxPwBwwYf5sC7nqiPs_vzbfsB4sWXuDYHZexar8nNh4H8FtBOdGoo8eMV9D67J-GMqftWCbSjaw9WLcYrHTCAZYcm1gpxvc816gvhBFQrDPix8ueIR-7WrvlahoSzPiyFBIhxhhyich60Eu_v8T_Z0uBuPsGLqO5hWmuP9xyPVHGIOw-oRhpMjEG2SDoG63rrttt-6dL_rMszpfo5BqGwC14CpZF55xHdRQhin9H2UOyIIsO3eUWsZd8j9rGBxxE93uW8iFrMF1P_4er5XMQUgaAHVKUceJY8lzR5Sjat4nRGgrMO_tIiiFaTG_r0AB3VTyf46po7dYdJ4-d2j0kP_ICNso1SXmwsLF9Iz3y0

BUG=325020
R=rlarocque@chromium.org

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

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

chrome/browser/invalidation/gcm_network_channel_delegate_impl.h → chrome/browser/invalidation/gcm_invalidation_bridge.h

@@ -2,10 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_INVALIDATION_GCM_NETWORK_CHANNEL_DELEGATE_IMPL_H_
-#define CHROME_BROWSER_INVALIDATION_GCM_NETWORK_CHANNEL_DELEGATE_IMPL_H_
+#ifndef CHROME_BROWSER_INVALIDATION_GCM_INVALIDATION_BRIDGE_H_
+#define CHROME_BROWSER_INVALIDATION_GCM_INVALIDATION_BRIDGE_H_
 
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/non_thread_safe.h"
 #include "google_apis/gaia/oauth2_token_service.h"
+#include "google_apis/gcm/gcm_client.h"
 #include "sync/notifier/gcm_network_channel_delegate.h"
 
 class Profile;
@@ -16,22 +21,18 @@ class SingleThreadTaskRunner;
 
 namespace invalidation {
 
-// Implementation of GCMNetworkChannelDelegate.
-// GCMNetworkChannel lives in sync/notifier and therefore doesn't have access to
-// ProfileOAuth2TokenService and GCMPRofileService that it needs.
-// GCMNetworkChannelDelegate declares abstract interface for these functions
-// which GCMNetworkChannelDelegateImpl implements in chrome/browser/invalidation
-// where it has access to BrowserContext keyed services.
-class GCMNetworkChannelDelegateImpl : public syncer::GCMNetworkChannelDelegate,
-                                      public OAuth2TokenService::Consumer {
+// GCMInvalidationBridge and GCMInvalidationBridge::Core implement functions
+// needed for GCMNetworkChannel. GCMInvalidationBridge lives on UI thread while
+// Core lives on IO thread. Core implements GCMNetworkChannelDelegate and posts
+// all function calls to GCMInvalidationBridge which does actual work to perform
+// them.
+class GCMInvalidationBridge : public OAuth2TokenService::Consumer,
+                              public base::NonThreadSafe {
  public:
-  explicit GCMNetworkChannelDelegateImpl(Profile* profile);
-  virtual ~GCMNetworkChannelDelegateImpl();
+  class Core;
 
-  // GCMNetworkChannelDelegate implementation.
-  virtual void Register(RegisterCallback callback) OVERRIDE;
-  virtual void RequestToken(RequestTokenCallback callback) OVERRIDE;
-  virtual void InvalidateToken(const std::string& token) OVERRIDE;
+  explicit GCMInvalidationBridge(Profile* profile);
+  virtual ~GCMInvalidationBridge();
 
   // OAuth2TokenService::Consumer implementation.
   virtual void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
@@ -40,19 +41,43 @@ class GCMNetworkChannelDelegateImpl : public syncer::GCMNetworkChannelDelegate,
   virtual void OnGetTokenFailure(const OAuth2TokenService::Request* request,
                                  const GoogleServiceAuthError& error) OVERRIDE;
 
- private:
-  static void InvalidateTokenOnUIThread(Profile* profile,
-                                        const std::string& token);
+  scoped_ptr<syncer::GCMNetworkChannelDelegate> CreateDelegate();
+
+  void CoreInitializationDone(
+      base::WeakPtr<Core> core,
+      scoped_refptr<base::SingleThreadTaskRunner> core_thread_task_runner);
+
+  // Functions reflecting GCMNetworkChannelDelegate interface. These are called
+  // on UI thread to perform actual work.
+  void RequestToken(
+      syncer::GCMNetworkChannelDelegate::RequestTokenCallback callback);
+  void InvalidateToken(const std::string& token);
+
+  void Register(syncer::GCMNetworkChannelDelegate::RegisterCallback callback);
+
+  void RegisterFinished(
+      syncer::GCMNetworkChannelDelegate::RegisterCallback callback,
+      const std::string& registration_id,
+      gcm::GCMClient::Result result);
 
+ private:
+  // GCMInvalidationBridge is owned by TiclInvalidationService therefore it is
+  // expected that profile_ pointer is valid throughout lifetime of this object.
   Profile* profile_;
-  scoped_refptr<base::SingleThreadTaskRunner> ui_thread_task_runner_;
+
+  base::WeakPtr<Core> core_;
+  scoped_refptr<base::SingleThreadTaskRunner> core_thread_task_runner_;
 
   // Fields related to RequestToken function.
-  std::string account_id_;
   scoped_ptr<OAuth2TokenService::Request> access_token_request_;
-  RequestTokenCallback request_token_callback_;
+  syncer::GCMNetworkChannelDelegate::RequestTokenCallback
+      request_token_callback_;
+
+  base::WeakPtrFactory<GCMInvalidationBridge> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(GCMInvalidationBridge);
 };
 
 }  // namespace invalidation
 
-#endif  // CHROME_BROWSER_INVALIDATION_GCM_NETWORK_CHANNEL_DELEGATE_IMPL_H_
+#endif  // CHROME_BROWSER_INVALIDATION_GCM_INVALIDATION_BRIDGE_H_

chrome/browser/invalidation/gcm_network_channel_delegate_impl_unittest.cc → chrome/browser/invalidation/gcm_invalidation_bridge_unittest.cc

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/run_loop.h"
-#include "chrome/browser/invalidation/gcm_network_channel_delegate_impl.h"
+#include "chrome/browser/invalidation/gcm_invalidation_bridge.h"
 #include "chrome/browser/services/gcm/fake_gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
@@ -18,11 +18,11 @@
 namespace invalidation {
 namespace {
 
-class GCMNetworkChannelDelegateImplTest : public ::testing::Test {
+class GCMInvalidationBridgeTest : public ::testing::Test {
  protected:
-  GCMNetworkChannelDelegateImplTest() {}
+  GCMInvalidationBridgeTest() {}
 
-  virtual ~GCMNetworkChannelDelegateImplTest() {}
+  virtual ~GCMInvalidationBridgeTest() {}
 
   virtual void SetUp() OVERRIDE {
     TestingProfile::Builder builder;
@@ -36,7 +36,12 @@ class GCMNetworkChannelDelegateImplTest : public ::testing::Test {
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile_.get());
     token_service->IssueRefreshTokenForUser("", "refresh_token");
 
-    delegate_.reset(new GCMNetworkChannelDelegateImpl(profile_.get()));
+    bridge_.reset(new GCMInvalidationBridge(profile_.get()));
+
+    delegate_ = bridge_->CreateDelegate();
+    delegate_->Initialize();
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
   }
 
  public:
@@ -56,14 +61,15 @@ class GCMNetworkChannelDelegateImplTest : public ::testing::Test {
   std::vector<std::string> issued_tokens_;
   std::vector<GoogleServiceAuthError> request_token_errors_;
 
-  scoped_ptr<GCMNetworkChannelDelegateImpl> delegate_;
+  scoped_ptr<GCMInvalidationBridge> bridge_;
+  scoped_ptr<syncer::GCMNetworkChannelDelegate> delegate_;
 };
 
-TEST_F(GCMNetworkChannelDelegateImplTest, RequestToken) {
+TEST_F(GCMInvalidationBridgeTest, RequestToken) {
   // Make sure that call to RequestToken reaches OAuth2TokenService and gets
   // back to callback.
   delegate_->RequestToken(
-      base::Bind(&GCMNetworkChannelDelegateImplTest::RequestTokenFinished,
+      base::Bind(&GCMInvalidationBridgeTest::RequestTokenFinished,
                  base::Unretained(this)));
   base::RunLoop run_loop;
   run_loop.RunUntilIdle();
@@ -72,14 +78,14 @@ TEST_F(GCMNetworkChannelDelegateImplTest, RequestToken) {
   EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(), request_token_errors_[0]);
 }
 
-TEST_F(GCMNetworkChannelDelegateImplTest, RequestTokenTwoConcurrentRequests) {
+TEST_F(GCMInvalidationBridgeTest, RequestTokenTwoConcurrentRequests) {
   // First call should finish with REQUEST_CANCELLED error.
   delegate_->RequestToken(
-      base::Bind(&GCMNetworkChannelDelegateImplTest::RequestTokenFinished,
+      base::Bind(&GCMInvalidationBridgeTest::RequestTokenFinished,
                  base::Unretained(this)));
   // Second request should succeed.
   delegate_->RequestToken(
-      base::Bind(&GCMNetworkChannelDelegateImplTest::RequestTokenFinished,
+      base::Bind(&GCMInvalidationBridgeTest::RequestTokenFinished,
                  base::Unretained(this)));
   base::RunLoop run_loop;
   run_loop.RunUntilIdle();

chrome/browser/invalidation/invalidation_service_factory.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/invalidation/p2p_invalidation_service.h"
 #include "chrome/browser/invalidation/ticl_invalidation_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
@@ -44,6 +45,7 @@ InvalidationServiceFactory::InvalidationServiceFactory()
 #if !defined(OS_ANDROID)
   DependsOn(SigninManagerFactory::GetInstance());
   DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+  DependsOn(gcm::GCMProfileServiceFactory::GetInstance());
 #endif
 }
 

chrome/browser/invalidation/ticl_invalidation_service.cc

@@ -6,7 +6,7 @@
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
-#include "chrome/browser/invalidation/gcm_network_channel_delegate_impl.h"
+#include "chrome/browser/invalidation/gcm_invalidation_bridge.h"
 #include "chrome/browser/invalidation/invalidation_logger.h"
 #include "chrome/browser/invalidation/invalidation_service_util.h"
 #include "chrome/browser/profiles/profile.h"
@@ -356,12 +356,11 @@ void TiclInvalidationService::StartInvalidator(
       break;
     }
     case GCM_NETWORK_CHANNEL: {
-      scoped_ptr<syncer::GCMNetworkChannelDelegate> delegate;
-      delegate.reset(new GCMNetworkChannelDelegateImpl(profile_));
+      gcm_invalidation_bridge_.reset(new GCMInvalidationBridge(profile_));
       network_channel_creator =
           syncer::NonBlockingInvalidator::MakeGCMNetworkChannelCreator(
               profile_->GetRequestContext(),
-              delegate.Pass());
+              gcm_invalidation_bridge_->CreateDelegate().Pass());
       break;
     }
     default: {
@@ -398,6 +397,7 @@ void TiclInvalidationService::UpdateInvalidatorCredentials() {
 
 void TiclInvalidationService::StopInvalidator() {
   DCHECK(invalidator_);
+  gcm_invalidation_bridge_.reset();
   invalidator_->UnregisterHandler(this);
   invalidator_.reset();
 }

chrome/browser/invalidation/ticl_invalidation_service.h

@@ -26,6 +26,7 @@ class Invalidator;
 }
 
 namespace invalidation {
+class GCMInvalidationBridge;
 
 // This InvalidationService wraps the C++ Invalidation Client (TICL) library.
 // It provides invalidations for desktop platforms (Win, Mac, Linux).
@@ -122,6 +123,8 @@ class TiclInvalidationService : public base::NonThreadSafe,
   base::OneShotTimer<TiclInvalidationService> request_access_token_retry_timer_;
   net::BackoffEntry request_access_token_backoff_;
 
+  scoped_ptr<GCMInvalidationBridge> gcm_invalidation_bridge_;
+
   // The invalidation logger object we use to record state changes
   // and invalidations.
   InvalidationLogger logger_;

chrome/chrome_browser.gypi

@@ -978,8 +978,8 @@
         'browser/invalidation/invalidation_logger.h',
         'browser/invalidation/fake_invalidation_service.cc',
         'browser/invalidation/fake_invalidation_service.h',
-        'browser/invalidation/gcm_network_channel_delegate_impl.cc',
-        'browser/invalidation/gcm_network_channel_delegate_impl.h',
+        'browser/invalidation/gcm_invalidation_bridge.cc',
+        'browser/invalidation/gcm_invalidation_bridge.h',
         'browser/invalidation/invalidation_controller_android.cc',
         'browser/invalidation/invalidation_controller_android.h',
         'browser/invalidation/invalidation_service.h',
@@ -3337,8 +3337,8 @@
             'test/base/test_switches.cc',
 
             # Android uses a different invalidation service
-            'browser/invalidation/gcm_network_channel_delegate_impl.cc',
-            'browser/invalidation/gcm_network_channel_delegate_impl.h',
+            'browser/invalidation/gcm_invalidation_bridge.cc',
+            'browser/invalidation/gcm_invalidation_bridge.h',
             'browser/invalidation/ticl_invalidation_service.cc',
             'browser/invalidation/ticl_invalidation_service.h',
 

chrome/chrome_tests_unit.gypi

@@ -1024,7 +1024,7 @@
         'browser/install_verification/win/module_list_unittest.cc',
         'browser/install_verification/win/module_verification_test.cc',
         'browser/install_verification/win/module_verification_test.h',
-        'browser/invalidation/gcm_network_channel_delegate_impl_unittest.cc',
+        'browser/invalidation/gcm_invalidation_bridge_unittest.cc',
         'browser/invalidation/invalidation_logger_unittest.cc',
         'browser/invalidation/invalidation_service_android_unittest.cc',
         'browser/invalidation/invalidation_service_test_template.cc',
@@ -2576,7 +2576,7 @@
             'browser/ui/autofill/autofill_popup_controller_unittest.cc',
 
             # Android uses a different invaliator.
-            'browser/invalidation/gcm_network_channel_delegate_impl_unittest.cc',
+            'browser/invalidation/gcm_invalidation_bridge_unittest.cc',
             'browser/invalidation/ticl_invalidation_service_unittest.cc',
 
             # The importer code is not used on Android.

sync/notifier/gcm_network_channel.cc

@@ -49,6 +49,7 @@ GCMNetworkChannel::GCMNetworkChannel(
       delegate_(delegate.Pass()),
       register_backoff_entry_(new net::BackoffEntry(&kRegisterBackoffPolicy)),
       weak_factory_(this) {
+  delegate_->Initialize();
   Register();
 }
 

sync/notifier/gcm_network_channel_delegate.h

@@ -31,6 +31,7 @@ class GCMNetworkChannelDelegate {
 
   virtual ~GCMNetworkChannelDelegate() {}
 
+  virtual void Initialize() = 0;
   // Request access token. Callback should be called either with access token or
   // error code.
   virtual void RequestToken(RequestTokenCallback callback) = 0;

sync/notifier/gcm_network_channel_unittest.cc

@@ -17,6 +17,8 @@ class TestGCMNetworkChannelDelegate : public GCMNetworkChannelDelegate {
   TestGCMNetworkChannelDelegate()
       : register_call_count_(0) {}
 
+  virtual void Initialize() OVERRIDE {}
+
   virtual void RequestToken(RequestTokenCallback callback) OVERRIDE {
     request_token_callback = callback;
   }