[CrOS Multidevice] Make AuthenticatedChannel::GetConnectionMetadata() async.

This change allows clients which need to frequently poll SecureChannel API
for connection metadata updates (e.g., Smart Lock) to do so.

Bug: 824568, 752273
Change-Id: Iad7cd7d9f37e22563ee46fc257b4f146ce260a60
Reviewed-on: https://chromium-review.googlesource.com/1091223
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565409}

chromeos/services/secure_channel/authenticated_channel_impl.cc

@@ -61,12 +61,9 @@ AuthenticatedChannelImpl::~AuthenticatedChannelImpl() {
   secure_channel_->RemoveObserver(this);
 }
 
-const mojom::ConnectionMetadata&
-AuthenticatedChannelImpl::GetConnectionMetadata() const {
-  // TODO(khorimoto): Update |connection_metadata_.rssi_rolling_average|
-  // periodically throughout the connection when applicable. For now,
-  // kNoRssiAvailable is used in all cases.
-  return connection_metadata_;
+void AuthenticatedChannelImpl::GetConnectionMetadata(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  std::move(callback).Run(connection_metadata_);
 }
 
 void AuthenticatedChannelImpl::PerformSendMessage(

chromeos/services/secure_channel/authenticated_channel_impl.h

@@ -47,7 +47,8 @@ class AuthenticatedChannelImpl : public AuthenticatedChannel,
       std::unique_ptr<cryptauth::SecureChannel> secure_channel);
 
   // AuthenticatedChannel:
-  const mojom::ConnectionMetadata& GetConnectionMetadata() const override;
+  void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) override;
   void PerformSendMessage(const std::string& feature,
                           const std::string& payload,
                           base::OnceClosure on_sent_callback) final;

chromeos/services/secure_channel/authenticated_channel_impl_unittest.cc

@@ -94,6 +94,16 @@ class SecureChannelAuthenticatedChannelImplTest : public testing::Test {
     return base::ContainsKey(sent_sequence_numbers_, sequence_number);
   }
 
+  void CallGetConnectionMetadata() {
+    channel()->GetConnectionMetadata(base::BindOnce(
+        &SecureChannelAuthenticatedChannelImplTest::OnGetConnectionMetadata,
+        base::Unretained(this)));
+  }
+
+  void OnGetConnectionMetadata(mojom::ConnectionMetadata connection_metadata) {
+    connection_metadata_ = std::move(connection_metadata);
+  }
+
   cryptauth::FakeSecureChannel* fake_secure_channel() {
     return fake_secure_channel_;
   }
@@ -104,6 +114,8 @@ class SecureChannelAuthenticatedChannelImplTest : public testing::Test {
 
   AuthenticatedChannel* channel() { return channel_.get(); }
 
+  base::Optional<mojom::ConnectionMetadata> connection_metadata_;
+
  private:
   void OnMessageSent(int sequence_number) {
     sent_sequence_numbers_.insert(sequence_number);
@@ -125,15 +137,16 @@ class SecureChannelAuthenticatedChannelImplTest : public testing::Test {
 };
 
 TEST_F(SecureChannelAuthenticatedChannelImplTest, ConnectionMetadata) {
-  const auto& connection_metadata = channel()->GetConnectionMetadata();
+  CallGetConnectionMetadata();
+
   EXPECT_EQ(std::vector<mojom::ConnectionCreationDetail>(
                 std::begin(kTestConnectionCreationDetails),
                 std::end(kTestConnectionCreationDetails)),
-            connection_metadata.creation_details);
+            connection_metadata_->creation_details);
   // TODO(khorimoto): Update test to test RSSI rolling average when implemented.
   // https://crbug.com/844759.
   EXPECT_EQ(mojom::ConnectionMetadata::kNoRssiAvailable,
-            connection_metadata.rssi_rolling_average);
+            connection_metadata_->rssi_rolling_average);
 }
 
 TEST_F(SecureChannelAuthenticatedChannelImplTest, DisconnectRequestFromClient) {

chromeos/services/secure_channel/channel_impl.cc

@@ -15,7 +15,7 @@ const char kReasonForDisconnection[] = "Remote device disconnected.";
 }  // namespace
 
 ChannelImpl::ChannelImpl(Delegate* delegate)
