Add SendMessage to ConnectionManager

- ConnectionManager exposes SendMessage to send a message
  through the client channel.
- Adds the OnMessageReceived() to ConnectionManager's observer
  to notify clients of new messages from the client channel.

Bug: 1106937
Test: unit_tests
Change-Id: I9b2de4c7c05855deaf1ea8cebf2d01b2d4e3c69c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390972
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805085}

chromeos/components/phonehub/connection_manager.cc

@@ -23,6 +23,11 @@ void ConnectionManager::NotifyStatusChanged() {
     observer.OnStatusChanged();
 }
 
+void ConnectionManager::NotifyMessageReceived(const std::string& payload) {
+  for (auto& observer : observer_list_)
+    observer.OnMessageReceived(payload);
+}
+
 std::ostream& operator<<(std::ostream& stream,
                          ConnectionManager::Status status) {
   switch (status) {

chromeos/components/phonehub/connection_manager.h

@@ -32,7 +32,10 @@ class ConnectionManager {
     ~Observer() override = default;
 
     // Called the status has changed; use GetStatus() to get the new status.
-    virtual void OnStatusChanged() = 0;
+    virtual void OnStatusChanged() {}
+
+    // Called when a payload message has been received.
+    virtual void OnMessageReceived(const std::string& payload) {}
   };
 
   ConnectionManager(const ConnectionManager&) = delete;
@@ -46,6 +49,9 @@ class ConnectionManager {
   // the connection.
   virtual void AttemptConnection() = 0;
 
+  // Sends a message with the specified |payload|.
+  virtual void SendMessage(const std::string& payload) = 0;
+
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
@@ -53,6 +59,7 @@ class ConnectionManager {
   ConnectionManager();
 
   void NotifyStatusChanged();
+  void NotifyMessageReceived(const std::string& payload);
 
  private:
   base::ObserverList<Observer> observer_list_;

chromeos/components/phonehub/connection_manager_impl.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/components/phonehub/connection_manager_impl.h"
 
+#include "base/bind_helpers.h"
 #include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
 #include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 #include "chromeos/services/secure_channel/public/cpp/client/secure_channel_client.h"
@@ -73,6 +74,15 @@ void ConnectionManagerImpl::AttemptConnection() {
   NotifyStatusChanged();
 }
 
+void ConnectionManagerImpl::SendMessage(const std::string& payload) {
+  if (!channel_) {
+    PA_LOG(ERROR) << "SendMessage() failed because channel is null.";
+    return;
+  }
+
+  channel_->SendMessage(payload, base::DoNothing());
+}
+
 void ConnectionManagerImpl::OnConnectionAttemptFailure(
     chromeos::secure_channel::mojom::ConnectionAttemptFailureReason reason) {
   PA_LOG(WARNING) << "AttemptConnection() failed to establish connection.";
@@ -97,7 +107,7 @@ void ConnectionManagerImpl::OnDisconnected() {
 }
 
 void ConnectionManagerImpl::OnMessageReceived(const std::string& payload) {
-  // TODO(jimmyxgong): Handle the payload. This is just an empty stub.
+  NotifyMessageReceived(payload);
 }
 
 }  // namespace phonehub

chromeos/components/phonehub/connection_manager_impl.h

@@ -45,6 +45,7 @@ class ConnectionManagerImpl
   // ConnectionManager:
   ConnectionManager::Status GetStatus() const override;
   void AttemptConnection() override;
+  void SendMessage(const std::string& payload) override;
 
  private:
   // chromeos::secure_channel::ConnectionAttempt::Delegate:

chromeos/components/phonehub/connection_manager_impl_unittest.cc

@@ -26,13 +26,22 @@ class FakeObserver : public ConnectionManager::Observer {
   FakeObserver() = default;
   ~FakeObserver() override = default;
 
-  size_t num_calls() const { return num_calls_; }
+  size_t status_num_calls() const { return status_changed_num_calls_; }
+  size_t message_cb_num_calls() const { return message_received_num_calls_; }
+
+  const std::string& last_message() const { return last_message_; }
 
   // ConnectionManager::Observer:
-  void OnStatusChanged() override { ++num_calls_; }
+  void OnStatusChanged() override { ++status_changed_num_calls_; }
+  void OnMessageReceived(const std::string& payload) override {
+    last_message_ = payload;
+    ++message_received_num_calls_;
+  }
 
  private:
-  size_t num_calls_ = 0;
+  size_t status_changed_num_calls_ = 0;
+  size_t message_received_num_calls_ = 0;
+  std::string last_message_;
 };
 
 }  // namespace
@@ -73,7 +82,13 @@ class ConnectionManagerImplTest : public testing::Test {
     return connection_manager_->GetStatus();
   }
 
-  size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
+  size_t GetNumStatusObserverCalls() const {
+    return fake_observer_.status_num_calls();
+  }
+
+  size_t GetNumMessageReceivedObserverCalls() const {
+    return fake_observer_.message_cb_num_calls();
+  }
 
   void CreateFakeConnectionAttempt() {
     auto fake_connection_attempt =
@@ -101,7 +116,7 @@ TEST_F(ConnectionManagerImplTest, SuccessfullyAttemptConnection) {
 
   // Status has been updated to connecting, verify that the status observer
   // has been called.
-  EXPECT_EQ(1u, GetNumObserverCalls());
+  EXPECT_EQ(1u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kConnecting, GetStatus());
 
   auto fake_client_channel =
@@ -110,7 +125,7 @@ TEST_F(ConnectionManagerImplTest, SuccessfullyAttemptConnection) {
 
   // Status has been updated to connected, verify that the status observer has
   // been called.
-  EXPECT_EQ(2u, GetNumObserverCalls());
+  EXPECT_EQ(2u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kConnected, GetStatus());
 }
 
@@ -120,7 +135,7 @@ TEST_F(ConnectionManagerImplTest, FailedToAttemptConnection) {
 
   // Status has been updated to connecting, verify that the status observer
   // has been called.
-  EXPECT_EQ(1u, GetNumObserverCalls());
+  EXPECT_EQ(1u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kConnecting, GetStatus());
 
   fake_connection_attempt_->NotifyConnectionAttemptFailure(
@@ -129,7 +144,7 @@ TEST_F(ConnectionManagerImplTest, FailedToAttemptConnection) {
 
   // Status has been updated to disconnected, verify that the status observer
   // has been called.
-  EXPECT_EQ(2u, GetNumObserverCalls());
+  EXPECT_EQ(2u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kDisconnected, GetStatus());
 }
 
@@ -139,7 +154,7 @@ TEST_F(ConnectionManagerImplTest, SuccessfulAttemptConnectionButDisconnected) {
 
   // Status has been updated to connecting, verify that the status observer
   // has been called.
-  EXPECT_EQ(1u, GetNumObserverCalls());
+  EXPECT_EQ(1u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kConnecting, GetStatus());
 
   auto fake_client_channel =
@@ -150,17 +165,46 @@ TEST_F(ConnectionManagerImplTest, SuccessfulAttemptConnectionButDisconnected) {
 
   // Status has been updated to connected, verify that the status observer has
   // been called.
-  EXPECT_EQ(2u, GetNumObserverCalls());
+  EXPECT_EQ(2u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kConnected, GetStatus());
 
   // Simulate a disconnected channel.
   fake_client_channel_raw->NotifyDisconnected();
 
   // Expect status to be updated to disconnected.
-  EXPECT_EQ(3u, GetNumObserverCalls());
+  EXPECT_EQ(3u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kDisconnected, GetStatus());
 }
 
+TEST_F(ConnectionManagerImplTest, AttemptConnectionWithMessageReceived) {
+  CreateFakeConnectionAttempt();
+  connection_manager_->AttemptConnection();
+
+  // Status has been updated to connecting, verify that the status observer
+  // has been called.
+  EXPECT_EQ(1u, GetNumStatusObserverCalls());
+  EXPECT_EQ(ConnectionManager::Status::kConnecting, GetStatus());
+
+  auto fake_client_channel =
+      std::make_unique<chromeos::secure_channel::FakeClientChannel>();
+  chromeos::secure_channel::FakeClientChannel* fake_client_channel_raw =
+      fake_client_channel.get();
+  fake_connection_attempt_->NotifyConnection(std::move(fake_client_channel));
+
+  // Status has been updated to connected, verify that the status observer has
+  // been called.
+  EXPECT_EQ(2u, GetNumStatusObserverCalls());
+  EXPECT_EQ(ConnectionManager::Status::kConnected, GetStatus());
+
+  // Simulate a message being sent.
+  const std::string expected_payload = "payload";
+  fake_client_channel_raw->NotifyMessageReceived(expected_payload);
+
+  // Expected MessageReceived() callback to be called.
+  EXPECT_EQ(1u, GetNumMessageReceivedObserverCalls());
+  EXPECT_EQ(expected_payload, fake_observer_.last_message());
+}
+
 TEST_F(ConnectionManagerImplTest, AttemptConnectionWithoutLocalDevice) {
   // Simulate a missing local device.
   fake_device_sync_client_.set_local_device_metadata(
@@ -169,7 +213,7 @@ TEST_F(ConnectionManagerImplTest, AttemptConnectionWithoutLocalDevice) {
 
   // Status is still disconnected since there is a missing device, verify that
   // the status observer did not get called (exited early).
-  EXPECT_EQ(0u, GetNumObserverCalls());
+  EXPECT_EQ(0u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kDisconnected, GetStatus());
 }
 
@@ -182,7 +226,7 @@ TEST_F(ConnectionManagerImplTest, AttemptConnectionWithoutRemoteDevice) {
 
   // Status is still disconnected since there is a missing device, verify that
   // the status observer did not get called (exited early).
-  EXPECT_EQ(0u, GetNumObserverCalls());
+  EXPECT_EQ(0u, GetNumStatusObserverCalls());
   EXPECT_EQ(ConnectionManager::Status::kDisconnected, GetStatus());
 }
 

chromeos/components/phonehub/fake_connection_manager.cc

@@ -29,5 +29,9 @@ void FakeConnectionManager::AttemptConnection() {
     SetStatus(Status::kConnecting);
 }
 
+void FakeConnectionManager::SendMessage(const std::string& payload) {
+  sent_messages_.push_back(payload);
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/fake_connection_manager.h

@@ -5,6 +5,7 @@
 #ifndef CHROMEOS_COMPONENTS_PHONEHUB_FAKE_CONNECTION_MANAGER_H_
 #define CHROMEOS_COMPONENTS_PHONEHUB_FAKE_CONNECTION_MANAGER_H_
 
+#include <vector>
 #include "chromeos/components/phonehub/connection_manager.h"
 
 namespace chromeos {
@@ -15,14 +16,21 @@ class FakeConnectionManager : public ConnectionManager {
   FakeConnectionManager();
   ~FakeConnectionManager() override;
 
+  using ConnectionManager::NotifyMessageReceived;
+
   void SetStatus(Status status);
+  const std::vector<std::string>& sent_messages() const {
+    return sent_messages_;
+  }
 
  private:
   // ConnectionManager:
   Status GetStatus() const override;
   void AttemptConnection() override;
+  void SendMessage(const std::string& payload) override;
 
   Status status_;
+  std::vector<std::string> sent_messages_;
 };
 
 }  // namespace phonehub