[GCM] Extracting GCMConnectionObserver from GCMAppHandler

Extracting GCMConnectionObserver from GCMAppHandler
* Updating consuming code: GCMInvalidationBridge
* Updating GCMAppHandler overrides
* Updating tests for GCMDriverDesktop

BUG=407841
R=jianli@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#292396}

components/gcm_driver.gypi

@@ -33,6 +33,8 @@
         'gcm_driver/gcm_client_factory.h',
         'gcm_driver/gcm_client_impl.cc',
         'gcm_driver/gcm_client_impl.h',
+        'gcm_driver/gcm_connection_observer.cc',
+        'gcm_driver/gcm_connection_observer.h',
         'gcm_driver/gcm_driver.cc',
         'gcm_driver/gcm_driver.h',
         'gcm_driver/gcm_driver_android.cc',

components/gcm_driver/default_gcm_app_handler.cc

@@ -5,7 +5,6 @@
 #include "components/gcm_driver/default_gcm_app_handler.h"
 
 #include "base/logging.h"
-#include "net/base/ip_endpoint.h"
 
 namespace gcm {
 
@@ -42,14 +41,4 @@ void DefaultGCMAppHandler::OnSendAcknowledged(const std::string& app_id,
            << app_id;
 }
 
-void DefaultGCMAppHandler::OnConnected(const net::IPEndPoint& ip_endpoint) {
-  // TODO(semenzato): update CrOS NIC state.
-  DVLOG(1) << "GCM connected to " << ip_endpoint.ToString();
-}
-
-void DefaultGCMAppHandler::OnDisconnected() {
-  // TODO(semenzato): update CrOS NIC state.
-  DVLOG(1) << "GCM disconnected";
-}
-
 }  // namespace gcm

components/gcm_driver/default_gcm_app_handler.h

@@ -27,8 +27,6 @@ class DefaultGCMAppHandler : public GCMAppHandler {
       const GCMClient::SendErrorDetails& send_error_details) OVERRIDE;
   virtual void OnSendAcknowledged(const std::string& app_id,
                                   const std::string& message_id) OVERRIDE;
-  virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE;
-  virtual void OnDisconnected() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DefaultGCMAppHandler);

components/gcm_driver/fake_gcm_app_handler.cc