-    : delegate_(delegate), binding_(this) {}
+    : delegate_(delegate), binding_(this), weak_ptr_factory_(this) {}
 
 ChannelImpl::~ChannelImpl() = default;
 
@@ -48,7 +48,15 @@ void ChannelImpl::SendMessage(const std::string& message,
 
 void ChannelImpl::GetConnectionMetadata(
     GetConnectionMetadataCallback callback) {
-  std::move(callback).Run(delegate_->GetConnectionMetadata().Clone());
+  delegate_->GetConnectionMetadata(
+      base::BindOnce(&ChannelImpl::OnConnectionMetadataFetchedFromDelegate,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void ChannelImpl::OnConnectionMetadataFetchedFromDelegate(
+    GetConnectionMetadataCallback callback,
+    mojom::ConnectionMetadata connection_metadata_from_delegate) {
+  std::move(callback).Run(connection_metadata_from_delegate.Clone());
 }
 
 void ChannelImpl::OnBindingDisconnected() {

chromeos/services/secure_channel/channel_impl.h

@@ -9,6 +9,7 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -28,7 +29,8 @@ class ChannelImpl : public mojom::Channel {
     virtual ~Delegate() = default;
     virtual void OnSendMessageRequested(const std::string& message,
                                         base::OnceClosure on_sent_callback) = 0;
-    virtual const mojom::ConnectionMetadata& GetConnectionMetadata() = 0;
+    virtual void GetConnectionMetadata(
+        base::OnceCallback<void(mojom::ConnectionMetadata)> callback) = 0;
     virtual void OnClientDisconnected() = 0;
   };
 
@@ -49,11 +51,17 @@ class ChannelImpl : public mojom::Channel {
                    SendMessageCallback callback) override;
   void GetConnectionMetadata(GetConnectionMetadataCallback callback) override;
 
+  void OnConnectionMetadataFetchedFromDelegate(
+      GetConnectionMetadataCallback callback,
+      mojom::ConnectionMetadata connection_metadata_from_delegate);
+
   void OnBindingDisconnected();
 
   Delegate* delegate_;
   mojo::Binding<mojom::Channel> binding_;
 
+  base::WeakPtrFactory<ChannelImpl> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ChannelImpl);
 };
 

chromeos/services/secure_channel/fake_single_client_message_proxy.cc

@@ -48,9 +48,9 @@ void FakeSingleClientMessageProxyDelegate::OnSendMessageRequested(
       message_feaure, message_payload, std::move(on_sent_callback)));
 }
 
-const mojom::ConnectionMetadata&
-FakeSingleClientMessageProxyDelegate::GetConnectionMetadata() {
-  return connection_metadata_;
+void FakeSingleClientMessageProxyDelegate::GetConnectionMetadata(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  return std::move(callback).Run(connection_metadata_);
 }
 
 void FakeSingleClientMessageProxyDelegate::OnClientDisconnected(

chromeos/services/secure_channel/fake_single_client_message_proxy.h

@@ -89,7 +89,8 @@ class FakeSingleClientMessageProxyDelegate
   void OnSendMessageRequested(const std::string& message_feaure,
                               const std::string& message_payload,
                               base::OnceClosure on_sent_callback) override;
-  const mojom::ConnectionMetadata& GetConnectionMetadata() override;
+  void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) override;
   void OnClientDisconnected(const base::UnguessableToken& proxy_id) override;
 
   std::vector<std::tuple<std::string, std::string, base::OnceClosure>>

chromeos/services/secure_channel/multiplexed_channel_impl.cc

@@ -117,9 +117,9 @@ void MultiplexedChannelImpl::OnSendMessageRequested(
                                       std::move(on_sent_callback));
 }
 
-const mojom::ConnectionMetadata&
-MultiplexedChannelImpl::GetConnectionMetadata() {
-  return authenticated_channel_->GetConnectionMetadata();
+void MultiplexedChannelImpl::GetConnectionMetadata(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  authenticated_channel_->GetConnectionMetadata(std::move(callback));
 }
 
 void MultiplexedChannelImpl::OnClientDisconnected(

chromeos/services/secure_channel/multiplexed_channel_impl.h

@@ -73,7 +73,8 @@ class MultiplexedChannelImpl : public MultiplexedChannel,
   void OnSendMessageRequested(const std::string& message_feaure,
                               const std::string& message_payload,
                               base::OnceClosure on_sent_callback) override;
-  const mojom::ConnectionMetadata& GetConnectionMetadata() override;
+  void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) override;
   void OnClientDisconnected(const base::UnguessableToken& proxy_id) override;
 
   std::unique_ptr<AuthenticatedChannel> authenticated_channel_;

chromeos/services/secure_channel/multiplexed_channel_impl_unittest.cc

@@ -256,6 +256,17 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
     EXPECT_TRUE(success);
   }
 
+  void CallGetConnectionMetadataFromDelegate(
+      FakeSingleClientMessageProxy* proxy) {
+    proxy->GetConnectionMetadataFromDelegate(base::BindOnce(
+        &SecureChannelMultiplexedChannelImplTest::OnGetConnectionMetadata,
+        base::Unretained(this)));
+  }
+
+  void OnGetConnectionMetadata(mojom::ConnectionMetadata connection_metadata) {
+    connection_metadata_ = std::move(connection_metadata);
+  }
+
   std::vector<std::unique_ptr<ClientConnectionParameters>>&
   initial_client_list() {
     return initial_client_list_;
@@ -277,6 +288,8 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
     return fake_authenticated_channel_;
   }
 
+  base::Optional<mojom::ConnectionMetadata> connection_metadata_;
+
  private:
   void OnMessageSent(int message_counter) {
     sent_message_counters_.insert(message_counter);
@@ -314,15 +327,11 @@ TEST_F(SecureChannelMultiplexedChannelImplTest, ConnectionMetadata) {
   // Retrieving the metadata through the proxy should cause
   // |fake_authenticated_channel_|'s metadata to be passed through
   // |multiplexed_channel_|.
-  EXPECT_EQ(details, id_to_active_proxy_map()
-                         .begin()
-                         ->second->GetConnectionMetadataFromDelegate()
-                         .creation_details);
+  CallGetConnectionMetadataFromDelegate(
+      id_to_active_proxy_map().begin()->second);
+  EXPECT_EQ(details, connection_metadata_->creation_details);
   EXPECT_EQ(mojom::ConnectionMetadata::kNoRssiAvailable,
-            id_to_active_proxy_map()
-                .begin()
-                ->second->GetConnectionMetadataFromDelegate()
-                .rssi_rolling_average);
+            connection_metadata_->rssi_rolling_average);
 
   // Now, change the values and set them on |fake_authenticated_channel_|.
   connection_metadata.creation_details =
@@ -331,15 +340,11 @@ TEST_F(SecureChannelMultiplexedChannelImplTest, ConnectionMetadata) {
   fake_authenticated_channel()->set_connection_metadata(connection_metadata);
 
   // The new updates should be available.
+  CallGetConnectionMetadataFromDelegate(
+      id_to_active_proxy_map().begin()->second);
   EXPECT_EQ(std::vector<mojom::ConnectionCreationDetail>(),
-            id_to_active_proxy_map()
-                .begin()
-                ->second->GetConnectionMetadataFromDelegate()
-                .creation_details);
-  EXPECT_EQ(-5.5f, id_to_active_proxy_map()
-                       .begin()
-                       ->second->GetConnectionMetadataFromDelegate()
-                       .rssi_rolling_average);
+            connection_metadata_->creation_details);
+  EXPECT_EQ(-5.5f, connection_metadata_->rssi_rolling_average);
 
   DisconnectClientAndVerifyState(id_to_active_proxy_map().begin()->second,
                                  true /* expected_to_be_last_client */);

chromeos/services/secure_channel/public/cpp/shared/authenticated_channel.h

@@ -31,7 +31,8 @@ class AuthenticatedChannel {
 
   virtual ~AuthenticatedChannel();
 
-  virtual const mojom::ConnectionMetadata& GetConnectionMetadata() const = 0;
+  virtual void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) = 0;
 
   // Sends a message with the specified |feature| and |payload|. Once the
   // message has been sent, |on_sent_callback| will be invoked. Returns whether

chromeos/services/secure_channel/public/cpp/shared/fake_authenticated_channel.cc

@@ -12,9 +12,9 @@ FakeAuthenticatedChannel::FakeAuthenticatedChannel() : AuthenticatedChannel() {}
 
 FakeAuthenticatedChannel::~FakeAuthenticatedChannel() = default;
 
-const mojom::ConnectionMetadata&
-FakeAuthenticatedChannel::GetConnectionMetadata() const {
-  return connection_metadata_;
+void FakeAuthenticatedChannel::GetConnectionMetadata(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  return std::move(callback).Run(connection_metadata_);
 }
 
 void FakeAuthenticatedChannel::PerformSendMessage(

chromeos/services/secure_channel/public/cpp/shared/fake_authenticated_channel.h

@@ -40,7 +40,8 @@ class FakeAuthenticatedChannel : public AuthenticatedChannel {
   }
 
   // AuthenticatedChannel:
-  const mojom::ConnectionMetadata& GetConnectionMetadata() const override;
+  void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) override;
   void PerformSendMessage(const std::string& feature,
                           const std::string& payload,
                           base::OnceClosure on_sent_callback) override;

chromeos/services/secure_channel/single_client_message_proxy.cc

@@ -25,9 +25,9 @@ void SingleClientMessageProxy::NotifyClientDisconnected() {
   delegate_->OnClientDisconnected(GetProxyId());
 }
 
-const mojom::ConnectionMetadata&
-SingleClientMessageProxy::GetConnectionMetadataFromDelegate() {
-  return delegate_->GetConnectionMetadata();
+void SingleClientMessageProxy::GetConnectionMetadataFromDelegate(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  delegate_->GetConnectionMetadata(std::move(callback));
 }
 
 }  // namespace secure_channel

chromeos/services/secure_channel/single_client_message_proxy.h

@@ -28,7 +28,8 @@ class SingleClientMessageProxy {
     virtual void OnSendMessageRequested(const std::string& message_feaure,
                                         const std::string& message_payload,
                                         base::OnceClosure on_sent_callback) = 0;
-    virtual const mojom::ConnectionMetadata& GetConnectionMetadata() = 0;
+    virtual void GetConnectionMetadata(
+        base::OnceCallback<void(mojom::ConnectionMetadata)> callback) = 0;
     virtual void OnClientDisconnected(
         const base::UnguessableToken& proxy_id) = 0;
   };
@@ -53,7 +54,8 @@ class SingleClientMessageProxy {
                                   const std::string& message_payload,
                                   base::OnceClosure on_sent_callback);
   void NotifyClientDisconnected();
-  const mojom::ConnectionMetadata& GetConnectionMetadataFromDelegate();
+  void GetConnectionMetadataFromDelegate(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback);
 
  private:
   Delegate* delegate_;

chromeos/services/secure_channel/single_client_message_proxy_impl.cc

@@ -79,9 +79,9 @@ void SingleClientMessageProxyImpl::OnSendMessageRequested(
                              std::move(on_sent_callback));
 }
 
-const mojom::ConnectionMetadata&
-SingleClientMessageProxyImpl::GetConnectionMetadata() {
-  return GetConnectionMetadataFromDelegate();
+void SingleClientMessageProxyImpl::GetConnectionMetadata(
+    base::OnceCallback<void(mojom::ConnectionMetadata)> callback) {
+  GetConnectionMetadataFromDelegate(std::move(callback));
 }
 
 void SingleClientMessageProxyImpl::OnClientDisconnected() {

chromeos/services/secure_channel/single_client_message_proxy_impl.h

@@ -57,7 +57,8 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy,
   // ChannelImpl::Delegate:
   void OnSendMessageRequested(const std::string& message,
                               base::OnceClosure on_sent_callback) override;
-  const mojom::ConnectionMetadata& GetConnectionMetadata() override;
+  void GetConnectionMetadata(
+      base::OnceCallback<void(mojom::ConnectionMetadata)> callback) override;
   void OnClientDisconnected() override;
 
   void FlushForTesting();