Extend FieldTrials api
Intent of new api is to replace
field_trial::FieldTrialsStringIsValid with FieldTrials::Create
field_trial::MergeFieldTrialsStrings with FieldTrials::Merge and FieldTrials::Set
Thus allow to assemble field trials without relying on functions designed for creating global field trials string.
Bug: webrtc:42220378
Change-Id: Icc10b202f4d9c8d540fe579bd213d3c83c415263
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/392461
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44700}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index d0e6394..d7d103d 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -1712,6 +1712,8 @@
":field_trials_registry",
"../rtc_base:checks",
"../rtc_base/containers:flat_map",
+ "//third_party/abseil-cpp/absl/base:nullability",
+ "//third_party/abseil-cpp/absl/memory",
"//third_party/abseil-cpp/absl/strings:string_view",
]
}
diff --git a/api/field_trials.cc b/api/field_trials.cc
index c39d04c..fc896e1 100644
--- a/api/field_trials.cc
+++ b/api/field_trials.cc
@@ -10,36 +10,78 @@
#include "api/field_trials.h"
+#include <memory>
#include <string>
#include <utility>
+#include "absl/base/nullability.h"
+#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/containers/flat_map.h"
namespace webrtc {
+namespace {
+
+absl::string_view NextKeyOrValue(absl::string_view& s) {
+ absl::string_view::size_type separator_pos = s.find('/');
+ if (separator_pos == absl::string_view::npos) {
+ // Missing separator '/' after field trial key or value.
+ return "";
+ }
+ absl::string_view result = s.substr(0, separator_pos);
+ s.remove_prefix(separator_pos + 1);
+ return result;
+}
+
+bool Parse(absl::string_view s,
+ flat_map<std::string, std::string>& key_value_map) {
+ while (!s.empty()) {
+ absl::string_view key = NextKeyOrValue(s);
+ absl::string_view value = NextKeyOrValue(s);
+ if (key.empty() || value.empty()) {
+ return false;
+ }
+
+ auto it = key_value_map.emplace(key, value).first;
+ if (it->second != value) {
+ // Duplicate trials with different values is not fine.
+ return false;
+ }
+ }
+ return true;
+}
+
+} // namespace
+
+absl_nullable std::unique_ptr<FieldTrials> FieldTrials::Create(
+ absl::string_view s) {
+ flat_map<std::string, std::string> key_value_map;
+ if (!Parse(s, key_value_map)) {
+ return nullptr;
+ }
+ // Using `new` to access a private constructor.
+ return absl::WrapUnique(new FieldTrials(std::move(key_value_map)));
+}
FieldTrials::FieldTrials(absl::string_view s) {
- while (!s.empty()) {
- absl::string_view::size_type separator_pos = s.find('/');
- RTC_CHECK_NE(separator_pos, absl::string_view::npos)
- << "Missing separator '/' after field trial key.";
- RTC_CHECK_GT(separator_pos, 0) << "Field trial key cannot be empty.";
- absl::string_view key = s.substr(0, separator_pos);
- s.remove_prefix(separator_pos + 1);
+ RTC_CHECK(Parse(s, key_value_map_));
+}
- RTC_CHECK(!s.empty())
- << "Missing value after field trial key. String ended.";
- separator_pos = s.find('/');
- RTC_CHECK_NE(separator_pos, absl::string_view::npos)
- << "Missing terminating '/' in field trial string.";
- RTC_CHECK_GT(separator_pos, 0) << "Field trial value cannot be empty.";
- absl::string_view value = s.substr(0, separator_pos);
- s.remove_prefix(separator_pos + 1);
+void FieldTrials::Merge(const FieldTrials& other) {
+ for (const auto& [trial, group] : other.key_value_map_) {
+ key_value_map_.insert_or_assign(trial, group);
+ }
+}
- // If a key is specified multiple times, only the value linked to the first
- // key is stored.
- key_value_map_.emplace(key, value);
+void FieldTrials::Set(absl::string_view trial, absl::string_view group) {
+ RTC_CHECK(!trial.empty());
+ RTC_CHECK_EQ(trial.find('/'), absl::string_view::npos);
+ RTC_CHECK_EQ(group.find('/'), absl::string_view::npos);
+ if (group.empty()) {
+ key_value_map_.erase(trial);
+ } else {
+ key_value_map_.insert_or_assign(trial, group);
}
}
diff --git a/api/field_trials.h b/api/field_trials.h
index ff42c32..4937b50 100644
--- a/api/field_trials.h
+++ b/api/field_trials.h
@@ -13,7 +13,9 @@
#include <memory>
#include <string>
+#include <utility>
+#include "absl/base/nullability.h"
#include "absl/strings/string_view.h"
#include "api/field_trials_registry.h"
#include "rtc_base/containers/flat_map.h"
@@ -32,10 +34,41 @@
// The field trials are injected into objects that use them at creation time.
class FieldTrials : public FieldTrialsRegistry {
public:
+ // Creates field trials from a valid field trial string.
+ // Returns nullptr if the string is invalid.
+ // E.g., valid string:
+ // "WebRTC-ExperimentFoo/Enabled/WebRTC-ExperimentBar/Enabled100kbps/"
+ // Assigns to group "Enabled" on WebRTC-ExperimentFoo trial
+ // and to group "Enabled100kbps" on WebRTC-ExperimentBar.
+ //
+ // E.g., invalid string:
+ // "WebRTC-experiment1/Enabled" (note missing / separator at the end).
+ static absl_nullable std::unique_ptr<FieldTrials> Create(absl::string_view s);
+
+ // Creates field trials from a string.
+ // It is an error to call the constructor with an invalid field trial string.
explicit FieldTrials(absl::string_view s);
+
+ FieldTrials(const FieldTrials&) = default;
FieldTrials(FieldTrials&&) = default;
+ FieldTrials& operator=(const FieldTrials&) = default;
FieldTrials& operator=(FieldTrials&&) = default;
- ~FieldTrials() = default;
+
+ ~FieldTrials() override = default;
+
+ template <typename Sink>
+ friend void AbslStringify(Sink& sink, const FieldTrials& self);
+
+ // Merges field trials from the `other` into this.
+ //
+ // If a key (trial) exists twice with conflicting values (groups), the value
+ // in `other` takes precedence.
+ void Merge(const FieldTrials& other);
+
+ // Sets value (`group`) for an indvidual `trial`.
+ // It is an error to call this function with an invalid `trial` or `group`.
+ // Setting empty `group` is valid and removes the `trial`.
+ void Set(absl::string_view trial, absl::string_view group);
// TODO: bugs.webrtc.org/42220378 - Deprecate and inline once no longer used
// within webrtc.
@@ -44,11 +77,27 @@
}
private:
+ explicit FieldTrials(flat_map<std::string, std::string> key_value_map)
+ : key_value_map_(std::move(key_value_map)) {}
+
std::string GetValue(absl::string_view key) const override;
flat_map<std::string, std::string> key_value_map_;
};
+template <typename Sink>
+void AbslStringify(Sink& sink, const FieldTrials& self) {
+ for (const auto& [trial, group] : self.key_value_map_) {
+ sink.Append(trial);
+ sink.Append("/");
+ sink.Append(group);
+ // Intentionally output a string that is not a valid field trial string.
+ // Stringification is intended only for human readable logs, and is not
+ // intended for reusing as `FieldTrials` construction parameter.
+ sink.Append("//");
+ }
+}
+
} // namespace webrtc
#endif // API_FIELD_TRIALS_H_
diff --git a/api/field_trials_unittest.cc b/api/field_trials_unittest.cc
index 9d49252..d3521b4 100644
--- a/api/field_trials_unittest.cc
+++ b/api/field_trials_unittest.cc
@@ -10,10 +10,11 @@
#include "api/field_trials.h"
-
+#include "absl/strings/str_cat.h"
#include "rtc_base/containers/flat_set.h"
#include "system_wrappers/include/field_trial.h"
#include "test/field_trial.h"
+#include "test/gmock.h"
#include "test/gtest.h"
namespace webrtc {
@@ -21,6 +22,11 @@
using field_trial::FieldTrialsAllowedInScopeForTesting;
using test::ScopedFieldTrials;
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+using ::testing::IsNull;
+using ::testing::Not;
+using ::testing::NotNull;
TEST(FieldTrialsTest, EmptyStringHasNoEffect) {
FieldTrialsAllowedInScopeForTesting k({"MyCoolTrial"});
@@ -89,5 +95,103 @@
EXPECT_TRUE(f.IsEnabled("SomeOtherString"));
}
+TEST(FieldTrialsTest, CreateAcceptsValidInputs) {
+ EXPECT_THAT(FieldTrials::Create(""), NotNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/"), NotNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Video/Disabled/"), NotNull());
+
+ // Duplicate trials with the same value is fine
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Audio/Enabled/"), NotNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/B/C/Audio/Enabled/"),
+ NotNull());
+}
+
+TEST(FieldTrialsTest, CreateRejectsBadInputs) {
+ // Bad delimiters
+ EXPECT_THAT(FieldTrials::Create("Audio/EnabledVideo/Disabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled//Video/Disabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("/Audio/Enabled/Video/Disabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Video/Disabled"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Video/Disabled/garbage"),
+ IsNull());
+
+ // Empty trial or group
+ EXPECT_THAT(FieldTrials::Create("Audio//"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("/Enabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("//"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("//Enabled"), IsNull());
+
+ // Duplicate trials with different values is not fine
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Audio/Disabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/B/C/Audio/Disabled/"),
+ IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/Audio/Disabled/"), IsNull());
+ EXPECT_THAT(FieldTrials::Create("Audio/Enabled/B/C/Audio/Disabled/"),
+ IsNull());
+}
+
+TEST(FieldTrialsTest, StringfiyMentionsKeysAndValues) {
+ // Exact format of the stringification is undefined.
+ EXPECT_THAT(absl::StrCat(FieldTrials("Audio/Enabled/Video/Value/")),
+ AllOf(HasSubstr("Audio"), HasSubstr("Enabled"),
+ HasSubstr("Video"), HasSubstr("Value")));
+}
+
+TEST(FieldTrialsTest, MergeCombinesFieldTrials) {
+ FieldTrials f("Video/Value1/");
+ FieldTrials other("Audio/Value2/");
+
+ f.Merge(other);
+
+ f.RegisterKeysForTesting({"Audio", "Video"});
+ EXPECT_EQ(f.Lookup("Video"), "Value1");
+ EXPECT_EQ(f.Lookup("Audio"), "Value2");
+}
+
+TEST(FieldTrialsTest, MergeGivesPrecedenceToOther) {
+ FieldTrials f("Audio/Disabled/Video/Enabled/");
+ FieldTrials other("Audio/Enabled/");
+
+ f.Merge(other);
+
+ f.RegisterKeysForTesting({"Audio"});
+ EXPECT_EQ(f.Lookup("Audio"), "Enabled");
+}
+
+TEST(FieldTrialsTest, MergeDoesntChangeTrialAbsentInOther) {
+ FieldTrials f("Audio/Enabled/Video/Enabled/");
+ FieldTrials other("Audio/Enabled/");
+
+ f.Merge(other);
+
+ f.RegisterKeysForTesting({"Video"});
+ EXPECT_EQ(f.Lookup("Video"), "Enabled");
+}
+
+TEST(FieldTrialsTest, SetUpdatesTrial) {
+ FieldTrials f("Audio/Enabled/Video/Enabled/");
+
+ f.Set("Audio", "Disabled");
+
+ f.RegisterKeysForTesting({"Audio"});
+ EXPECT_EQ(f.Lookup("Audio"), "Disabled");
+}
+
+TEST(FieldTrialsTest, SettingEmptyValueRemovesFieldTrial) {
+ FieldTrials f("Audio/Enabled/Video/Enabled/");
+
+ f.Set("Audio", "");
+
+ f.RegisterKeysForTesting({"Audio"});
+ EXPECT_EQ(f.Lookup("Audio"), "");
+ EXPECT_THAT(absl::StrCat(f), Not(HasSubstr("Audio")));
+
+ // Absent field trials shouldn't override previous value during merge.
+ FieldTrials f2("Audio/Disabled/");
+ f2.Merge(f);
+ f2.RegisterKeysForTesting({"Audio"});
+ EXPECT_EQ(f2.Lookup("Audio"), "Disabled");
+}
+
} // namespace
} // namespace webrtc