@@ -9,7 +9,7 @@
 namespace gcm {
 
 FakeGCMAppHandler::FakeGCMAppHandler()
-    : received_event_(NO_EVENT), connected_(false) {
+    : received_event_(NO_EVENT) {
 }
 
 FakeGCMAppHandler::~FakeGCMAppHandler() {
@@ -71,12 +71,4 @@ void FakeGCMAppHandler::ClearResults() {
   send_error_details_ = GCMClient::SendErrorDetails();
 }
 
-void FakeGCMAppHandler::OnConnected(const net::IPEndPoint& ip_endpoint) {
-  connected_ = true;
-}
-
-void FakeGCMAppHandler::OnDisconnected() {
-  connected_ = false;
-}
-
 }  // namespace gcm

components/gcm_driver/fake_gcm_app_handler.h

@@ -34,7 +34,6 @@ class FakeGCMAppHandler : public GCMAppHandler {
   const GCMClient::SendErrorDetails& send_error_details() const {
     return send_error_details_;
   }
-  bool connected() const { return connected_; }
 
   void WaitForNotification();
 
@@ -48,8 +47,6 @@ class FakeGCMAppHandler : public GCMAppHandler {
       const GCMClient::SendErrorDetails& send_error_details) OVERRIDE;
   virtual void OnSendAcknowledged(const std::string& app_id,
                                   const std::string& message_id) OVERRIDE;
-  virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE;
-  virtual void OnDisconnected() OVERRIDE;
 
  private:
   void ClearResults();
@@ -61,7 +58,6 @@ class FakeGCMAppHandler : public GCMAppHandler {
   std::string acked_message_id_;
   GCMClient::IncomingMessage message_;
   GCMClient::SendErrorDetails send_error_details_;
-  bool connected_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeGCMAppHandler);
 };

components/gcm_driver/fake_gcm_driver.cc

@@ -28,6 +28,12 @@ void FakeGCMDriver::OnSignedIn() {
 void FakeGCMDriver::Purge() {
 }
 
+void FakeGCMDriver::AddConnectionObserver(GCMConnectionObserver* observer) {
+}
+
+void FakeGCMDriver::RemoveConnectionObserver(GCMConnectionObserver* observer) {
+}
+
 void FakeGCMDriver::Enable() {
 }
 

components/gcm_driver/fake_gcm_driver.h

@@ -23,6 +23,9 @@ class FakeGCMDriver : public GCMDriver {
   virtual void RemoveAppHandler(const std::string& app_id) OVERRIDE;
   virtual void OnSignedIn() OVERRIDE;
   virtual void Purge() OVERRIDE;
+  virtual void AddConnectionObserver(GCMConnectionObserver* observer) OVERRIDE;
+  virtual void RemoveConnectionObserver(
+      GCMConnectionObserver* observer) OVERRIDE;
   virtual void Enable() OVERRIDE;
   virtual void Disable() OVERRIDE;
   virtual GCMClient* GetGCMClientForTesting() const OVERRIDE;

components/gcm_driver/gcm_app_handler.cc

@@ -9,12 +9,6 @@ namespace gcm {
 GCMAppHandler::GCMAppHandler() {}
 GCMAppHandler::~GCMAppHandler() {}
 
-void GCMAppHandler::OnConnected(const net::IPEndPoint& ip_endpoint) {
-}
-
-void GCMAppHandler::OnDisconnected() {
-}
-
 bool GCMAppHandler::CanHandle(const std::string& app_id) const {
   return false;
 }

components/gcm_driver/gcm_app_handler.h

@@ -40,16 +40,6 @@ class GCMAppHandler {
   virtual void OnSendAcknowledged(const std::string& app_id,
                                   const std::string& message_id) = 0;
 
-  // Called when a new connection is established and a successful handshake
-  // has been performed. Note that |ip_endpoint| is only set if available for
-  // the current platform.
-  // Default implementation does nothing.
-  virtual void OnConnected(const net::IPEndPoint& ip_endpoint);
-
-  // Called when the connection is interrupted.
-  // Default implementation does nothing.
-  virtual void OnDisconnected();
-
   // If no app handler has been added with the exact app_id of an incoming
   // event, all handlers will be asked (in arbitrary order) whether they can
   // handle the app_id, and the first to return true will receive the event.

components/gcm_driver/gcm_connection_observer.cc

+// Copyright 2014 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 "components/gcm_driver/gcm_connection_observer.h"
+
+namespace gcm {
+
+GCMConnectionObserver::GCMConnectionObserver() {}
+GCMConnectionObserver::~GCMConnectionObserver() {}
+
+void GCMConnectionObserver::OnConnected(const net::IPEndPoint& ip_endpoint) {
+}
+
+void GCMConnectionObserver::OnDisconnected() {
+}
+
+}  // namespace gcm

components/gcm_driver/gcm_connection_observer.h

+// Copyright (c) 2014 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.
+
+#ifndef COMPONENTS_GCM_DRIVER_GCM_CONNECTION_OBSERVER_H_
+#define COMPONENTS_GCM_DRIVER_GCM_CONNECTION_OBSERVER_H_
+
+#include "base/basictypes.h"
+
+namespace net {
+class IPEndPoint;
+}
+
+namespace gcm {
+
+// Interface for objects observing GCM connection events.
+class GCMConnectionObserver {
+ public:
+  GCMConnectionObserver();
+  virtual ~GCMConnectionObserver();
+
+  // Called when a new connection is established and a successful handshake
+  // has been performed. Note that |ip_endpoint| is only set if available for
+  // the current platform.
+  // Default implementation does nothing.
+  virtual void OnConnected(const net::IPEndPoint& ip_endpoint);
+
+  // Called when the connection is interrupted.
+  // Default implementation does nothing.
+  virtual void OnDisconnected();
+};
+
+}  // namespace gcm
+
+#endif  // COMPONENTS_GCM_DRIVER_GCM_CONNECTION_OBSERVER_H_

components/gcm_driver/gcm_driver.h

@@ -18,6 +18,7 @@
 namespace gcm {
 
 class GCMAppHandler;
+class GCMConnectionObserver;
 struct AccountMapping;
 
 // Bridge between GCM users in Chrome and the platform-specific implementation.
@@ -88,6 +89,12 @@ class GCMDriver {
   // Returns the handler for the given app.
   GCMAppHandler* GetAppHandler(const std::string& app_id);
 
+  // Adds a connection state observer.
+  virtual void AddConnectionObserver(GCMConnectionObserver* observer) = 0;
+
+  // Removes a connection state observer.
+  virtual void RemoveConnectionObserver(GCMConnectionObserver* observer) = 0;
+
   // Enables/disables GCM service.
   virtual void Enable() = 0;
   virtual void Disable() = 0;

components/gcm_driver/gcm_driver_android.cc

@@ -98,6 +98,13 @@ void GCMDriverAndroid::OnSignedIn() {
 void GCMDriverAndroid::Purge() {
 }
 
+void GCMDriverAndroid::AddConnectionObserver(GCMConnectionObserver* observer) {
+}
+
+void GCMDriverAndroid::RemoveConnectionObserver(
+    GCMConnectionObserver* observer) {
+}
+
 void GCMDriverAndroid::Enable() {
 }
 

components/gcm_driver/gcm_driver_android.h

@@ -47,6 +47,9 @@ class GCMDriverAndroid : public GCMDriver {
   virtual void OnSignedIn() OVERRIDE;
   virtual void Purge() OVERRIDE;
   virtual void Enable() OVERRIDE;
+  virtual void AddConnectionObserver(GCMConnectionObserver* observer) OVERRIDE;
+  virtual void RemoveConnectionObserver(
+      GCMConnectionObserver* observer) OVERRIDE;
   virtual void Disable() OVERRIDE;
   virtual GCMClient* GetGCMClientForTesting() const OVERRIDE;
   virtual bool IsStarted() const OVERRIDE;

components/gcm_driver/gcm_driver_desktop.cc

@@ -22,13 +22,6 @@
 
 namespace gcm {
 
-namespace {
-
-// Empty string is reserved for the default app handler.
-const char kDefaultAppHandler[] = "";
-
-}  // namespace
-
 // Helper class to save tasks to run until we're ready to execute them.
 class GCMDriverDesktop::DelayedTaskController {
  public:
@@ -463,6 +456,15 @@ void GCMDriverDesktop::RemoveAppHandler(const std::string& app_id) {
     Stop();
 }
 
+void GCMDriverDesktop::AddConnectionObserver(GCMConnectionObserver* observer) {
+  connection_observer_list_.AddObserver(observer);
+}
+
+void GCMDriverDesktop::RemoveConnectionObserver(
+    GCMConnectionObserver* observer) {
+  connection_observer_list_.RemoveObserver(observer);
+}
+
 void GCMDriverDesktop::Enable() {
   DCHECK(ui_thread_->RunsTasksOnCurrentThread());
 
@@ -759,13 +761,9 @@ void GCMDriverDesktop::OnConnected(const net::IPEndPoint& ip_endpoint) {
   if (!gcm_started_)
     return;
 
-  const GCMAppHandlerMap& app_handler_map = app_handlers();
-  for (GCMAppHandlerMap::const_iterator iter = app_handler_map.begin();
-       iter != app_handler_map.end(); ++iter) {
-    iter->second->OnConnected(ip_endpoint);
-  }
-
-  GetAppHandler(kDefaultAppHandler)->OnConnected(ip_endpoint);
+  FOR_EACH_OBSERVER(GCMConnectionObserver,
+                    connection_observer_list_,
+                    OnConnected(ip_endpoint));
 }
 
 void GCMDriverDesktop::OnDisconnected() {
@@ -777,13 +775,8 @@ void GCMDriverDesktop::OnDisconnected() {
   if (!gcm_started_)
     return;
 
-  const GCMAppHandlerMap& app_handler_map = app_handlers();
-  for (GCMAppHandlerMap::const_iterator iter = app_handler_map.begin();
-       iter != app_handler_map.end(); ++iter) {
-    iter->second->OnDisconnected();
-  }
-
-  GetAppHandler(kDefaultAppHandler)->OnDisconnected();
+  FOR_EACH_OBSERVER(
+      GCMConnectionObserver, connection_observer_list_, OnDisconnected());
 }
 
 void GCMDriverDesktop::GetGCMStatisticsFinished(

components/gcm_driver/gcm_driver_desktop.h

@@ -14,7 +14,9 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "components/gcm_driver/gcm_client.h"
+#include "components/gcm_driver/gcm_connection_observer.h"
 #include "components/gcm_driver/gcm_driver.h"
 
 namespace base {
@@ -55,6 +57,9 @@ class GCMDriverDesktop : public GCMDriver {
   virtual void AddAppHandler(const std::string& app_id,
                              GCMAppHandler* handler) OVERRIDE;
   virtual void RemoveAppHandler(const std::string& app_id) OVERRIDE;
+  virtual void AddConnectionObserver(GCMConnectionObserver* observer) OVERRIDE;
+  virtual void RemoveConnectionObserver(
+      GCMConnectionObserver* observer) OVERRIDE;
   virtual void Enable() OVERRIDE;
   virtual void Disable() OVERRIDE;
   virtual GCMClient* GetGCMClientForTesting() const OVERRIDE;
@@ -134,6 +139,10 @@ class GCMDriverDesktop : public GCMDriver {
   // it may be out of date while connection changes are happening.
   bool connected_;
 
+  // List of observers to notify when connection state changes.
+  // Makes sure list is empty on destruction.
+  ObserverList<GCMConnectionObserver, true> connection_observer_list_;
+
   scoped_refptr<base::SequencedTaskRunner> ui_thread_;
   scoped_refptr<base::SequencedTaskRunner> io_thread_;
 

components/gcm_driver/gcm_driver_desktop_unittest.cc

@@ -20,6 +20,7 @@
 #include "components/gcm_driver/fake_gcm_client_factory.h"
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/gcm_client_factory.h"
+#include "components/gcm_driver/gcm_connection_observer.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -34,6 +35,36 @@ const char kTestAppID1[] = "TestApp1";
 const char kTestAppID2[] = "TestApp2";
 const char kUserID1[] = "user1";
 
+class FakeGCMConnectionObserver : public GCMConnectionObserver {
+ public:
+  FakeGCMConnectionObserver();
+  virtual ~FakeGCMConnectionObserver();
+
+  // gcm::GCMConnectionObserver implementation:
+  virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE;
+  virtual void OnDisconnected() OVERRIDE;
+
+  bool connected() const { return connected_; }
+
+ private:
+  bool connected_;
+};
+
+FakeGCMConnectionObserver::FakeGCMConnectionObserver() : connected_(false) {
+}
+
+FakeGCMConnectionObserver::~FakeGCMConnectionObserver() {
+}
+
+void FakeGCMConnectionObserver::OnConnected(
+    const net::IPEndPoint& ip_endpoint) {
+  connected_ = true;
+}
+
+void FakeGCMConnectionObserver::OnDisconnected() {
+  connected_ = false;
+}
+
 void PumpCurrentLoop() {
   base::MessageLoop::ScopedNestableTaskAllower
       nestable_task_allower(base::MessageLoop::current());
@@ -68,6 +99,9 @@ class GCMDriverTest : public testing::Test {
 
   GCMDriver* driver() { return driver_.get(); }
   FakeGCMAppHandler* gcm_app_handler() { return gcm_app_handler_.get(); }
+  FakeGCMConnectionObserver* gcm_connection_observer() {
+    return gcm_connection_observer_.get();
+  }
   const std::string& registration_id() const { return registration_id_; }
   GCMClient::Result registration_result() const { return registration_result_; }
   const std::string& send_message_id() const { return send_message_id_; }
@@ -114,6 +148,7 @@ class GCMDriverTest : public testing::Test {
   base::FieldTrialList field_trial_list_;
   scoped_ptr<GCMDriver> driver_;
   scoped_ptr<FakeGCMAppHandler> gcm_app_handler_;
+  scoped_ptr<FakeGCMConnectionObserver> gcm_connection_observer_;
 
   base::Closure async_operation_completed_callback_;
 
@@ -147,6 +182,8 @@ void GCMDriverTest::TearDown() {
   if (!driver_)
     return;
 
+  if (gcm_connection_observer_.get())
+    driver_->RemoveConnectionObserver(gcm_connection_observer_.get());
   driver_->Shutdown();
   driver_.reset();
   PumpIOLoop();
@@ -199,6 +236,9 @@ void GCMDriverTest::CreateDriver(
       task_runner_));
 
   gcm_app_handler_.reset(new FakeGCMAppHandler);
+  gcm_connection_observer_.reset(new FakeGCMConnectionObserver);
+
+  driver_->AddConnectionObserver(gcm_connection_observer_.get());
 }
 
 void GCMDriverTest::AddAppHandlers() {
@@ -299,14 +339,14 @@ TEST_F(GCMDriverTest, Create) {
   SignIn(kTestAccountID1);
   EXPECT_FALSE(driver()->IsStarted());
   EXPECT_FALSE(driver()->IsConnected());
-  EXPECT_FALSE(gcm_app_handler()->connected());
+  EXPECT_FALSE(gcm_connection_observer()->connected());
 
   // GCM will be started only after both sign-in and app handler being added.
   AddAppHandlers();
   EXPECT_TRUE(driver()->IsStarted());
   PumpIOLoop();
   EXPECT_TRUE(driver()->IsConnected());
-  EXPECT_TRUE(gcm_app_handler()->connected());
+  EXPECT_TRUE(gcm_connection_observer()->connected());
 }
 
 TEST_F(GCMDriverTest, CreateByFieldTrial) {
@@ -316,14 +356,14 @@ TEST_F(GCMDriverTest, CreateByFieldTrial) {
   CreateDriver(FakeGCMClient::NO_DELAY_START);
   EXPECT_FALSE(driver()->IsStarted());
   EXPECT_FALSE(driver()->IsConnected());
-  EXPECT_FALSE(gcm_app_handler()->connected());
+  EXPECT_FALSE(gcm_connection_observer()->connected());
 
   // GCM will be started after app handler is added.
   AddAppHandlers();
   EXPECT_TRUE(driver()->IsStarted());
   PumpIOLoop();
   EXPECT_TRUE(driver()->IsConnected());
-  EXPECT_TRUE(gcm_app_handler()->connected());
+  EXPECT_TRUE(gcm_connection_observer()->connected());
 }
 
 TEST_F(GCMDriverTest, Shutdown) {
@@ -336,7 +376,7 @@ TEST_F(GCMDriverTest, Shutdown) {
   driver()->Shutdown();
   EXPECT_FALSE(HasAppHandlers());
   EXPECT_FALSE(driver()->IsConnected());
-  EXPECT_FALSE(gcm_app_handler()->connected());
+  EXPECT_FALSE(gcm_connection_observer()->connected());
 }
 
 TEST_F(GCMDriverTest, SignInAndSignOutOnGCMEnabled) {

components/invalidation/gcm_invalidation_bridge.cc

@@ -165,8 +165,10 @@ GCMInvalidationBridge::GCMInvalidationBridge(
       weak_factory_(this) {}
 
 GCMInvalidationBridge::~GCMInvalidationBridge() {
-  if (subscribed_for_incoming_messages_)
+  if (subscribed_for_incoming_messages_) {
     gcm_driver_->RemoveAppHandler(kInvalidationsAppId);
+    gcm_driver_->RemoveConnectionObserver(this);
+  }
 }
 
 scoped_ptr<syncer::GCMNetworkChannelDelegate>
@@ -285,6 +287,7 @@ void GCMInvalidationBridge::SubscribeForIncomingMessages() {
 
   DCHECK(!subscribed_for_incoming_messages_);
   gcm_driver_->AddAppHandler(kInvalidationsAppId, this);
+  gcm_driver_->AddConnectionObserver(this);
   core_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&GCMInvalidationBridge::Core::OnConnectionStateChanged,

components/invalidation/gcm_invalidation_bridge.h

@@ -11,6 +11,7 @@
 #include "base/threading/non_thread_safe.h"
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/gcm_client.h"
+#include "components/gcm_driver/gcm_connection_observer.h"
 #include "components/invalidation/gcm_network_channel_delegate.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 
@@ -32,6 +33,7 @@ namespace invalidation {
 // all function calls to GCMInvalidationBridge which does actual work to perform
 // them.
 class GCMInvalidationBridge : public gcm::GCMAppHandler,
+                              public gcm::GCMConnectionObserver,
                               public OAuth2TokenService::Consumer,
                               public base::NonThreadSafe {
  public:
@@ -59,6 +61,8 @@ class GCMInvalidationBridge : public gcm::GCMAppHandler,
       const gcm::GCMClient::SendErrorDetails& send_error_details) OVERRIDE;
   virtual void OnSendAcknowledged(const std::string& app_id,
                                   const std::string& message_id) OVERRIDE;
+
+  // gcm::GCMConnectionObserver implementation.
   virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE;
   virtual void OnDisconnected() OVERRIDE;