diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index 830fd50a06..0101fd8a34 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -56,6 +56,7 @@ rtc_static_library("field_trial_parser") { "../../api/units:time_delta", "../../rtc_base:checks", "../../rtc_base:logging", + "../../rtc_base:safe_conversions", "../../rtc_base:stringutils", "//third_party/abseil-cpp/absl/memory:memory", "//third_party/abseil-cpp/absl/strings:strings", diff --git a/rtc_base/experiments/field_trial_parser.cc b/rtc_base/experiments/field_trial_parser.cc index 4f68e5c592..5f33b6eff8 100644 --- a/rtc_base/experiments/field_trial_parser.cc +++ b/rtc_base/experiments/field_trial_parser.cc @@ -9,6 +9,8 @@ */ #include "rtc_base/experiments/field_trial_parser.h" +#include + #include #include #include @@ -16,6 +18,7 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/numerics/safe_conversions.h" namespace webrtc { namespace { @@ -116,12 +119,24 @@ absl::optional ParseTypedParameter(std::string str) { template <> absl::optional ParseTypedParameter(std::string str) { - int value; - if (sscanf(str.c_str(), "%i", &value) == 1) { - return value; - } else { - return absl::nullopt; + int64_t value; + if (sscanf(str.c_str(), "%" SCNd64, &value) == 1) { + if (rtc::IsValueInRangeForNumericType(value)) { + return static_cast(value); + } } + return absl::nullopt; +} + +template <> +absl::optional ParseTypedParameter(std::string str) { + int64_t value; + if (sscanf(str.c_str(), "%" SCNd64, &value) == 1) { + if (rtc::IsValueInRangeForNumericType(value)) { + return static_cast(value); + } + } + return absl::nullopt; } template <> @@ -140,6 +155,11 @@ absl::optional> ParseTypedParameter>( return ParseOptionalParameter(str); } template <> +absl::optional> +ParseTypedParameter>(std::string str) { + return ParseOptionalParameter(str); +} +template <> absl::optional> ParseTypedParameter>(std::string str) { return ParseOptionalParameter(str); @@ -205,13 +225,16 @@ bool AbstractFieldTrialEnum::Parse(absl::optional str_value) { template class FieldTrialParameter; template class FieldTrialParameter; template class FieldTrialParameter; +template class FieldTrialParameter; template class FieldTrialParameter; template class FieldTrialConstrained; template class FieldTrialConstrained; +template class FieldTrialConstrained; template class FieldTrialOptional; template class FieldTrialOptional; +template class FieldTrialOptional; template class FieldTrialOptional; template class FieldTrialOptional; diff --git a/rtc_base/experiments/field_trial_parser.h b/rtc_base/experiments/field_trial_parser.h index 997a7fd1aa..42535ed6a4 100644 --- a/rtc_base/experiments/field_trial_parser.h +++ b/rtc_base/experiments/field_trial_parser.h @@ -245,6 +245,8 @@ absl::optional ParseTypedParameter(std::string str); template <> absl::optional ParseTypedParameter(std::string str); template <> +absl::optional ParseTypedParameter(std::string str); +template <> absl::optional ParseTypedParameter(std::string str); template <> @@ -254,6 +256,9 @@ template <> absl::optional> ParseTypedParameter>( std::string str); template <> +absl::optional> +ParseTypedParameter>(std::string str); +template <> absl::optional> ParseTypedParameter>(std::string str); @@ -263,14 +268,18 @@ extern template class FieldTrialParameter; extern template class FieldTrialParameter; // Interpreted using sscanf %i. extern template class FieldTrialParameter; +// Interpreted using sscanf %u. +extern template class FieldTrialParameter; // Using the given value as is. extern template class FieldTrialParameter; extern template class FieldTrialConstrained; extern template class FieldTrialConstrained; +extern template class FieldTrialConstrained; extern template class FieldTrialOptional; extern template class FieldTrialOptional; +extern template class FieldTrialOptional; extern template class FieldTrialOptional; extern template class FieldTrialOptional; diff --git a/rtc_base/experiments/field_trial_parser_unittest.cc b/rtc_base/experiments/field_trial_parser_unittest.cc index cf483d7334..d36b3c7d95 100644 --- a/rtc_base/experiments/field_trial_parser_unittest.cc +++ b/rtc_base/experiments/field_trial_parser_unittest.cc @@ -23,16 +23,19 @@ struct DummyExperiment { FieldTrialFlag enabled = FieldTrialFlag("Enabled"); FieldTrialParameter factor = FieldTrialParameter("f", 0.5); FieldTrialParameter retries = FieldTrialParameter("r", 5); + FieldTrialParameter size = FieldTrialParameter("s", 3); FieldTrialParameter ping = FieldTrialParameter("p", 0); FieldTrialParameter hash = FieldTrialParameter("h", "a80"); explicit DummyExperiment(std::string field_trial) { - ParseFieldTrial({&enabled, &factor, &retries, &ping, &hash}, field_trial); + ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash}, + field_trial); } DummyExperiment() { std::string trial_string = field_trial::FindFullName(kDummyExperiment); - ParseFieldTrial({&enabled, &factor, &retries, &ping, &hash}, trial_string); + ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash}, + trial_string); } }; @@ -45,22 +48,24 @@ enum class CustomEnum { } // namespace TEST(FieldTrialParserTest, ParsesValidParameters) { - DummyExperiment exp("Enabled,f:-1.7,r:2,p:1,h:x7c"); + DummyExperiment exp("Enabled,f:-1.7,r:2,s:10,p:1,h:x7c"); EXPECT_TRUE(exp.enabled.Get()); EXPECT_EQ(exp.factor.Get(), -1.7); EXPECT_EQ(exp.retries.Get(), 2); + EXPECT_EQ(exp.size.Get(), 10u); EXPECT_EQ(exp.ping.Get(), true); EXPECT_EQ(exp.hash.Get(), "x7c"); } TEST(FieldTrialParserTest, InitializesFromFieldTrial) { test::ScopedFieldTrials field_trials( "WebRTC-OtherExperiment/Disabled/" - "WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,p:1,h:x7c/" + "WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,s:10,p:1,h:x7c/" "WebRTC-AnotherExperiment/Enabled,f:-3.1,otherstuff:beef/"); DummyExperiment exp; EXPECT_TRUE(exp.enabled.Get()); EXPECT_EQ(exp.factor.Get(), -1.7); EXPECT_EQ(exp.retries.Get(), 2); + EXPECT_EQ(exp.size.Get(), 10u); EXPECT_EQ(exp.ping.Get(), true); EXPECT_EQ(exp.hash.Get(), "x7c"); } @@ -69,6 +74,7 @@ TEST(FieldTrialParserTest, UsesDefaults) { EXPECT_FALSE(exp.enabled.Get()); EXPECT_EQ(exp.factor.Get(), 0.5); EXPECT_EQ(exp.retries.Get(), 5); + EXPECT_EQ(exp.size.Get(), 3u); EXPECT_EQ(exp.ping.Get(), false); EXPECT_EQ(exp.hash.Get(), "a80"); } @@ -77,6 +83,7 @@ TEST(FieldTrialParserTest, CanHandleMixedInput) { EXPECT_TRUE(exp.enabled.Get()); EXPECT_EQ(exp.factor.Get(), 0.5); EXPECT_EQ(exp.retries.Get(), 5); + EXPECT_EQ(exp.size.Get(), 3u); EXPECT_EQ(exp.ping.Get(), true); EXPECT_EQ(exp.hash.Get(), ""); } @@ -96,10 +103,11 @@ TEST(FieldTrialParserTest, IgnoresNewKey) { EXPECT_EQ(exp.retries.Get(), -11); } TEST(FieldTrialParserTest, IgnoresInvalid) { - DummyExperiment exp("Enabled,f,p:,r:%,,:foo,h"); + DummyExperiment exp("Enabled,f,p:,r:%,,s:-1,:foo,h"); EXPECT_TRUE(exp.enabled.Get()); EXPECT_EQ(exp.factor.Get(), 0.5); EXPECT_EQ(exp.retries.Get(), 5); + EXPECT_EQ(exp.size.Get(), 3u); EXPECT_EQ(exp.ping.Get(), false); EXPECT_EQ(exp.hash.Get(), "a80"); } @@ -115,6 +123,10 @@ TEST(FieldTrialParserTest, IgnoresOutOfRange) { ParseFieldTrial({&low, &high}, "low:20,high:20"); EXPECT_EQ(low.Get(), 20); EXPECT_EQ(high.Get(), 20); + + FieldTrialConstrained size("size", 5, 1, 10); + ParseFieldTrial({&size}, "size:0"); + EXPECT_EQ(size.Get(), 5u); } TEST(FieldTrialParserTest, ReadsValuesFromFieldWithoutKey) { FieldTrialFlag enabled("Enabled"); @@ -136,6 +148,17 @@ TEST(FieldTrialParserTest, ParsesOptionalParameters) { EXPECT_EQ(max_count.GetOptional().value(), 20); ParseFieldTrial({&max_count}, "c:"); EXPECT_EQ(max_count.GetOptional().value(), 20); + + FieldTrialOptional max_size("c", absl::nullopt); + ParseFieldTrial({&max_size}, ""); + EXPECT_FALSE(max_size.GetOptional().has_value()); + ParseFieldTrial({&max_size}, "c:10"); + EXPECT_EQ(max_size.GetOptional().value(), 10u); + ParseFieldTrial({&max_size}, "c"); + EXPECT_FALSE(max_size.GetOptional().has_value()); + ParseFieldTrial({&max_size}, "c:20"); + EXPECT_EQ(max_size.GetOptional().value(), 20u); + FieldTrialOptional optional_string("s", std::string("ab")); ParseFieldTrial({&optional_string}, "s:"); EXPECT_EQ(optional_string.GetOptional().value(), ""); diff --git a/rtc_base/experiments/struct_parameters_parser.cc b/rtc_base/experiments/struct_parameters_parser.cc index 24058b50bd..2605da8fef 100644 --- a/rtc_base/experiments/struct_parameters_parser.cc +++ b/rtc_base/experiments/struct_parameters_parser.cc @@ -9,6 +9,8 @@ */ #include "rtc_base/experiments/struct_parameters_parser.h" +#include + #include "rtc_base/logging.h" namespace webrtc { @@ -31,6 +33,9 @@ inline void StringEncode(std::string* target, double val) { inline void StringEncode(std::string* target, int val) { *target += rtc::ToString(val); } +inline void StringEncode(std::string* target, unsigned val) { + *target += rtc::ToString(val); +} inline void StringEncode(std::string* target, DataRate val) { *target += webrtc::ToString(val); } @@ -62,8 +67,10 @@ void TypedParser::Encode(const void* src, std::string* target) { template class TypedParser; template class TypedParser; template class TypedParser; +template class TypedParser; template class TypedParser>; template class TypedParser>; +template class TypedParser>; template class TypedParser; template class TypedParser; diff --git a/rtc_base/experiments/struct_parameters_parser.h b/rtc_base/experiments/struct_parameters_parser.h index b40f381594..523ecfb05d 100644 --- a/rtc_base/experiments/struct_parameters_parser.h +++ b/rtc_base/experiments/struct_parameters_parser.h @@ -53,8 +53,10 @@ class TypedParser { extern template class TypedParser; extern template class TypedParser; extern template class TypedParser; +extern template class TypedParser; extern template class TypedParser>; extern template class TypedParser>; +extern template class TypedParser>; extern template class TypedParser; extern template class TypedParser; diff --git a/rtc_base/experiments/struct_parameters_parser_unittest.cc b/rtc_base/experiments/struct_parameters_parser_unittest.cc index 69103bd046..71b117f9dd 100644 --- a/rtc_base/experiments/struct_parameters_parser_unittest.cc +++ b/rtc_base/experiments/struct_parameters_parser_unittest.cc @@ -16,6 +16,7 @@ struct DummyConfig { bool enabled = false; double factor = 0.5; int retries = 5; + unsigned size = 3; bool ping = 0; absl::optional duration; absl::optional latency = TimeDelta::ms(100); @@ -27,6 +28,7 @@ std::unique_ptr DummyConfig::Parser() { return StructParametersParser::Create("e", &enabled, // "f", &factor, // "r", &retries, // + "s", &size, // "p", &ping, // "d", &duration, // "l", &latency); @@ -35,10 +37,11 @@ std::unique_ptr DummyConfig::Parser() { TEST(StructParametersParserTest, ParsesValidParameters) { DummyConfig exp; - exp.Parser()->Parse("e:1,f:-1.7,r:2,p:1,d:8,l:,"); + exp.Parser()->Parse("e:1,f:-1.7,r:2,s:7,p:1,d:8,l:,"); EXPECT_TRUE(exp.enabled); EXPECT_EQ(exp.factor, -1.7); EXPECT_EQ(exp.retries, 2); + EXPECT_EQ(exp.size, 7u); EXPECT_EQ(exp.ping, true); EXPECT_EQ(exp.duration.value().ms(), 8); EXPECT_FALSE(exp.latency); @@ -50,6 +53,7 @@ TEST(StructParametersParserTest, UsesDefaults) { EXPECT_FALSE(exp.enabled); EXPECT_EQ(exp.factor, 0.5); EXPECT_EQ(exp.retries, 5); + EXPECT_EQ(exp.size, 3u); EXPECT_EQ(exp.ping, false); } @@ -57,7 +61,7 @@ TEST(StructParametersParserTest, EncodeAll) { DummyConfig exp; auto encoded = exp.Parser()->Encode(); // All parameters are encoded. - EXPECT_EQ(encoded, "e:false,f:0.5,r:5,p:false,d:,l:100 ms"); + EXPECT_EQ(encoded, "e:false,f:0.5,r:5,s:3,p:false,d:,l:100 ms"); } } // namespace webrtc