mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-12 21:30:45 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Verify field trials looked up through field_trial::FindFullName

Browse source 
For now, the run-time check will only be enabled if the
rtc_strict_field_trials GN arg is set.

In order to allow testing with imaginary field trial keys, two test
helpers have been added. It's a bit awkward to test these since the
field trial string is already global, hence the helpers are also
modifying global state. Tests must make sure this global state is reset
between runs. Things won't be an issue anymore when [1] has removed the
global string.

[1] https://crbug.com/webrtc/10335

Bug: webrtc:14154
Change-Id: Ida44cc817079d7177325e2228cf1f1d242b799e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276269
Commit-Queue: Emil Lundmark <lndmrk@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38447}
This commit is contained in:
Emil Lundmark 2022-09-21 15:20:22 +02:00 committed by WebRTC LUCI CQ
parent bbc840f608
commit 6bf20cc76a
9 changed files with 88 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
BUILD.gn
View file

@ -273,6 +273,12 @@ config("common_config") {
defines += [ "WEBRTC_ENABLE_PROTOBUF=0" ]
}
if (rtc_strict_field_trials) {
defines += [ "WEBRTC_STRICT_FIELD_TRIALS=1" ]
} else {
defines += [ "WEBRTC_STRICT_FIELD_TRIALS=0" ]
}
if (rtc_include_internal_audio_device) {
defines += [ "WEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE" ]
}

6
api/BUILD.gn
View file

