Commit 66f0b915 authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS MultiDevice] Add ClientConnectionParameters.

This class encapsulates the parameters passed by clients of
SecureChannel which are meant to be tightly coupled to an individual
connection request.

Encapulsulating these within a class makes clients of the relevant
classes much less complex and allows PendingConnectionRequest and
SingleClientMessageProxy to use the ID associated with the parameters.

Bug: 824568, 752273
Change-Id: I9da3824fef3ede7727b5cdccb879ee2b49140807
Reviewed-on: https://chromium-review.googlesource.com/1070990
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561548}
parent 8e4de903
...@@ -12,6 +12,8 @@ static_library("secure_channel") { ...@@ -12,6 +12,8 @@ static_library("secure_channel") {
"authenticated_channel_impl.h", "authenticated_channel_impl.h",
"channel_impl.cc", "channel_impl.cc",
"channel_impl.h", "channel_impl.h",
"client_connection_parameters.cc",
"client_connection_parameters.h",
"connect_to_device_operation.h", "connect_to_device_operation.h",
"connect_to_device_operation_base.h", "connect_to_device_operation_base.h",
"connect_to_device_operation_factory.h", "connect_to_device_operation_factory.h",
......
// Copyright 2018 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 "chromeos/services/secure_channel/client_connection_parameters.h"
namespace chromeos {
namespace secure_channel {
ClientConnectionParameters::ClientConnectionParameters(
const std::string& feature,
mojom::ConnectionDelegatePtr connection_delegate_ptr)
: feature_(feature),
connection_delegate_ptr_(std::move(connection_delegate_ptr)),
id_(base::UnguessableToken::Create()) {
DCHECK(!feature_.empty());
DCHECK(connection_delegate_ptr_);
}
ClientConnectionParameters::ClientConnectionParameters(
ClientConnectionParameters&& other)
: feature_(other.feature_),
connection_delegate_ptr_(std::move(other.connection_delegate_ptr_)),
id_(other.id_) {}
ClientConnectionParameters& ClientConnectionParameters::operator=(
ClientConnectionParameters&& other) {
feature_ = other.feature_;
connection_delegate_ptr_ = std::move(other.connection_delegate_ptr_);
id_ = other.id_;
return *this;
}
ClientConnectionParameters::~ClientConnectionParameters() = default;
bool ClientConnectionParameters::operator==(
const ClientConnectionParameters& other) const {
return feature() == other.feature() && id() == other.id() &&
connection_delegate_ptr_.get() == other.connection_delegate_ptr_.get();
}
bool ClientConnectionParameters::operator<(
const ClientConnectionParameters& other) const {
if (feature() != other.feature())
return feature() < other.feature();
if (id() != other.id())
return id() < other.id();
// Use pointer comparison of proxies.
return connection_delegate_ptr_.get() < other.connection_delegate_ptr_.get();
}
std::ostream& operator<<(std::ostream& stream,
const ClientConnectionParameters& details) {
stream << "{feature: \"" << details.feature() << "\", "
<< "id: \"" << details.id().ToString() << "\"}";
return stream;
}
} // namespace secure_channel
} // namespace chromeos
// Copyright 2018 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 CHROMEOS_SERVICES_SECURE_CHANNEL_CLIENT_CONNECTION_PARAMETERS_H_
#define CHROMEOS_SERVICES_SECURE_CHANNEL_CLIENT_CONNECTION_PARAMETERS_H_
#include <ostream>
#include <string>
#include "base/macros.h"
#include "base/unguessable_token.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
namespace chromeos {
namespace secure_channel {
// Parameters associated with a client request, which should be tightly-coupled
// to the associated communication channel.
class ClientConnectionParameters {
public:
ClientConnectionParameters(
const std::string& feature,
mojom::ConnectionDelegatePtr connection_delegate_ptr);
ClientConnectionParameters(ClientConnectionParameters&& other);
ClientConnectionParameters& operator=(ClientConnectionParameters&& other);
virtual ~ClientConnectionParameters();
const base::UnguessableToken& id() const { return id_; }
const std::string& feature() const { return feature_; }
mojom::ConnectionDelegatePtr& connection_delegate_ptr() {
return connection_delegate_ptr_;
}
bool operator==(const ClientConnectionParameters& other) const;
bool operator<(const ClientConnectionParameters& other) const;
private:
std::string feature_;
mojom::ConnectionDelegatePtr connection_delegate_ptr_;
base::UnguessableToken id_;
DISALLOW_COPY_AND_ASSIGN(ClientConnectionParameters);
};
std::ostream& operator<<(std::ostream& stream,
const ClientConnectionParameters& details);
} // namespace secure_channel
} // namespace chromeos
#endif // CHROMEOS_SERVICES_SECURE_CHANNEL_CLIENT_CONNECTION_PARAMETERS_H_
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "chromeos/components/proximity_auth/logging/logging.h" #include "chromeos/components/proximity_auth/logging/logging.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/connection_attempt_delegate.h" #include "chromeos/services/secure_channel/connection_attempt_delegate.h"
#include "chromeos/services/secure_channel/connection_details.h" #include "chromeos/services/secure_channel/connection_details.h"
#include "chromeos/services/secure_channel/pending_connection_request.h" #include "chromeos/services/secure_channel/pending_connection_request.h"
...@@ -29,13 +30,13 @@ class AuthenticatedChannel; ...@@ -29,13 +30,13 @@ class AuthenticatedChannel;
template <typename FailureDetailType> template <typename FailureDetailType>
class ConnectionAttempt { class ConnectionAttempt {
public: public:
// Extracts all of the features and ConnectionDelegates owned by |attempt|'s // Extracts all of the ClientConnectionParameters owned by |attempt|'s
// PendingConnectionRequests. This function deletes |attempt| as part of this // PendingConnectionRequests. This function deletes |attempt| as part of this
// process to ensure that it is no longer used after extraction is complete. // process to ensure that it is no longer used after extraction is complete.
static std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> static std::vector<ClientConnectionParameters>
ExtractClientData( ExtractClientConnectionParameters(
std::unique_ptr<ConnectionAttempt<FailureDetailType>> attempt) { std::unique_ptr<ConnectionAttempt<FailureDetailType>> attempt) {
return attempt->ExtractClientData(); return attempt->ExtractClientConnectionParameters();
} }
virtual ~ConnectionAttempt() = default; virtual ~ConnectionAttempt() = default;
...@@ -78,10 +79,10 @@ class ConnectionAttempt { ...@@ -78,10 +79,10 @@ class ConnectionAttempt {
virtual void ProcessAddingNewConnectionRequest( virtual void ProcessAddingNewConnectionRequest(
std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request) = 0; std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request) = 0;
// Extracts the features and ConnectionDelegates from all child // Extracts the ClientConnectionParameters from all child
// PendingConnectionRequests. // PendingConnectionRequests.
virtual std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> virtual std::vector<ClientConnectionParameters>
ExtractClientData() = 0; ExtractClientConnectionParameters() = 0;
void OnConnectionAttemptSucceeded( void OnConnectionAttemptSucceeded(
std::unique_ptr<AuthenticatedChannel> authenticated_channel) { std::unique_ptr<AuthenticatedChannel> authenticated_channel) {
......
...@@ -71,7 +71,7 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>, ...@@ -71,7 +71,7 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>,
void ProcessAddingNewConnectionRequest( void ProcessAddingNewConnectionRequest(
std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request) std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request)
override { override {
if (base::ContainsKey(id_to_request_map_, request->request_id())) { if (base::ContainsKey(id_to_request_map_, request->GetRequestId())) {
PA_LOG(ERROR) << "ConnectionAttemptBase::" PA_LOG(ERROR) << "ConnectionAttemptBase::"
<< "ProcessAddingNewConnectionRequest(): Processing " << "ProcessAddingNewConnectionRequest(): Processing "
<< "request whose ID has already been processed."; << "request whose ID has already been processed.";
...@@ -79,7 +79,7 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>, ...@@ -79,7 +79,7 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>,
} }
bool was_empty = id_to_request_map_.empty(); bool was_empty = id_to_request_map_.empty();
id_to_request_map_[request->request_id()] = std::move(request); id_to_request_map_[request->GetRequestId()] = std::move(request);
// In the case that this ConnectionAttempt was just created and had not yet // In the case that this ConnectionAttempt was just created and had not yet
// received a request yet, start up an operation. // received a request yet, start up an operation.
...@@ -87,13 +87,13 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>, ...@@ -87,13 +87,13 @@ class ConnectionAttemptBase : public ConnectionAttempt<FailureDetailType>,
StartNextConnectToDeviceOperation(); StartNextConnectToDeviceOperation();
} }
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> ExtractClientConnectionParameters()
ExtractClientData() override { override {
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> data_list; std::vector<ClientConnectionParameters> data_list;
for (auto& map_entry : id_to_request_map_) { for (auto& map_entry : id_to_request_map_) {
data_list.push_back( data_list.push_back(
PendingConnectionRequest<FailureDetailType>::ExtractClientData( PendingConnectionRequest<FailureDetailType>::
std::move(map_entry.second))); ExtractClientConnectionParameters(std::move(map_entry.second)));
} }
return data_list; return data_list;
} }
......
...@@ -69,8 +69,8 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test { ...@@ -69,8 +69,8 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test {
} }
void TearDown() override { void TearDown() override {
// ExtractClientData() tests destroy |connection_attempt_|, so no additional // ExtractClientConnectionParameters() tests destroy |connection_attempt_|,
// verifications should be performed. // so no additional verifications should be performed.
if (is_extract_client_data_test_) if (is_extract_client_data_test_)
return; return;
...@@ -138,7 +138,7 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test { ...@@ -138,7 +138,7 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test {
base::UnguessableTokenHash> base::UnguessableTokenHash>
id_to_num_details_map; id_to_num_details_map;
for (const auto* request : active_requests_) { for (const auto* request : active_requests_) {
id_to_num_details_map[request->request_id()] = id_to_num_details_map[request->GetRequestId()] =
request->handled_failure_details().size(); request->handled_failure_details().size();
} }
...@@ -149,7 +149,7 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test { ...@@ -149,7 +149,7 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test {
// Now, ensure that each active request had one additional failure detail // Now, ensure that each active request had one additional failure detail
// added, and verify that it was |kTestFailureDetail|. // added, and verify that it was |kTestFailureDetail|.
for (const auto* request : active_requests_) { for (const auto* request : active_requests_) {
EXPECT_EQ(id_to_num_details_map[request->request_id()] + 1, EXPECT_EQ(id_to_num_details_map[request->GetRequestId()] + 1,
request->handled_failure_details().size()); request->handled_failure_details().size());
EXPECT_EQ(kTestFailureDetail, request->handled_failure_details().back()); EXPECT_EQ(kTestFailureDetail, request->handled_failure_details().back());
} }
...@@ -193,10 +193,9 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test { ...@@ -193,10 +193,9 @@ class SecureChannelConnectionAttemptBaseTest : public testing::Test {
EXPECT_FALSE(fake_delegate_->authenticated_channel()); EXPECT_FALSE(fake_delegate_->authenticated_channel());
} }
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> ExtractClientConnectionParameters() {
ExtractClientData() {
is_extract_client_data_test_ = true; is_extract_client_data_test_ = true;
return ConnectionAttempt<std::string>::ExtractClientData( return ConnectionAttempt<std::string>::ExtractClientConnectionParameters(
std::move(connection_attempt_)); std::move(connection_attempt_));
} }
...@@ -360,13 +359,14 @@ TEST_F(SecureChannelConnectionAttemptBaseTest, TwoRequests_FailThenSuccess) { ...@@ -360,13 +359,14 @@ TEST_F(SecureChannelConnectionAttemptBaseTest, TwoRequests_FailThenSuccess) {
FinishOperationSuccessfully(operation3); FinishOperationSuccessfully(operation3);
} }
TEST_F(SecureChannelConnectionAttemptBaseTest, ExtractClientData) { TEST_F(SecureChannelConnectionAttemptBaseTest,
ExtractClientConnectionParameters) {
FakePendingConnectionRequest* request1 = AddNewRequest(); FakePendingConnectionRequest* request1 = AddNewRequest();
auto fake_connection_delegate_1 = std::make_unique<FakeConnectionDelegate>(); auto fake_connection_delegate_1 = std::make_unique<FakeConnectionDelegate>();
auto fake_connection_delegate_ptr_1 = auto fake_connection_delegate_ptr_1 =
fake_connection_delegate_1->GenerateInterfacePtr(); fake_connection_delegate_1->GenerateInterfacePtr();
auto* fake_connection_delegate_proxy_1 = fake_connection_delegate_ptr_1.get(); auto* fake_connection_delegate_proxy_1 = fake_connection_delegate_ptr_1.get();
request1->set_client_data_for_extraction(std::make_pair( request1->set_client_data_for_extraction(ClientConnectionParameters(
"request1Feature", std::move(fake_connection_delegate_ptr_1))); "request1Feature", std::move(fake_connection_delegate_ptr_1)));
FakePendingConnectionRequest* request2 = AddNewRequest(); FakePendingConnectionRequest* request2 = AddNewRequest();
...@@ -374,27 +374,24 @@ TEST_F(SecureChannelConnectionAttemptBaseTest, ExtractClientData) { ...@@ -374,27 +374,24 @@ TEST_F(SecureChannelConnectionAttemptBaseTest, ExtractClientData) {
auto fake_connection_delegate_ptr_2 = auto fake_connection_delegate_ptr_2 =
fake_connection_delegate_2->GenerateInterfacePtr(); fake_connection_delegate_2->GenerateInterfacePtr();
auto* fake_connection_delegate_proxy_2 = fake_connection_delegate_ptr_2.get(); auto* fake_connection_delegate_proxy_2 = fake_connection_delegate_ptr_2.get();
request2->set_client_data_for_extraction(std::make_pair( request2->set_client_data_for_extraction(ClientConnectionParameters(
"request2Feature", std::move(fake_connection_delegate_ptr_2))); "request2Feature", std::move(fake_connection_delegate_ptr_2)));
auto extracted_client_data = ExtractClientData(); auto extracted_client_data = ExtractClientConnectionParameters();
EXPECT_EQ(2u, extracted_client_data.size()); EXPECT_EQ(2u, extracted_client_data.size());
// The extracted client data may not be returned in the same order that as the // The extracted client data may not be returned in the same order that as the
// associated requests were added to |conenction_attempt_|, since // associated requests were added to |conenction_attempt_|, since
// ConnectionAttemptBase internally utilizes a std::unordered_map. Sort the // ConnectionAttemptBase internally utilizes a std::unordered_map. Sort the
// data before making verifications to ensure correctness. // data before making verifications to ensure correctness.
std::sort(extracted_client_data.begin(), extracted_client_data.end(), std::sort(extracted_client_data.begin(), extracted_client_data.end());
[](const auto& client_data_pair_1, const auto& client_data_pair_2) {
return client_data_pair_1.first < client_data_pair_2.first;
});
EXPECT_EQ("request1Feature", extracted_client_data[0].first); EXPECT_EQ("request1Feature", extracted_client_data[0].feature());
EXPECT_EQ(fake_connection_delegate_proxy_1, EXPECT_EQ(fake_connection_delegate_proxy_1,
extracted_client_data[0].second.get()); extracted_client_data[0].connection_delegate_ptr().get());
EXPECT_EQ("request2Feature", extracted_client_data[1].first); EXPECT_EQ("request2Feature", extracted_client_data[1].feature());
EXPECT_EQ(fake_connection_delegate_proxy_2, EXPECT_EQ(fake_connection_delegate_proxy_2,
extracted_client_data[1].second.get()); extracted_client_data[1].connection_delegate_ptr().get());
} }
} // namespace secure_channel } // namespace secure_channel
......
...@@ -20,13 +20,13 @@ FakeConnectionAttempt::~FakeConnectionAttempt() = default; ...@@ -20,13 +20,13 @@ FakeConnectionAttempt::~FakeConnectionAttempt() = default;
void FakeConnectionAttempt::ProcessAddingNewConnectionRequest( void FakeConnectionAttempt::ProcessAddingNewConnectionRequest(
std::unique_ptr<PendingConnectionRequest<std::string>> request) { std::unique_ptr<PendingConnectionRequest<std::string>> request) {
DCHECK(request); DCHECK(request);
DCHECK(!base::ContainsKey(id_to_request_map_, request->request_id())); DCHECK(!base::ContainsKey(id_to_request_map_, request->GetRequestId()));
id_to_request_map_[request->request_id()] = std::move(request); id_to_request_map_[request->GetRequestId()] = std::move(request);
} }
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters>
FakeConnectionAttempt::ExtractClientData() { FakeConnectionAttempt::ExtractClientConnectionParameters() {
return std::move(client_data_for_extraction_); return std::move(client_data_for_extraction_);
} }
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/connection_attempt.h" #include "chromeos/services/secure_channel/connection_attempt.h"
#include "chromeos/services/secure_channel/pending_connection_request.h" #include "chromeos/services/secure_channel/pending_connection_request.h"
...@@ -34,8 +35,7 @@ class FakeConnectionAttempt : public ConnectionAttempt<std::string> { ...@@ -34,8 +35,7 @@ class FakeConnectionAttempt : public ConnectionAttempt<std::string> {
const IdToRequestMap& id_to_request_map() const { return id_to_request_map_; } const IdToRequestMap& id_to_request_map() const { return id_to_request_map_; }
void set_client_data_for_extraction( void set_client_data_for_extraction(
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> client_data_for_extraction) {
client_data_for_extraction) {
client_data_for_extraction_ = std::move(client_data_for_extraction); client_data_for_extraction_ = std::move(client_data_for_extraction);
} }
...@@ -49,13 +49,12 @@ class FakeConnectionAttempt : public ConnectionAttempt<std::string> { ...@@ -49,13 +49,12 @@ class FakeConnectionAttempt : public ConnectionAttempt<std::string> {
// ConnectionAttempt<std::string>: // ConnectionAttempt<std::string>:
void ProcessAddingNewConnectionRequest( void ProcessAddingNewConnectionRequest(
std::unique_ptr<PendingConnectionRequest<std::string>> request) override; std::unique_ptr<PendingConnectionRequest<std::string>> request) override;
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> ExtractClientConnectionParameters()
ExtractClientData() override; override;
IdToRequestMap id_to_request_map_; IdToRequestMap id_to_request_map_;
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> client_data_for_extraction_;
client_data_for_extraction_;
DISALLOW_COPY_AND_ASSIGN(FakeConnectionAttempt); DISALLOW_COPY_AND_ASSIGN(FakeConnectionAttempt);
}; };
......
...@@ -36,10 +36,8 @@ bool FakeMultiplexedChannel::IsDisconnected() { ...@@ -36,10 +36,8 @@ bool FakeMultiplexedChannel::IsDisconnected() {
} }
void FakeMultiplexedChannel::PerformAddClientToChannel( void FakeMultiplexedChannel::PerformAddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) {
mojom::ConnectionDelegatePtr connection_delegate_ptr) { added_clients_.push_back(std::move(client_connection_parameters));
added_clients_.push_back(
std::make_pair(feature, std::move(connection_delegate_ptr)));
} }
FakeMultiplexedChannelDelegate::FakeMultiplexedChannelDelegate() = default; FakeMultiplexedChannelDelegate::FakeMultiplexedChannelDelegate() = default;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <vector> #include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/connection_details.h" #include "chromeos/services/secure_channel/connection_details.h"
#include "chromeos/services/secure_channel/multiplexed_channel.h" #include "chromeos/services/secure_channel/multiplexed_channel.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
...@@ -24,8 +25,7 @@ class FakeMultiplexedChannel : public MultiplexedChannel { ...@@ -24,8 +25,7 @@ class FakeMultiplexedChannel : public MultiplexedChannel {
ConnectionDetails connection_details); ConnectionDetails connection_details);
~FakeMultiplexedChannel() override; ~FakeMultiplexedChannel() override;
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>>& std::vector<ClientConnectionParameters>& added_clients() {
added_clients() {
return added_clients_; return added_clients_;
} }
...@@ -40,14 +40,12 @@ class FakeMultiplexedChannel : public MultiplexedChannel { ...@@ -40,14 +40,12 @@ class FakeMultiplexedChannel : public MultiplexedChannel {
bool IsDisconnecting() override; bool IsDisconnecting() override;
bool IsDisconnected() override; bool IsDisconnected() override;
void PerformAddClientToChannel( void PerformAddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) override;
mojom::ConnectionDelegatePtr connection_delegate_ptr) override;
bool is_disconnecting_ = false; bool is_disconnecting_ = false;
bool is_disconnected_ = false; bool is_disconnected_ = false;
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>> std::vector<ClientConnectionParameters> added_clients_;
added_clients_;
DISALLOW_COPY_AND_ASSIGN(FakeMultiplexedChannel); DISALLOW_COPY_AND_ASSIGN(FakeMultiplexedChannel);
}; };
......
...@@ -4,24 +4,33 @@ ...@@ -4,24 +4,33 @@
#include "chromeos/services/secure_channel/fake_pending_connection_request.h" #include "chromeos/services/secure_channel/fake_pending_connection_request.h"
#include "base/logging.h"
namespace chromeos { namespace chromeos {
namespace secure_channel { namespace secure_channel {
FakePendingConnectionRequest::FakePendingConnectionRequest( FakePendingConnectionRequest::FakePendingConnectionRequest(
PendingConnectionRequestDelegate* delegate) PendingConnectionRequestDelegate* delegate)
: PendingConnectionRequest<std::string>(delegate) {} : PendingConnectionRequest<std::string>(delegate),
id_(base::UnguessableToken::Create()) {}
FakePendingConnectionRequest::~FakePendingConnectionRequest() = default; FakePendingConnectionRequest::~FakePendingConnectionRequest() = default;
const base::UnguessableToken& FakePendingConnectionRequest::GetRequestId()
const {
return id_;
}
void FakePendingConnectionRequest::HandleConnectionFailure( void FakePendingConnectionRequest::HandleConnectionFailure(
std::string failure_detail) { std::string failure_detail) {
handled_failure_details_.push_back(failure_detail); handled_failure_details_.push_back(failure_detail);
} }
std::pair<std::string, mojom::ConnectionDelegatePtr> ClientConnectionParameters
FakePendingConnectionRequest::ExtractClientData() { FakePendingConnectionRequest::ExtractClientConnectionParameters() {
return std::move(client_data_for_extraction_); DCHECK(client_data_for_extraction_);
return std::move(*client_data_for_extraction_);
} }
} // namespace secure_channel } // namespace secure_channel
......
...@@ -10,6 +10,9 @@ ...@@ -10,6 +10,9 @@
#include <vector> #include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "base/unguessable_token.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/pending_connection_request.h" #include "chromeos/services/secure_channel/pending_connection_request.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
...@@ -32,11 +35,13 @@ class FakePendingConnectionRequest ...@@ -32,11 +35,13 @@ class FakePendingConnectionRequest
} }
void set_client_data_for_extraction( void set_client_data_for_extraction(
std::pair<std::string, mojom::ConnectionDelegatePtr> ClientConnectionParameters client_data_for_extraction) {
client_data_for_extraction) {
client_data_for_extraction_ = std::move(client_data_for_extraction); client_data_for_extraction_ = std::move(client_data_for_extraction);
} }
// PendingConnectionRequest<std::string>:
const base::UnguessableToken& GetRequestId() const override;
// Make NotifyRequestFinishedWithoutConnection() public for testing. // Make NotifyRequestFinishedWithoutConnection() public for testing.
using PendingConnectionRequest< using PendingConnectionRequest<
std::string>::NotifyRequestFinishedWithoutConnection; std::string>::NotifyRequestFinishedWithoutConnection;
...@@ -44,14 +49,13 @@ class FakePendingConnectionRequest ...@@ -44,14 +49,13 @@ class FakePendingConnectionRequest
private: private:
// PendingConnectionRequest<std::string>: // PendingConnectionRequest<std::string>:
void HandleConnectionFailure(std::string failure_detail) override; void HandleConnectionFailure(std::string failure_detail) override;
ClientConnectionParameters ExtractClientConnectionParameters() override;
std::pair<std::string, mojom::ConnectionDelegatePtr> ExtractClientData() const base::UnguessableToken id_;
override;
std::vector<std::string> handled_failure_details_; std::vector<std::string> handled_failure_details_;
std::pair<std::string, mojom::ConnectionDelegatePtr> base::Optional<ClientConnectionParameters> client_data_for_extraction_;
client_data_for_extraction_;
DISALLOW_COPY_AND_ASSIGN(FakePendingConnectionRequest); DISALLOW_COPY_AND_ASSIGN(FakePendingConnectionRequest);
}; };
......
...@@ -12,11 +12,16 @@ FakeSingleClientMessageProxy::FakeSingleClientMessageProxy( ...@@ -12,11 +12,16 @@ FakeSingleClientMessageProxy::FakeSingleClientMessageProxy(
Delegate* delegate, Delegate* delegate,
base::OnceCallback<void(const base::UnguessableToken&)> destructor_callback) base::OnceCallback<void(const base::UnguessableToken&)> destructor_callback)
: SingleClientMessageProxy(delegate), : SingleClientMessageProxy(delegate),
proxy_id_(base::UnguessableToken::Create()),
destructor_callback_(std::move(destructor_callback)) {} destructor_callback_(std::move(destructor_callback)) {}
FakeSingleClientMessageProxy::~FakeSingleClientMessageProxy() { FakeSingleClientMessageProxy::~FakeSingleClientMessageProxy() {
if (destructor_callback_) if (destructor_callback_)
std::move(destructor_callback_).Run(proxy_id()); std::move(destructor_callback_).Run(GetProxyId());
}
const base::UnguessableToken& FakeSingleClientMessageProxy::GetProxyId() {
return proxy_id_;
} }
void FakeSingleClientMessageProxy::HandleReceivedMessage( void FakeSingleClientMessageProxy::HandleReceivedMessage(
......
...@@ -35,6 +35,9 @@ class FakeSingleClientMessageProxy : public SingleClientMessageProxy { ...@@ -35,6 +35,9 @@ class FakeSingleClientMessageProxy : public SingleClientMessageProxy {
return processed_messages_; return processed_messages_;
} }
// SingleClientMessageProxy:
const base::UnguessableToken& GetProxyId() override;
// Public for testing. // Public for testing.
using SingleClientMessageProxy::NotifySendMessageRequested; using SingleClientMessageProxy::NotifySendMessageRequested;
using SingleClientMessageProxy::NotifyClientDisconnected; using SingleClientMessageProxy::NotifyClientDisconnected;
...@@ -46,6 +49,7 @@ class FakeSingleClientMessageProxy : public SingleClientMessageProxy { ...@@ -46,6 +49,7 @@ class FakeSingleClientMessageProxy : public SingleClientMessageProxy {
const std::string& payload) override; const std::string& payload) override;
void HandleRemoteDeviceDisconnection() override; void HandleRemoteDeviceDisconnection() override;
const base::UnguessableToken proxy_id_;
base::OnceCallback<void(const base::UnguessableToken&)> destructor_callback_; base::OnceCallback<void(const base::UnguessableToken&)> destructor_callback_;
std::vector<std::pair<std::string, std::string>> processed_messages_; std::vector<std::pair<std::string, std::string>> processed_messages_;
......
...@@ -20,12 +20,11 @@ MultiplexedChannel::MultiplexedChannel(Delegate* delegate, ...@@ -20,12 +20,11 @@ MultiplexedChannel::MultiplexedChannel(Delegate* delegate,
MultiplexedChannel::~MultiplexedChannel() = default; MultiplexedChannel::~MultiplexedChannel() = default;
bool MultiplexedChannel::AddClientToChannel( bool MultiplexedChannel::AddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) {
mojom::ConnectionDelegatePtr connection_delegate_ptr) {
if (IsDisconnecting() || IsDisconnected()) if (IsDisconnecting() || IsDisconnected())
return false; return false;
PerformAddClientToChannel(feature, std::move(connection_delegate_ptr)); PerformAddClientToChannel(std::move(client_connection_parameters));
return true; return true;
} }
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define CHROMEOS_SERVICES_SECURE_CHANNEL_MULTIPLEXED_CHANNEL_H_ #define CHROMEOS_SERVICES_SECURE_CHANNEL_MULTIPLEXED_CHANNEL_H_
#include "base/macros.h" #include "base/macros.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/connection_details.h" #include "chromeos/services/secure_channel/connection_details.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
...@@ -35,8 +36,8 @@ class MultiplexedChannel { ...@@ -35,8 +36,8 @@ class MultiplexedChannel {
// Shares this channel with an additional client. Returns whether this action // Shares this channel with an additional client. Returns whether this action
// was successful; all calls are expected to succeed unless the channel is // was successful; all calls are expected to succeed unless the channel is
// disconnected or disconnecting. // disconnected or disconnecting.
bool AddClientToChannel(const std::string& feature, bool AddClientToChannel(
mojom::ConnectionDelegatePtr connection_delegate_ptr); ClientConnectionParameters client_connection_parameters);
const ConnectionDetails& connection_details() { return connection_details_; } const ConnectionDetails& connection_details() { return connection_details_; }
...@@ -44,8 +45,7 @@ class MultiplexedChannel { ...@@ -44,8 +45,7 @@ class MultiplexedChannel {
MultiplexedChannel(Delegate* delegate, ConnectionDetails connection_details); MultiplexedChannel(Delegate* delegate, ConnectionDetails connection_details);
virtual void PerformAddClientToChannel( virtual void PerformAddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) = 0;
mojom::ConnectionDelegatePtr connection_delegate_ptr) = 0;
void NotifyDisconnected(); void NotifyDisconnected();
......
...@@ -40,7 +40,7 @@ MultiplexedChannelImpl::Factory::BuildInstance( ...@@ -40,7 +40,7 @@ MultiplexedChannelImpl::Factory::BuildInstance(
std::unique_ptr<AuthenticatedChannel> authenticated_channel, std::unique_ptr<AuthenticatedChannel> authenticated_channel,
MultiplexedChannel::Delegate* delegate, MultiplexedChannel::Delegate* delegate,
ConnectionDetails connection_details, ConnectionDetails connection_details,
InitialClientList* initial_clients) { std::vector<ClientConnectionParameters>* initial_clients) {
DCHECK(authenticated_channel); DCHECK(authenticated_channel);
DCHECK(!authenticated_channel->is_disconnected()); DCHECK(!authenticated_channel->is_disconnected());
DCHECK(delegate); DCHECK(delegate);
...@@ -49,9 +49,9 @@ MultiplexedChannelImpl::Factory::BuildInstance( ...@@ -49,9 +49,9 @@ MultiplexedChannelImpl::Factory::BuildInstance(
auto channel = base::WrapUnique(new MultiplexedChannelImpl( auto channel = base::WrapUnique(new MultiplexedChannelImpl(
std::move(authenticated_channel), delegate, connection_details)); std::move(authenticated_channel), delegate, connection_details));
for (auto& client : *initial_clients) { for (auto& client_connection_parameters : *initial_clients) {
bool success = bool success =
channel->AddClientToChannel(client.first, std::move(client.second)); channel->AddClientToChannel(std::move(client_connection_parameters));
if (!success) { if (!success) {
PA_LOG(ERROR) << "MultiplexedChannelImpl::Factory::BuildInstance(): " PA_LOG(ERROR) << "MultiplexedChannelImpl::Factory::BuildInstance(): "
<< "Failed to add initial client."; << "Failed to add initial client.";
...@@ -84,15 +84,14 @@ bool MultiplexedChannelImpl::IsDisconnected() { ...@@ -84,15 +84,14 @@ bool MultiplexedChannelImpl::IsDisconnected() {
} }
void MultiplexedChannelImpl::PerformAddClientToChannel( void MultiplexedChannelImpl::PerformAddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) {
mojom::ConnectionDelegatePtr connection_delegate_ptr) { DCHECK(!client_connection_parameters.feature().empty());
DCHECK(!feature.empty()); DCHECK(client_connection_parameters.connection_delegate_ptr());
DCHECK(connection_delegate_ptr);
auto proxy = SingleClientMessageProxyImpl::Factory::Get()->BuildInstance( auto proxy = SingleClientMessageProxyImpl::Factory::Get()->BuildInstance(
this /* delegate */, feature, std::move(connection_delegate_ptr)); this /* delegate */, std::move(client_connection_parameters));
DCHECK(!base::ContainsKey(id_to_proxy_map_, proxy->proxy_id())); DCHECK(!base::ContainsKey(id_to_proxy_map_, proxy->GetProxyId()));
id_to_proxy_map_[proxy->proxy_id()] = std::move(proxy); id_to_proxy_map_[proxy->GetProxyId()] = std::move(proxy);
} }
void MultiplexedChannelImpl::OnDisconnected() { void MultiplexedChannelImpl::OnDisconnected() {
......
...@@ -39,13 +39,11 @@ class MultiplexedChannelImpl : public MultiplexedChannel, ...@@ -39,13 +39,11 @@ class MultiplexedChannelImpl : public MultiplexedChannel,
virtual ~Factory(); virtual ~Factory();
using InitialClientList =
std::vector<std::pair<std::string, mojom::ConnectionDelegatePtr>>;
virtual std::unique_ptr<MultiplexedChannel> BuildInstance( virtual std::unique_ptr<MultiplexedChannel> BuildInstance(
std::unique_ptr<AuthenticatedChannel> authenticated_channel, std::unique_ptr<AuthenticatedChannel> authenticated_channel,
MultiplexedChannel::Delegate* delegate, MultiplexedChannel::Delegate* delegate,
ConnectionDetails connection_details, ConnectionDetails connection_details,
InitialClientList* initial_clients); std::vector<ClientConnectionParameters>* initial_clients);
private: private:
static Factory* test_factory_; static Factory* test_factory_;
...@@ -63,8 +61,7 @@ class MultiplexedChannelImpl : public MultiplexedChannel, ...@@ -63,8 +61,7 @@ class MultiplexedChannelImpl : public MultiplexedChannel,
bool IsDisconnecting() override; bool IsDisconnecting() override;
bool IsDisconnected() override; bool IsDisconnected() override;
void PerformAddClientToChannel( void PerformAddClientToChannel(
const std::string& feature, ClientConnectionParameters client_connection_parameters) override;
mojom::ConnectionDelegatePtr connection_delegate_ptr) override;
// AuthenticatedChannel::Observer: // AuthenticatedChannel::Observer:
void OnDisconnected() override; void OnDisconnected() override;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/connection_details.h" #include "chromeos/services/secure_channel/connection_details.h"
#include "chromeos/services/secure_channel/connection_medium.h" #include "chromeos/services/secure_channel/connection_medium.h"
#include "chromeos/services/secure_channel/fake_authenticated_channel.h" #include "chromeos/services/secure_channel/fake_authenticated_channel.h"
...@@ -52,10 +53,9 @@ class FakeSingleClientMessageProxyImplFactory ...@@ -52,10 +53,9 @@ class FakeSingleClientMessageProxyImplFactory
private: private:
std::unique_ptr<SingleClientMessageProxy> BuildInstance( std::unique_ptr<SingleClientMessageProxy> BuildInstance(
SingleClientMessageProxy::Delegate* delegate, SingleClientMessageProxy::Delegate* delegate,
const std::string& feature, ClientConnectionParameters client_connection_parameters) override {
mojom::ConnectionDelegatePtr connection_delegate_ptr) override { EXPECT_EQ(kTestFeature, client_connection_parameters.feature());
EXPECT_EQ(kTestFeature, feature); EXPECT_TRUE(client_connection_parameters.connection_delegate_ptr());
EXPECT_TRUE(connection_delegate_ptr);
if (!expected_delegate_) if (!expected_delegate_)
expected_delegate_ = delegate; expected_delegate_ = delegate;
...@@ -70,7 +70,7 @@ class FakeSingleClientMessageProxyImplFactory ...@@ -70,7 +70,7 @@ class FakeSingleClientMessageProxyImplFactory
base::Unretained(this))); base::Unretained(this)));
FakeSingleClientMessageProxy* proxy_raw = FakeSingleClientMessageProxy* proxy_raw =
static_cast<FakeSingleClientMessageProxy*>(proxy.get()); static_cast<FakeSingleClientMessageProxy*>(proxy.get());
id_to_active_proxy_map_[proxy->proxy_id()] = proxy_raw; id_to_active_proxy_map_[proxy->GetProxyId()] = proxy_raw;
return proxy; return proxy;
} }
...@@ -108,7 +108,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test { ...@@ -108,7 +108,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
// The default list contains one client. // The default list contains one client.
initial_fake_connection_delegate_ = initial_fake_connection_delegate_ =
std::make_unique<FakeConnectionDelegate>(); std::make_unique<FakeConnectionDelegate>();
initial_client_list_.push_back(std::make_pair( initial_client_list_.push_back(ClientConnectionParameters(
kTestFeature, kTestFeature,
initial_fake_connection_delegate_->GenerateInterfacePtr())); initial_fake_connection_delegate_->GenerateInterfacePtr()));
} }
...@@ -196,7 +196,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test { ...@@ -196,7 +196,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
bool is_last_client = id_to_active_proxy_map().size() == 1u; bool is_last_client = id_to_active_proxy_map().size() == 1u;
EXPECT_EQ(is_last_client, expected_to_be_last_client); EXPECT_EQ(is_last_client, expected_to_be_last_client);
base::UnguessableToken proxy_id = sending_proxy->proxy_id(); base::UnguessableToken proxy_id = sending_proxy->GetProxyId();
// All relevant parties should still indicate that the connection is valid. // All relevant parties should still indicate that the connection is valid.
EXPECT_TRUE(base::ContainsKey(id_to_active_proxy_map(), proxy_id)); EXPECT_TRUE(base::ContainsKey(id_to_active_proxy_map(), proxy_id));
...@@ -255,12 +255,13 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test { ...@@ -255,12 +255,13 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
void AddClientToChannel( void AddClientToChannel(
const std::string& feature, const std::string& feature,
mojom::ConnectionDelegatePtr connection_delegate_ptr) { mojom::ConnectionDelegatePtr connection_delegate_ptr) {
bool success = multiplexed_channel_->AddClientToChannel( bool success =
feature, std::move(connection_delegate_ptr)); multiplexed_channel_->AddClientToChannel(ClientConnectionParameters(
feature, std::move(connection_delegate_ptr)));
EXPECT_TRUE(success); EXPECT_TRUE(success);
} }
MultiplexedChannelImpl::Factory::InitialClientList& initial_client_list() { std::vector<ClientConnectionParameters>& initial_client_list() {
return initial_client_list_; return initial_client_list_;
} }
...@@ -292,7 +293,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test { ...@@ -292,7 +293,7 @@ class SecureChannelMultiplexedChannelImplTest : public testing::Test {
std::unique_ptr<FakeSingleClientMessageProxyImplFactory> fake_proxy_factory_; std::unique_ptr<FakeSingleClientMessageProxyImplFactory> fake_proxy_factory_;
MultiplexedChannelImpl::Factory::InitialClientList initial_client_list_; std::vector<ClientConnectionParameters> initial_client_list_;
std::unique_ptr<FakeConnectionDelegate> initial_fake_connection_delegate_; std::unique_ptr<FakeConnectionDelegate> initial_fake_connection_delegate_;
FakeAuthenticatedChannel* fake_authenticated_channel_ = nullptr; FakeAuthenticatedChannel* fake_authenticated_channel_ = nullptr;
...@@ -393,7 +394,7 @@ TEST_F(SecureChannelMultiplexedChannelImplTest, ...@@ -393,7 +394,7 @@ TEST_F(SecureChannelMultiplexedChannelImplTest,
// Add a second initial client. // Add a second initial client.
std::unique_ptr<FakeConnectionDelegate> fake_connection_delegate_2 = std::unique_ptr<FakeConnectionDelegate> fake_connection_delegate_2 =
std::make_unique<FakeConnectionDelegate>(); std::make_unique<FakeConnectionDelegate>();
initial_client_list().push_back(std::make_pair( initial_client_list().push_back(ClientConnectionParameters(
kTestFeature, fake_connection_delegate_2->GenerateInterfacePtr())); kTestFeature, fake_connection_delegate_2->GenerateInterfacePtr()));
CreateChannel(); CreateChannel();
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
#include <utility> #include <utility>
#include "base/macros.h" #include "base/macros.h"
#include "base/unguessable_token.h" #include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/pending_connection_request_delegate.h" #include "chromeos/services/secure_channel/pending_connection_request_delegate.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
...@@ -26,12 +26,12 @@ namespace secure_channel { ...@@ -26,12 +26,12 @@ namespace secure_channel {
template <typename FailureDetailType> template <typename FailureDetailType>
class PendingConnectionRequest { class PendingConnectionRequest {
public: public:
// Extracts |request|'s feature and ConnectionDelegate. This function deletes // Extracts |request|'s ClientConnectionParameters. This function deletes
// |request| as part of this process to ensure that it is no longer used after // |request| as part of this process to ensure that it is no longer used after
// extraction is complete. // extraction is complete.
static std::pair<std::string, mojom::ConnectionDelegatePtr> ExtractClientData( static ClientConnectionParameters ExtractClientConnectionParameters(
std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request) { std::unique_ptr<PendingConnectionRequest<FailureDetailType>> request) {
return request->ExtractClientData(); return request->ExtractClientConnectionParameters();
} }
virtual ~PendingConnectionRequest() = default; virtual ~PendingConnectionRequest() = default;
...@@ -40,26 +40,24 @@ class PendingConnectionRequest { ...@@ -40,26 +40,24 @@ class PendingConnectionRequest {
// trying to connect after some number of failures. // trying to connect after some number of failures.
virtual void HandleConnectionFailure(FailureDetailType failure_detail) = 0; virtual void HandleConnectionFailure(FailureDetailType failure_detail) = 0;
const base::UnguessableToken& request_id() const { return request_id_; } virtual const base::UnguessableToken& GetRequestId() const = 0;
protected: protected:
PendingConnectionRequest(PendingConnectionRequestDelegate* delegate) PendingConnectionRequest(PendingConnectionRequestDelegate* delegate)
: delegate_(delegate), request_id_(base::UnguessableToken::Create()) { : delegate_(delegate) {
DCHECK(delegate_); DCHECK(delegate_);
} }
// Extracts the feature and ConnectionDelegate from this request. // Extracts the feature and ConnectionDelegate from this request.
virtual std::pair<std::string, mojom::ConnectionDelegatePtr> virtual ClientConnectionParameters ExtractClientConnectionParameters() = 0;
ExtractClientData() = 0;
void NotifyRequestFinishedWithoutConnection( void NotifyRequestFinishedWithoutConnection(
PendingConnectionRequestDelegate::FailedConnectionReason reason) { PendingConnectionRequestDelegate::FailedConnectionReason reason) {
delegate_->OnRequestFinishedWithoutConnection(request_id_, reason); delegate_->OnRequestFinishedWithoutConnection(GetRequestId(), reason);
} }
private: private:
PendingConnectionRequestDelegate* delegate_; PendingConnectionRequestDelegate* delegate_;
const base::UnguessableToken request_id_;
DISALLOW_COPY_AND_ASSIGN(PendingConnectionRequest); DISALLOW_COPY_AND_ASSIGN(PendingConnectionRequest);
}; };
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chromeos/components/proximity_auth/logging/logging.h" #include "chromeos/components/proximity_auth/logging/logging.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/pending_connection_request.h" #include "chromeos/services/secure_channel/pending_connection_request.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
...@@ -33,27 +34,28 @@ class PendingConnectionRequestBase ...@@ -33,27 +34,28 @@ class PendingConnectionRequestBase
public: public:
~PendingConnectionRequestBase() override = default; ~PendingConnectionRequestBase() override = default;
// PendingConnectionRequest<FailureDetailType>:
const base::UnguessableToken& GetRequestId() const override {
return client_connection_parameters_.id();
}
protected: protected:
PendingConnectionRequestBase( PendingConnectionRequestBase(
const std::string& feature, ClientConnectionParameters client_connection_parameters,
const std::string& readable_request_type_for_logging, const std::string& readable_request_type_for_logging,
PendingConnectionRequestDelegate* delegate, PendingConnectionRequestDelegate* delegate)
mojom::ConnectionDelegatePtr connection_delegate_ptr)
: PendingConnectionRequest<FailureDetailType>(delegate), : PendingConnectionRequest<FailureDetailType>(delegate),
feature_(feature), client_connection_parameters_(std::move(client_connection_parameters)),
readable_request_type_for_logging_(readable_request_type_for_logging), readable_request_type_for_logging_(readable_request_type_for_logging),
connection_delegate_ptr_(std::move(connection_delegate_ptr)),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
DCHECK(!feature_.empty());
DCHECK(connection_delegate_ptr_);
// If the client disconnects its delegate, the client is signaling that the // If the client disconnects its delegate, the client is signaling that the
// connection request has been canceled. // connection request has been canceled.
connection_delegate_ptr_.set_connection_error_handler(base::BindOnce( client_connection_parameters_.connection_delegate_ptr()
&PendingConnectionRequestBase::OnFinishedWithoutConnection, .set_connection_error_handler(base::BindOnce(
weak_ptr_factory_.GetWeakPtr(), &PendingConnectionRequestBase::OnFinishedWithoutConnection,
PendingConnectionRequestDelegate::FailedConnectionReason:: weak_ptr_factory_.GetWeakPtr(),
kRequestCanceledByClient)); PendingConnectionRequestDelegate::FailedConnectionReason::
kRequestCanceledByClient));
} }
// Derived classes should invoke this function if they would like to give up // Derived classes should invoke this function if they would like to give up
...@@ -67,7 +69,8 @@ class PendingConnectionRequestBase ...@@ -67,7 +69,8 @@ class PendingConnectionRequestBase
return; return;
} }
connection_delegate_ptr_->OnConnectionAttemptFailure(failure_reason); client_connection_parameters_.connection_delegate_ptr()
->OnConnectionAttemptFailure(failure_reason);
OnFinishedWithoutConnection(PendingConnectionRequestDelegate:: OnFinishedWithoutConnection(PendingConnectionRequestDelegate::
FailedConnectionReason::kRequestFailed); FailedConnectionReason::kRequestFailed);
...@@ -79,9 +82,9 @@ class PendingConnectionRequestBase ...@@ -79,9 +82,9 @@ class PendingConnectionRequestBase
using PendingConnectionRequest< using PendingConnectionRequest<
FailureDetailType>::NotifyRequestFinishedWithoutConnection; FailureDetailType>::NotifyRequestFinishedWithoutConnection;
std::pair<std::string, mojom::ConnectionDelegatePtr> ExtractClientData() // PendingConnectionRequest<FailureDetailType>:
override { ClientConnectionParameters ExtractClientConnectionParameters() override {
return std::make_pair(feature_, std::move(connection_delegate_ptr_)); return std::move(client_connection_parameters_);
} }
void OnFinishedWithoutConnection( void OnFinishedWithoutConnection(
...@@ -90,15 +93,14 @@ class PendingConnectionRequestBase ...@@ -90,15 +93,14 @@ class PendingConnectionRequestBase
has_finished_without_connection_ = true; has_finished_without_connection_ = true;
PA_LOG(INFO) << "Request finished without connection; notifying delegate. " PA_LOG(INFO) << "Request finished without connection; notifying delegate. "
<< "Feature: \"" << feature_ << "\"" << "Request type: \"" << readable_request_type_for_logging_
<< ", Reason: " << reason << ", Request type: \"" << "\", Reason: " << reason
<< readable_request_type_for_logging_ << "\""; << ", Client parameters: " << client_connection_parameters_;
NotifyRequestFinishedWithoutConnection(reason); NotifyRequestFinishedWithoutConnection(reason);
} }
const std::string feature_; ClientConnectionParameters client_connection_parameters_;
const std::string readable_request_type_for_logging_; const std::string readable_request_type_for_logging_;
mojom::ConnectionDelegatePtr connection_delegate_ptr_;
bool has_finished_without_connection_ = false; bool has_finished_without_connection_ = false;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/fake_connection_delegate.h" #include "chromeos/services/secure_channel/fake_connection_delegate.h"
#include "chromeos/services/secure_channel/fake_pending_connection_request_delegate.h" #include "chromeos/services/secure_channel/fake_pending_connection_request_delegate.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -34,14 +35,12 @@ class TestPendingConnectionRequest ...@@ -34,14 +35,12 @@ class TestPendingConnectionRequest
: public PendingConnectionRequestBase<TestFailureDetail> { : public PendingConnectionRequestBase<TestFailureDetail> {
public: public:
TestPendingConnectionRequest( TestPendingConnectionRequest(
const std::string& feature, ClientConnectionParameters client_connection_parameters,
mojom::ConnectionDelegatePtr connection_delegate_ptr,
PendingConnectionRequestDelegate* delegate) PendingConnectionRequestDelegate* delegate)
: PendingConnectionRequestBase<TestFailureDetail>( : PendingConnectionRequestBase<TestFailureDetail>(
feature, std::move(client_connection_parameters),
kTestReadableRequestTypeForLogging, kTestReadableRequestTypeForLogging,
delegate, delegate) {}
std::move(connection_delegate_ptr)) {}
~TestPendingConnectionRequest() override = default; ~TestPendingConnectionRequest() override = default;
// PendingConnectionRequestBase<TestFailureDetail>: // PendingConnectionRequestBase<TestFailureDetail>:
...@@ -72,9 +71,12 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test { ...@@ -72,9 +71,12 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test {
fake_connection_delegate_->GenerateInterfacePtr(); fake_connection_delegate_->GenerateInterfacePtr();
fake_connection_delegate_proxy_ = fake_connection_delegate_proxy_ =
fake_connection_delegate_interface_ptr.get(); fake_connection_delegate_interface_ptr.get();
test_pending_connection_request_ = test_pending_connection_request_ =
std::make_unique<TestPendingConnectionRequest>( std::make_unique<TestPendingConnectionRequest>(
kTestFeature, std::move(fake_connection_delegate_interface_ptr), ClientConnectionParameters(
kTestFeature,
std::move(fake_connection_delegate_interface_ptr)),
fake_pending_connection_request_delegate_.get()); fake_pending_connection_request_delegate_.get());
} }
...@@ -98,7 +100,7 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test { ...@@ -98,7 +100,7 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test {
GetFailedConnectionReason() { GetFailedConnectionReason() {
return fake_pending_connection_request_delegate_ return fake_pending_connection_request_delegate_
->GetFailedConnectionReasonForId( ->GetFailedConnectionReasonForId(
test_pending_connection_request_->request_id()); test_pending_connection_request_->GetRequestId());
} }
void DisconnectConnectionDelegatePtr() { void DisconnectConnectionDelegatePtr() {
...@@ -114,9 +116,10 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test { ...@@ -114,9 +116,10 @@ class SecureChannelPendingConnectionRequestBaseTest : public testing::Test {
return fake_connection_delegate_->connection_attempt_failure_reason(); return fake_connection_delegate_->connection_attempt_failure_reason();
} }
std::pair<std::string, mojom::ConnectionDelegatePtr> ExtractClientData() { ClientConnectionParameters ExtractClientConnectionParameters() {
return PendingConnectionRequest<TestFailureDetail>::ExtractClientData( return PendingConnectionRequest<TestFailureDetail>::
std::move(test_pending_connection_request_)); ExtractClientConnectionParameters(
std::move(test_pending_connection_request_));
} }
mojom::ConnectionDelegate::Proxy_* fake_connection_delegate_proxy() { mojom::ConnectionDelegate::Proxy_* fake_connection_delegate_proxy() {
...@@ -169,11 +172,12 @@ TEST_F(SecureChannelPendingConnectionRequestBaseTest, ...@@ -169,11 +172,12 @@ TEST_F(SecureChannelPendingConnectionRequestBaseTest,
*GetFailedConnectionReason()); *GetFailedConnectionReason());
} }
TEST_F(SecureChannelPendingConnectionRequestBaseTest, ExtractClientData) { TEST_F(SecureChannelPendingConnectionRequestBaseTest,
auto extracted_client_data = ExtractClientData(); ExtractClientConnectionParameters) {
EXPECT_EQ(kTestFeature, extracted_client_data.first); auto extracted_client_data = ExtractClientConnectionParameters();
EXPECT_EQ(kTestFeature, extracted_client_data.feature());
EXPECT_EQ(fake_connection_delegate_proxy(), EXPECT_EQ(fake_connection_delegate_proxy(),
extracted_client_data.second.get()); extracted_client_data.connection_delegate_ptr().get());
} }
} // namespace secure_channel } // namespace secure_channel
......
...@@ -9,7 +9,7 @@ namespace chromeos { ...@@ -9,7 +9,7 @@ namespace chromeos {
namespace secure_channel { namespace secure_channel {
SingleClientMessageProxy::SingleClientMessageProxy(Delegate* delegate) SingleClientMessageProxy::SingleClientMessageProxy(Delegate* delegate)
: delegate_(delegate), proxy_id_(base::UnguessableToken::Create()) {} : delegate_(delegate) {}
SingleClientMessageProxy::~SingleClientMessageProxy() = default; SingleClientMessageProxy::~SingleClientMessageProxy() = default;
...@@ -22,7 +22,7 @@ void SingleClientMessageProxy::NotifySendMessageRequested( ...@@ -22,7 +22,7 @@ void SingleClientMessageProxy::NotifySendMessageRequested(
} }
void SingleClientMessageProxy::NotifyClientDisconnected() { void SingleClientMessageProxy::NotifyClientDisconnected() {
delegate_->OnClientDisconnected(proxy_id_); delegate_->OnClientDisconnected(GetProxyId());
} }
const mojom::ConnectionMetadata& const mojom::ConnectionMetadata&
......
...@@ -44,7 +44,7 @@ class SingleClientMessageProxy { ...@@ -44,7 +44,7 @@ class SingleClientMessageProxy {
// because of instability on the communication channel). // because of instability on the communication channel).
virtual void HandleRemoteDeviceDisconnection() = 0; virtual void HandleRemoteDeviceDisconnection() = 0;
const base::UnguessableToken& proxy_id() { return proxy_id_; } virtual const base::UnguessableToken& GetProxyId() = 0;
protected: protected:
SingleClientMessageProxy(Delegate* delegate); SingleClientMessageProxy(Delegate* delegate);
...@@ -57,7 +57,6 @@ class SingleClientMessageProxy { ...@@ -57,7 +57,6 @@ class SingleClientMessageProxy {
private: private:
Delegate* delegate_; Delegate* delegate_;
const base::UnguessableToken proxy_id_;
DISALLOW_COPY_AND_ASSIGN(SingleClientMessageProxy); DISALLOW_COPY_AND_ASSIGN(SingleClientMessageProxy);
}; };
......
...@@ -35,33 +35,34 @@ SingleClientMessageProxyImpl::Factory::~Factory() = default; ...@@ -35,33 +35,34 @@ SingleClientMessageProxyImpl::Factory::~Factory() = default;
std::unique_ptr<SingleClientMessageProxy> std::unique_ptr<SingleClientMessageProxy>
SingleClientMessageProxyImpl::Factory::BuildInstance( SingleClientMessageProxyImpl::Factory::BuildInstance(
SingleClientMessageProxy::Delegate* delegate, SingleClientMessageProxy::Delegate* delegate,
const std::string& feature, ClientConnectionParameters client_connection_parameters) {
mojom::ConnectionDelegatePtr connection_delegate_ptr) {
return base::WrapUnique(new SingleClientMessageProxyImpl( return base::WrapUnique(new SingleClientMessageProxyImpl(
delegate, feature, std::move(connection_delegate_ptr))); delegate, std::move(client_connection_parameters)));
} }
SingleClientMessageProxyImpl::SingleClientMessageProxyImpl( SingleClientMessageProxyImpl::SingleClientMessageProxyImpl(
SingleClientMessageProxy::Delegate* delegate, SingleClientMessageProxy::Delegate* delegate,
const std::string& feature, ClientConnectionParameters client_connection_parameters)
mojom::ConnectionDelegatePtr connection_delegate_ptr)
: SingleClientMessageProxy(delegate), : SingleClientMessageProxy(delegate),
feature_(feature), client_connection_parameters_(std::move(client_connection_parameters)),
connection_delegate_ptr_(std::move(connection_delegate_ptr)),
channel_(std::make_unique<ChannelImpl>(this /* delegate */)) { channel_(std::make_unique<ChannelImpl>(this /* delegate */)) {
DCHECK(connection_delegate_ptr_); DCHECK(client_connection_parameters_.connection_delegate_ptr());
connection_delegate_ptr_->OnConnection( client_connection_parameters_.connection_delegate_ptr()->OnConnection(
channel_->GenerateInterfacePtr(), channel_->GenerateInterfacePtr(),
mojo::MakeRequest(&message_receiver_ptr_)); mojo::MakeRequest(&message_receiver_ptr_));
} }
SingleClientMessageProxyImpl::~SingleClientMessageProxyImpl() = default; SingleClientMessageProxyImpl::~SingleClientMessageProxyImpl() = default;
const base::UnguessableToken& SingleClientMessageProxyImpl::GetProxyId() {
return client_connection_parameters_.id();
}
void SingleClientMessageProxyImpl::HandleReceivedMessage( void SingleClientMessageProxyImpl::HandleReceivedMessage(
const std::string& feature, const std::string& feature,
const std::string& payload) { const std::string& payload) {
// Ignore messages intended for other clients. // Ignore messages intended for other clients.
if (feature != feature_) if (feature != client_connection_parameters_.feature())
return; return;
message_receiver_ptr_->OnMessageReceived(payload); message_receiver_ptr_->OnMessageReceived(payload);
...@@ -74,7 +75,8 @@ void SingleClientMessageProxyImpl::HandleRemoteDeviceDisconnection() { ...@@ -74,7 +75,8 @@ void SingleClientMessageProxyImpl::HandleRemoteDeviceDisconnection() {
void SingleClientMessageProxyImpl::OnSendMessageRequested( void SingleClientMessageProxyImpl::OnSendMessageRequested(
const std::string& message, const std::string& message,
base::OnceClosure on_sent_callback) { base::OnceClosure on_sent_callback) {
NotifySendMessageRequested(feature_, message, std::move(on_sent_callback)); NotifySendMessageRequested(client_connection_parameters_.feature(), message,
std::move(on_sent_callback));
} }
const mojom::ConnectionMetadata& const mojom::ConnectionMetadata&
...@@ -87,8 +89,8 @@ void SingleClientMessageProxyImpl::OnClientDisconnected() { ...@@ -87,8 +89,8 @@ void SingleClientMessageProxyImpl::OnClientDisconnected() {
} }
void SingleClientMessageProxyImpl::FlushForTesting() { void SingleClientMessageProxyImpl::FlushForTesting() {
DCHECK(connection_delegate_ptr_); DCHECK(client_connection_parameters_.connection_delegate_ptr());
connection_delegate_ptr_.FlushForTesting(); client_connection_parameters_.connection_delegate_ptr().FlushForTesting();
DCHECK(message_receiver_ptr_); DCHECK(message_receiver_ptr_);
message_receiver_ptr_.FlushForTesting(); message_receiver_ptr_.FlushForTesting();
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "chromeos/services/secure_channel/channel_impl.h" #include "chromeos/services/secure_channel/channel_impl.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h" #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
#include "chromeos/services/secure_channel/single_client_message_proxy.h" #include "chromeos/services/secure_channel/single_client_message_proxy.h"
#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding.h"
...@@ -29,8 +30,7 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy, ...@@ -29,8 +30,7 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy,
virtual ~Factory(); virtual ~Factory();
virtual std::unique_ptr<SingleClientMessageProxy> BuildInstance( virtual std::unique_ptr<SingleClientMessageProxy> BuildInstance(
SingleClientMessageProxy::Delegate* delegate, SingleClientMessageProxy::Delegate* delegate,
const std::string& feature, ClientConnectionParameters client_connection_parameters);
mojom::ConnectionDelegatePtr connection_delegate_ptr);
private: private:
static Factory* test_factory_; static Factory* test_factory_;
...@@ -38,13 +38,15 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy, ...@@ -38,13 +38,15 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy,
~SingleClientMessageProxyImpl() override; ~SingleClientMessageProxyImpl() override;
// SingleClientMessageProxy:
const base::UnguessableToken& GetProxyId() override;
private: private:
friend class SecureChannelSingleClientMessageProxyImplTest; friend class SecureChannelSingleClientMessageProxyImplTest;
SingleClientMessageProxyImpl( SingleClientMessageProxyImpl(
SingleClientMessageProxy::Delegate* delegate, SingleClientMessageProxy::Delegate* delegate,
const std::string& feature, ClientConnectionParameters client_connection_parameters);
mojom::ConnectionDelegatePtr connection_delegate_ptr);
// SingleClientMessageProxy: // SingleClientMessageProxy:
void HandleReceivedMessage(const std::string& feature, void HandleReceivedMessage(const std::string& feature,
...@@ -59,8 +61,7 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy, ...@@ -59,8 +61,7 @@ class SingleClientMessageProxyImpl : public SingleClientMessageProxy,
void FlushForTesting(); void FlushForTesting();
const std::string feature_; ClientConnectionParameters client_connection_parameters_;
mojom::ConnectionDelegatePtr connection_delegate_ptr_;
std::unique_ptr<ChannelImpl> channel_; std::unique_ptr<ChannelImpl> channel_;
mojom::MessageReceiverPtr message_receiver_ptr_; mojom::MessageReceiverPtr message_receiver_ptr_;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "chromeos/services/secure_channel/client_connection_parameters.h"
#include "chromeos/services/secure_channel/fake_connection_delegate.h" #include "chromeos/services/secure_channel/fake_connection_delegate.h"
#include "chromeos/services/secure_channel/fake_message_receiver.h" #include "chromeos/services/secure_channel/fake_message_receiver.h"
#include "chromeos/services/secure_channel/fake_single_client_message_proxy.h" #include "chromeos/services/secure_channel/fake_single_client_message_proxy.h"
...@@ -43,8 +44,9 @@ class SecureChannelSingleClientMessageProxyImplTest : public testing::Test { ...@@ -43,8 +44,9 @@ class SecureChannelSingleClientMessageProxyImplTest : public testing::Test {
std::move(fake_message_receiver)); std::move(fake_message_receiver));
proxy_ = SingleClientMessageProxyImpl::Factory::Get()->BuildInstance( proxy_ = SingleClientMessageProxyImpl::Factory::Get()->BuildInstance(
fake_proxy_delegate_.get(), kTestFeature, fake_proxy_delegate_.get(),
fake_connection_delegate_->GenerateInterfacePtr()); ClientConnectionParameters(
kTestFeature, fake_connection_delegate_->GenerateInterfacePtr()));
CompletePendingMojoCalls(); CompletePendingMojoCalls();
EXPECT_TRUE(fake_connection_delegate_->channel()); EXPECT_TRUE(fake_connection_delegate_->channel());
...@@ -152,7 +154,8 @@ class SecureChannelSingleClientMessageProxyImplTest : public testing::Test { ...@@ -152,7 +154,8 @@ class SecureChannelSingleClientMessageProxyImplTest : public testing::Test {
} }
bool WasDelegateNotifiedOfDisconnection() { bool WasDelegateNotifiedOfDisconnection() {
return proxy_->proxy_id() == fake_proxy_delegate_->disconnected_proxy_id(); return proxy_->GetProxyId() ==
fake_proxy_delegate_->disconnected_proxy_id();
} }
const mojom::ConnectionMetadata& GetConnectionMetadataFromChannel() { const mojom::ConnectionMetadata& GetConnectionMetadataFromChannel() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment