Inject Environment instead of just field trials to OpenSSLAdapter To prepare to use propagated rather than globally provided clock for testing Bug: webrtc:42223992 Change-Id: I71ed6b19cec446c73f95777a4f6ad3001788d25a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/466580 Reviewed-by: Evan Shrubsole <eshr@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47545}
diff --git a/p2p/dtls/dtls_transport.cc b/p2p/dtls/dtls_transport.cc index 3b6e75f..36e9469 100644 --- a/p2p/dtls/dtls_transport.cc +++ b/p2p/dtls/dtls_transport.cc
@@ -541,14 +541,12 @@ } if (ssl_stream_factory_) { dtls_ = ssl_stream_factory_( - std::move(downward), - [this](SSLHandshakeError error) { OnDtlsHandshakeError(error); }, - &env_.field_trials()); + env_, std::move(downward), + [this](SSLHandshakeError error) { OnDtlsHandshakeError(error); }); } else { dtls_ = SSLStreamAdapter::Create( - std::move(downward), - [this](SSLHandshakeError error) { OnDtlsHandshakeError(error); }, - &env_.field_trials()); + env_, std::move(downward), + [this](SSLHandshakeError error) { OnDtlsHandshakeError(error); }); } if (!dtls_) { RTC_LOG(LS_ERROR) << ToString() << ": Failed to create DTLS adapter.";
diff --git a/p2p/dtls/dtls_transport.h b/p2p/dtls/dtls_transport.h index 9eed516..54d7fd4 100644 --- a/p2p/dtls/dtls_transport.h +++ b/p2p/dtls/dtls_transport.h
@@ -25,7 +25,6 @@ #include "api/crypto/crypto_options.h" #include "api/dtls_transport_interface.h" #include "api/environment/environment.h" -#include "api/field_trials_view.h" #include "api/ice_transport_interface.h" #include "api/rtc_error.h" #include "api/scoped_refptr.h" @@ -144,9 +143,9 @@ // For testing purposes only. using SslStreamFactory = std::function<std::unique_ptr<SSLStreamAdapter>( + const Environment&, std::unique_ptr<StreamInterface>, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_callback, - const FieldTrialsView* field_trials)>; + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_callback)>; // `ice_transport` is the ICE transport this DTLS transport is wrapping. It // must outlive this DTLS transport.
diff --git a/p2p/dtls/dtls_transport_unittest.cc b/p2p/dtls/dtls_transport_unittest.cc index 70632f3..181550a 100644 --- a/p2p/dtls/dtls_transport_unittest.cc +++ b/p2p/dtls/dtls_transport_unittest.cc
@@ -30,7 +30,6 @@ #include "api/dtls_transport_interface.h" #include "api/environment/environment.h" #include "api/field_trials.h" -#include "api/field_trials_view.h" #include "api/ice_transport_interface.h" #include "api/make_ref_counted.h" #include "api/scoped_refptr.h" @@ -836,13 +835,12 @@ PrepareDtls(KT_DEFAULT); FakeSSLStreamAdapter* fake_stream = nullptr; client1_.set_ssl_stream_factory( - [&](std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_callback, - const FieldTrialsView* field_trials) { + [&](const Environment& env, std::unique_ptr<StreamInterface> stream, + absl::AnyInvocable<void(SSLHandshakeError)> + handshake_error_callback) { auto fake = std::make_unique<FakeSSLStreamAdapter>(SSLStreamAdapter::Create( - std::move(stream), std::move(handshake_error_callback), - field_trials)); + env, std::move(stream), std::move(handshake_error_callback))); fake->Init(); fake_stream = fake.get(); return fake; @@ -859,13 +857,12 @@ PrepareDtls(KT_DEFAULT); FakeSSLStreamAdapter* fake_stream = nullptr; client1_.set_ssl_stream_factory( - [&](std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_callback, - const FieldTrialsView* field_trials) { + [&](const Environment& env, std::unique_ptr<StreamInterface> stream, + absl::AnyInvocable<void(SSLHandshakeError)> + handshake_error_callback) { auto fake = std::make_unique<FakeSSLStreamAdapter>(SSLStreamAdapter::Create( - std::move(stream), std::move(handshake_error_callback), - field_trials)); + env, std::move(stream), std::move(handshake_error_callback))); fake->Init(); fake_stream = fake.get(); return fake;
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index abbc798..90f907c 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn
@@ -1681,11 +1681,14 @@ ":timeutils", "../api:field_trials_view", "../api:sequence_checker", + "../api/environment", "../api/task_queue", "../api/task_queue:pending_task_safety_flag", "../api/units:time_delta", "system:rtc_export", "task_utils:repeating_task", + "//third_party/abseil-cpp/absl/base:core_headers", + "//third_party/abseil-cpp/absl/base:nullability", "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings",
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index b0464ad..403f4ca 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc
@@ -29,6 +29,7 @@ #include "absl/functional/any_invocable.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" +#include "api/environment/environment.h" #include "api/field_trials_view.h" #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" @@ -288,10 +289,11 @@ ///////////////////////////////////////////////////////////////////////////// OpenSSLStreamAdapter::OpenSSLStreamAdapter( + std::optional<Environment> env, std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error, - const FieldTrialsView* field_trials) - : stream_(std::move(stream)), + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error) + : env_(std::move(env)), + stream_(std::move(stream)), handshake_error_(std::move(handshake_error)), owner_(TaskQueueBase::Current()), state_(SSL_NONE), @@ -302,8 +304,9 @@ ssl_ctx_(nullptr), ssl_mode_(SSL_MODE_DTLS), ssl_max_version_(MAX_SSL_PROTOCOL_DTLS), - disable_ssl_group_ids_(field_trials && field_trials->IsEnabled( - "WebRTC-DisableSslGroupIds")) { + disable_ssl_group_ids_( + env_.has_value() && + env_->field_trials().IsEnabled("WebRTC-DisableSslGroupIds")) { stream_->SetEventCallback( [this](int events, int err) { OnEvent(events, err); }); }
diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index 50ef419..6aa5fd1 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h
@@ -23,21 +23,22 @@ #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" +#include "api/environment/environment.h" +#include "api/task_queue/pending_task_safety_flag.h" +#include "api/task_queue/task_queue_base.h" #include "rtc_base/buffer.h" #include "rtc_base/ssl_certificate.h" +#include "rtc_base/ssl_identity.h" +#include "rtc_base/ssl_stream_adapter.h" +#include "rtc_base/stream.h" +#include "rtc_base/task_utils/repeating_task.h" + #ifdef OPENSSL_IS_BORINGSSL #include "rtc_base/boringssl_identity.h" #include "rtc_base/openssl.h" #else #include "rtc_base/openssl_identity.h" #endif -#include "api/field_trials_view.h" -#include "api/task_queue/pending_task_safety_flag.h" -#include "api/task_queue/task_queue_base.h" -#include "rtc_base/ssl_identity.h" -#include "rtc_base/ssl_stream_adapter.h" -#include "rtc_base/stream.h" -#include "rtc_base/task_utils/repeating_task.h" namespace webrtc { @@ -70,9 +71,9 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { public: OpenSSLStreamAdapter( + std::optional<Environment> env, std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error, - const FieldTrialsView* field_trials = nullptr); + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error); ~OpenSSLStreamAdapter() override; void SetIdentity(std::unique_ptr<SSLIdentity> identity) override; @@ -215,6 +216,7 @@ void MaybeSetTimeout(); + const std::optional<Environment> env_; const std::unique_ptr<StreamInterface> stream_; absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_;
diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc index 59216f0..8b6004a 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc
@@ -22,6 +22,7 @@ #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" +#include "api/environment/environment.h" #include "api/field_trials_view.h" #include "rtc_base/openssl_stream_adapter.h" #include "rtc_base/ssl_identity.h" @@ -86,10 +87,17 @@ std::unique_ptr<SSLStreamAdapter> SSLStreamAdapter::Create( std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error, - const FieldTrialsView* field_trials) { - return std::make_unique<OpenSSLStreamAdapter>( - std::move(stream), std::move(handshake_error), field_trials); + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error) { + return std::make_unique<OpenSSLStreamAdapter>(std::nullopt, std::move(stream), + std::move(handshake_error)); +} + +std::unique_ptr<SSLStreamAdapter> SSLStreamAdapter::Create( + const Environment& env, + std::unique_ptr<StreamInterface> stream, + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error) { + return std::make_unique<OpenSSLStreamAdapter>(env, std::move(stream), + std::move(handshake_error)); } bool SSLStreamAdapter::IsBoringSsl() {
diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index 75e6729..0568bd0 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h
@@ -19,10 +19,14 @@ #include <set> #include <span> #include <string> +#include <utility> #include <vector> +#include "absl/base/macros.h" +#include "absl/base/nullability.h" #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" +#include "api/environment/environment.h" #include "api/field_trials_view.h" #include "rtc_base/buffer.h" #include "rtc_base/checks.h" @@ -121,13 +125,22 @@ class SSLStreamAdapter : public StreamInterface { public: - // Instantiate an SSLStreamAdapter wrapping the given stream, - // (using the selected implementation for the platform). - // Caller is responsible for freeing the returned object. + ABSL_DEPRECATE_AND_INLINE() static std::unique_ptr<SSLStreamAdapter> Create( std::unique_ptr<StreamInterface> stream, - absl::AnyInvocable<void(SSLHandshakeError)> handshake_error = nullptr, - const FieldTrialsView* field_trials = nullptr); + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error, + nullptr_t /*field_trials*/) { + return Create(std::move(stream), std::move(handshake_error)); + } + + static absl_nonnull std::unique_ptr<SSLStreamAdapter> Create( + std::unique_ptr<StreamInterface> stream, + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error = nullptr); + + static absl_nonnull std::unique_ptr<SSLStreamAdapter> Create( + const Environment& env, + std::unique_ptr<StreamInterface> stream, + absl::AnyInvocable<void(SSLHandshakeError)> handshake_error); SSLStreamAdapter() = default; ~SSLStreamAdapter() override = default;
diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc index 7704a3b..0cbc02e 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc
@@ -34,7 +34,6 @@ #include "absl/memory/memory.h" #include "absl/strings/string_view.h" #include "api/crypto/crypto_options.h" -#include "api/field_trials.h" #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/test/rtc_error_matchers.h" @@ -51,7 +50,7 @@ #include "rtc_base/ssl_identity.h" #include "rtc_base/stream.h" #include "rtc_base/thread.h" -#include "test/create_test_field_trials.h" +#include "test/create_test_environment.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/time_controller/simulated_time_controller.h" @@ -474,19 +473,16 @@ void InitializeClientAndServerStreams( absl::string_view client_experiment = "", absl::string_view server_experiment = "") { - // Note: `client_ssl_` and `server_ssl_` may be non-nullptr. + client_ssl_ = SSLStreamAdapter::Create( + CreateTestEnvironment( + {.field_trials = client_experiment, .time = &time_controller_}), + CreateClientStream(), /*handshake_error=*/nullptr); - // The field trials are read when the OpenSSLStreamAdapter is initialized. - { - FieldTrials trial = CreateTestFieldTrials(client_experiment); - client_ssl_ = - SSLStreamAdapter::Create(CreateClientStream(), nullptr, &trial); - } - { - FieldTrials trial = CreateTestFieldTrials(server_experiment); - server_ssl_ = - SSLStreamAdapter::Create(CreateServerStream(), nullptr, &trial); - } + server_ssl_ = SSLStreamAdapter::Create( + CreateTestEnvironment( + {.field_trials = server_experiment, .time = &time_controller_}), + CreateServerStream(), /*handshake_error=*/nullptr); + client_ssl_->SetEventCallback( [this](int events, int err) { OnClientEvent(events, err); }); server_ssl_->SetEventCallback(