Add support for unsigned parameters in FieldTrialParser
Bug: webrtc:10932
Change-Id: I3f56244a6be532065e4096cf1a289e27a032bc44
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150886
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29018}
diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn
index 830fd50..0101fd8 100644
--- a/rtc_base/experiments/BUILD.gn
+++ b/rtc_base/experiments/BUILD.gn
@@ -56,6 +56,7 @@
"../../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 4f68e5c..5f33b6e 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 <inttypes.h>
+
#include <algorithm>
#include <map>
#include <type_traits>
@@ -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 @@
template <>
absl::optional<int> ParseTypedParameter<int>(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<int, int64_t>(value)) {
+ return static_cast<int>(value);
+ }
}
+ return absl::nullopt;
+}
+
+template <>
+absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str) {
+ int64_t value;
+ if (sscanf(str.c_str(), "%" SCNd64, &value) == 1) {
+ if (rtc::IsValueInRangeForNumericType<unsigned, int64_t>(value)) {
+ return static_cast<unsigned>(value);
+ }
+ }
+ return absl::nullopt;
}
template <>
@@ -140,6 +155,11 @@
return ParseOptionalParameter<int>(str);
}
template <>
+absl::optional<absl::optional<unsigned>>
+ParseTypedParameter<absl::optional<unsigned>>(std::string str) {
+ return ParseOptionalParameter<unsigned>(str);
+}
+template <>
absl::optional<absl::optional<double>>
ParseTypedParameter<absl::optional<double>>(std::string str) {
return ParseOptionalParameter<double>(str);
@@ -205,13 +225,16 @@
template class FieldTrialParameter<bool>;
template class FieldTrialParameter<double>;
template class FieldTrialParameter<int>;
+template class FieldTrialParameter<unsigned>;
template class FieldTrialParameter<std::string>;
template class FieldTrialConstrained<double>;
template class FieldTrialConstrained<int>;
+template class FieldTrialConstrained<unsigned>;
template class FieldTrialOptional<double>;
template class FieldTrialOptional<int>;
+template class FieldTrialOptional<unsigned>;
template class FieldTrialOptional<bool>;
template class FieldTrialOptional<std::string>;
diff --git a/rtc_base/experiments/field_trial_parser.h b/rtc_base/experiments/field_trial_parser.h
index 997a7fd..42535ed 100644
--- a/rtc_base/experiments/field_trial_parser.h
+++ b/rtc_base/experiments/field_trial_parser.h
@@ -245,6 +245,8 @@
template <>
absl::optional<int> ParseTypedParameter<int>(std::string str);
template <>
+absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str);
+template <>
absl::optional<std::string> ParseTypedParameter<std::string>(std::string str);
template <>
@@ -254,6 +256,9 @@
absl::optional<absl::optional<int>> ParseTypedParameter<absl::optional<int>>(
std::string str);
template <>
+absl::optional<absl::optional<unsigned>>
+ParseTypedParameter<absl::optional<unsigned>>(std::string str);
+template <>
absl::optional<absl::optional<double>>
ParseTypedParameter<absl::optional<double>>(std::string str);
@@ -263,14 +268,18 @@
extern template class FieldTrialParameter<double>;
// Interpreted using sscanf %i.
extern template class FieldTrialParameter<int>;
+// Interpreted using sscanf %u.
+extern template class FieldTrialParameter<unsigned>;
// Using the given value as is.
extern template class FieldTrialParameter<std::string>;
extern template class FieldTrialConstrained<double>;
extern template class FieldTrialConstrained<int>;
+extern template class FieldTrialConstrained<unsigned>;
extern template class FieldTrialOptional<double>;
extern template class FieldTrialOptional<int>;
+extern template class FieldTrialOptional<unsigned>;
extern template class FieldTrialOptional<bool>;
extern template class FieldTrialOptional<std::string>;
diff --git a/rtc_base/experiments/field_trial_parser_unittest.cc b/rtc_base/experiments/field_trial_parser_unittest.cc
index cf483d7..d36b3c7 100644
--- a/rtc_base/experiments/field_trial_parser_unittest.cc
+++ b/rtc_base/experiments/field_trial_parser_unittest.cc
@@ -23,16 +23,19 @@
FieldTrialFlag enabled = FieldTrialFlag("Enabled");
FieldTrialParameter<double> factor = FieldTrialParameter<double>("f", 0.5);
FieldTrialParameter<int> retries = FieldTrialParameter<int>("r", 5);
+ FieldTrialParameter<unsigned> size = FieldTrialParameter<unsigned>("s", 3);
FieldTrialParameter<bool> ping = FieldTrialParameter<bool>("p", 0);
FieldTrialParameter<std::string> hash =
FieldTrialParameter<std::string>("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 @@
} // 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 @@
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 @@
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 @@
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 @@
ParseFieldTrial({&low, &high}, "low:20,high:20");
EXPECT_EQ(low.Get(), 20);
EXPECT_EQ(high.Get(), 20);
+
+ FieldTrialConstrained<unsigned> size("size", 5, 1, 10);
+ ParseFieldTrial({&size}, "size:0");
+ EXPECT_EQ(size.Get(), 5u);
}
TEST(FieldTrialParserTest, ReadsValuesFromFieldWithoutKey) {
FieldTrialFlag enabled("Enabled");
@@ -136,6 +148,17 @@
EXPECT_EQ(max_count.GetOptional().value(), 20);
ParseFieldTrial({&max_count}, "c:");
EXPECT_EQ(max_count.GetOptional().value(), 20);
+
+ FieldTrialOptional<unsigned> 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<std::string> 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 24058b5..2605da8 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 <algorithm>
+
#include "rtc_base/logging.h"
namespace webrtc {
@@ -31,6 +33,9 @@
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 @@
template class TypedParser<bool>;
template class TypedParser<double>;
template class TypedParser<int>;
+template class TypedParser<unsigned>;
template class TypedParser<absl::optional<double>>;
template class TypedParser<absl::optional<int>>;
+template class TypedParser<absl::optional<unsigned>>;
template class TypedParser<DataRate>;
template class TypedParser<DataSize>;
diff --git a/rtc_base/experiments/struct_parameters_parser.h b/rtc_base/experiments/struct_parameters_parser.h
index b40f381..523ecfb 100644
--- a/rtc_base/experiments/struct_parameters_parser.h
+++ b/rtc_base/experiments/struct_parameters_parser.h
@@ -53,8 +53,10 @@
extern template class TypedParser<bool>;
extern template class TypedParser<double>;
extern template class TypedParser<int>;
+extern template class TypedParser<unsigned>;
extern template class TypedParser<absl::optional<double>>;
extern template class TypedParser<absl::optional<int>>;
+extern template class TypedParser<absl::optional<unsigned>>;
extern template class TypedParser<DataRate>;
extern template class TypedParser<DataSize>;
diff --git a/rtc_base/experiments/struct_parameters_parser_unittest.cc b/rtc_base/experiments/struct_parameters_parser_unittest.cc
index 69103bd..71b117f 100644
--- a/rtc_base/experiments/struct_parameters_parser_unittest.cc
+++ b/rtc_base/experiments/struct_parameters_parser_unittest.cc
@@ -16,6 +16,7 @@
bool enabled = false;
double factor = 0.5;
int retries = 5;
+ unsigned size = 3;
bool ping = 0;
absl::optional<TimeDelta> duration;
absl::optional<TimeDelta> latency = TimeDelta::ms(100);
@@ -27,6 +28,7 @@
return StructParametersParser::Create("e", &enabled, //
"f", &factor, //
"r", &retries, //
+ "s", &size, //
"p", &ping, //
"d", &duration, //
"l", &latency);
@@ -35,10 +37,11 @@
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 @@
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 @@
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