@ -1308,6 +1308,7 @@ if (rtc_include_tests) {
"../rtc_base:rtc_event",
"../rtc_base:rtc_task_queue",
"../rtc_base:task_queue_for_test",
"../rtc_base/containers:flat_set",
"../rtc_base/task_utils:repeating_task",
"../system_wrappers:field_trial",
"../test:fileutils",
@ -1322,7 +1323,10 @@ if (rtc_include_tests) {
"video:rtp_video_frame_assembler_unittests",
"video:video_unittests",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
absl_deps = [
"//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/types:optional",
]
}
rtc_library("compile_all_headers") {

40
api/field_trials_unittest.cc
View file

@ -11,8 +11,11 @@
#include "api/field_trials.h"
#include <memory>
#include <utility>
#include "absl/strings/string_view.h"
#include "api/transport/field_trial_based_config.h"
#include "rtc_base/containers/flat_set.h"
#include "system_wrappers/include/field_trial.h"
#include "test/gmock.h"
#include "test/gtest.h"
@ -26,22 +29,16 @@ namespace {
using ::testing::NotNull;
using ::webrtc::field_trial::InitFieldTrialsFromString;
using ::webrtc::field_trial::ScopedGlobalFieldTrialsForTesting;
class FieldTrialsTest : public testing::Test {
protected:
FieldTrialsTest() {
// Make sure global state is consistent between test runs.
InitFieldTrialsFromString(nullptr);
}
};
TEST_F(FieldTrialsTest, EmptyStringHasNoEffect) {
TEST(FieldTrialsTest, EmptyStringHasNoEffect) {
ScopedGlobalFieldTrialsForTesting g({"MyCoolTrial"});
FieldTrials f("");
EXPECT_FALSE(f.IsEnabled("MyCoolTrial"));
EXPECT_FALSE(f.IsDisabled("MyCoolTrial"));
}
TEST_F(FieldTrialsTest, EnabledDisabledMustBeFirstInValue) {
TEST(FieldTrialsTest, EnabledDisabledMustBeFirstInValue) {
FieldTrials f(
"MyCoolTrial/EnabledFoo/"
"MyUncoolTrial/DisabledBar/"
@ -51,7 +48,8 @@ TEST_F(FieldTrialsTest, EnabledDisabledMustBeFirstInValue) {
EXPECT_FALSE(f.IsEnabled("AnotherTrial"));
}
TEST_F(FieldTrialsTest, FieldTrialsDoesNotReadGlobalString) {
TEST(FieldTrialsTest, FieldTrialsDoesNotReadGlobalString) {
ScopedGlobalFieldTrialsForTesting g({"MyCoolTrial", "MyUncoolTrial"});
static constexpr char s[] = "MyCoolTrial/Enabled/MyUncoolTrial/Disabled/";
InitFieldTrialsFromString(s);
FieldTrials f("");
@ -59,13 +57,14 @@ TEST_F(FieldTrialsTest, FieldTrialsDoesNotReadGlobalString) {
EXPECT_FALSE(f.IsDisabled("MyUncoolTrial"));
}
TEST_F(FieldTrialsTest, FieldTrialsWritesGlobalString) {
TEST(FieldTrialsTest, FieldTrialsWritesGlobalString) {
ScopedGlobalFieldTrialsForTesting g({"MyCoolTrial", "MyUncoolTrial"});
FieldTrials f("MyCoolTrial/Enabled/MyUncoolTrial/Disabled/");
EXPECT_TRUE(webrtc::field_trial::IsEnabled("MyCoolTrial"));
EXPECT_TRUE(webrtc::field_trial::IsDisabled("MyUncoolTrial"));
}
TEST_F(FieldTrialsTest, FieldTrialsRestoresGlobalStringAfterDestruction) {
TEST(FieldTrialsTest, FieldTrialsRestoresGlobalStringAfterDestruction) {
static constexpr char s[] = "SomeString/Enabled/";
InitFieldTrialsFromString(s);
{
@ -77,19 +76,20 @@ TEST_F(FieldTrialsTest, FieldTrialsRestoresGlobalStringAfterDestruction) {
}
#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(FieldTrialsTest, FieldTrialsDoesNotSupportSimultaneousInstances) {
TEST(FieldTrialsTest, FieldTrialsDoesNotSupportSimultaneousInstances) {
FieldTrials f("SomeString/Enabled/");
RTC_EXPECT_DEATH(FieldTrials("SomeOtherString/Enabled/").Lookup("Whatever"),
"Only one instance");
}
#endif // GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(FieldTrialsTest, FieldTrialsSupportsSeparateInstances) {
TEST(FieldTrialsTest, FieldTrialsSupportsSeparateInstances) {
{ FieldTrials f("SomeString/Enabled/"); }
{ FieldTrials f("SomeOtherString/Enabled/"); }
}
TEST_F(FieldTrialsTest, NonGlobalFieldTrialsInstanceDoesNotModifyGlobalString) {
TEST(FieldTrialsTest, NonGlobalFieldTrialsInstanceDoesNotModifyGlobalString) {
ScopedGlobalFieldTrialsForTesting g({"SomeString"});
std::unique_ptr<FieldTrials> f =
FieldTrials::CreateNoGlobal("SomeString/Enabled/");
ASSERT_THAT(f, NotNull());
@ -97,7 +97,7 @@ TEST_F(FieldTrialsTest, NonGlobalFieldTrialsInstanceDoesNotModifyGlobalString) {
EXPECT_FALSE(webrtc::field_trial::IsEnabled("SomeString"));
}
TEST_F(FieldTrialsTest, NonGlobalFieldTrialsSupportSimultaneousInstances) {
TEST(FieldTrialsTest, NonGlobalFieldTrialsSupportSimultaneousInstances) {
std::unique_ptr<FieldTrials> f1 =
FieldTrials::CreateNoGlobal("SomeString/Enabled/");
std::unique_ptr<FieldTrials> f2 =
@ -112,7 +112,8 @@ TEST_F(FieldTrialsTest, NonGlobalFieldTrialsSupportSimultaneousInstances) {
EXPECT_TRUE(f2->IsEnabled("SomeOtherString"));
}
TEST_F(FieldTrialsTest, GlobalAndNonGlobalFieldTrialsAreDisjoint) {
TEST(FieldTrialsTest, GlobalAndNonGlobalFieldTrialsAreDisjoint) {
ScopedGlobalFieldTrialsForTesting g({"SomeString", "SomeOtherString"});
FieldTrials f1("SomeString/Enabled/");
std::unique_ptr<FieldTrials> f2 =
FieldTrials::CreateNoGlobal("SomeOtherString/Enabled/");
@ -125,7 +126,8 @@ TEST_F(FieldTrialsTest, GlobalAndNonGlobalFieldTrialsAreDisjoint) {
EXPECT_TRUE(f2->IsEnabled("SomeOtherString"));
}
TEST_F(FieldTrialsTest, FieldTrialBasedConfigReadsGlobalString) {
TEST(FieldTrialsTest, FieldTrialBasedConfigReadsGlobalString) {
ScopedGlobalFieldTrialsForTesting g({"MyCoolTrial", "MyUncoolTrial"});
static constexpr char s[] = "MyCoolTrial/Enabled/MyUncoolTrial/Disabled/";
InitFieldTrialsFromString(s);
FieldTrialBasedConfig f;

5
rtc_base/experiments/field_trial_parser_unittest.cc
View file

@ -18,7 +18,8 @@
namespace webrtc {
namespace {
const char kDummyExperiment[] = "WebRTC-DummyExperiment";
constexpr char kDummyExperiment[] = "WebRTC-DummyExperiment";
struct DummyExperiment {
FieldTrialFlag enabled = FieldTrialFlag("Enabled");
@ -29,6 +30,8 @@ struct DummyExperiment {
FieldTrialParameter<std::string> hash =
FieldTrialParameter<std::string>("h", "a80");
field_trial::ScopedGlobalFieldTrialsForTesting g{{kDummyExperiment}};
explicit DummyExperiment(absl::string_view field_trial) {
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash},
field_trial);

7
system_wrappers/BUILD.gn
View file

@ -87,11 +87,16 @@ rtc_library("field_trial") {
defines = [ "WEBRTC_EXCLUDE_FIELD_TRIAL_DEFAULT" ]
}
deps = [
"../experiments:registered_field_trials",
"../rtc_base:checks",
"../rtc_base:logging",
"../rtc_base:stringutils",
"../rtc_base/containers:flat_set",
]
absl_deps = [
"//third_party/abseil-cpp/absl/algorithm:container",
"//third_party/abseil-cpp/absl/strings",
]
absl_deps = [ "//third_party/abseil-cpp/absl/strings" ]
}
rtc_library("metrics") {

8
system_wrappers/include/field_trial.h
View file

@ -14,6 +14,7 @@
#include <string>
#include "absl/strings/string_view.h"
#include "rtc_base/containers/flat_set.h"
// Field trials allow webrtc clients (such as Chrome) to turn on feature code
// in binaries out in the field and gather information with that.
@ -97,6 +98,13 @@ bool FieldTrialsStringIsValid(absl::string_view trials_string);
std::string MergeFieldTrialsStrings(absl::string_view first,
absl::string_view second);
// RAII type that ensures global state is consistent between tests.
class ScopedGlobalFieldTrialsForTesting {
public:
explicit ScopedGlobalFieldTrialsForTesting(flat_set<std::string> keys);
~ScopedGlobalFieldTrialsForTesting();
};
} // namespace field_trial
} // namespace webrtc

6
system_wrappers/source/DEPS Normal file
View file

@ -0,0 +1,6 @@
specific_include_rules = {
# TODO(bugs.webrtc.org/10335): Remove rule when global string is removed.
"field_trial\.cc": [
"+experiments/registered_field_trials.h",
],
}

27
system_wrappers/source/field_trial.cc
View file

@ -13,9 +13,13 @@
#include <map>
#include <string>
#include <utility>
#include "absl/algorithm/container.h"
#include "absl/strings/string_view.h"
#include "experiments/registered_field_trials.h"
#include "rtc_base/checks.h"
#include "rtc_base/containers/flat_set.h"
#include "rtc_base/logging.h"
#include "rtc_base/string_encode.h"
@ -27,7 +31,14 @@ namespace field_trial {
static const char* trials_init_string = NULL;
namespace {
constexpr char kPersistentStringSeparator = '/';
flat_set<std::string>& TestKeys() {
static auto* test_keys = new flat_set<std::string>();
return *test_keys;
}
// Validates the given field trial string.
// E.g.:
// "WebRTC-experimentFoo/Enabled/WebRTC-experimentBar/Enabled100kbps/"
@ -67,6 +78,7 @@ bool FieldTrialsStringIsValidInternal(const absl::string_view trials) {
return true;
}
} // namespace
bool FieldTrialsStringIsValid(absl::string_view trials_string) {
@ -104,6 +116,12 @@ std::string MergeFieldTrialsStrings(absl::string_view first,
#ifndef WEBRTC_EXCLUDE_FIELD_TRIAL_DEFAULT
std::string FindFullName(absl::string_view name) {
#if WEBRTC_STRICT_FIELD_TRIALS
RTC_DCHECK(absl::c_linear_search(kRegisteredFieldTrials, name) ||
TestKeys().contains(name))
<< name << " is not registered.";
#endif
if (trials_init_string == NULL)
return std::string();
@ -150,5 +168,14 @@ const char* GetFieldTrialString() {
return trials_init_string;
}
ScopedGlobalFieldTrialsForTesting::ScopedGlobalFieldTrialsForTesting(
flat_set<std::string> keys) {
TestKeys() = std::move(keys);
}
ScopedGlobalFieldTrialsForTesting::~ScopedGlobalFieldTrialsForTesting() {
TestKeys().clear();
}
} // namespace field_trial
} // namespace webrtc

5
webrtc.gni
View file

@ -231,6 +231,11 @@ declare_args() {
# Includes the dav1d decoder in the internal decoder factory when set to true.
rtc_include_dav1d_in_internal_decoder_factory = true
# When set to true, a run-time check will make sure that all field trial keys
# have been registered in accordance with the field trial policy. The check
# will only run with builds that have RTC_DCHECKs enabled.
rtc_strict_field_trials = false
}
if (!build_with_mozilla) {