Remove webrtc::test::InitFieldTrialsFromString(const std::string&).
This is done to solve a problem where a string literal is implicitly cast
to a temporary std::string when calling webrtc::test::InitFieldTrialsFromString
which passes a pointer to the internal representation to
webrtc::field_trial::InitFieldTrialFromString(char*). This pointer is
stored for later use, but the temporary std::string is destroyed as soon
as the function returns.
Using webrtc::field_trial::InitFieldTrialFromString(char*) instead,
avoids the implicit casts (but the caller still needs to ensure that
the char* outlives the program). The validation previously done by
webrtc::test::InitFieldTrialsFromString can now be done by manually
calling webrtc::test::ValidateFieldTrialsStringOrDie(const std::string&).
Add system_wrappers:field_trial_default as a direct dependency to
various targets to allow including the field_trials_default.h header.
Bug: webrtc:8812
Change-Id: Ib5a641ea255b1c16a8f7f35e1fe67f6c38a61da6
Reviewed-on: https://webrtc-review.googlesource.com/46141
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Oleh Prypin <oprypin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21856}diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index f18c0f5..f0e2fd7 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -896,6 +896,7 @@
":rtc_base",
":rtc_base_approved",
":rtc_base_tests_utils",
+ "../system_wrappers:field_trial_default",
"../test:field_trial",
"../test:test_support",
]
diff --git a/rtc_base/unittest_main.cc b/rtc_base/unittest_main.cc
index 330bd3f..aa8a11b 100644
--- a/rtc_base/unittest_main.cc
+++ b/rtc_base/unittest_main.cc
@@ -19,6 +19,7 @@
#include "rtc_base/logging.h"
#include "rtc_base/ssladapter.h"
#include "rtc_base/sslstreamadapter.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "test/field_trial.h"
#include "test/testsupport/fileutils.h"
@@ -76,7 +77,10 @@
}
webrtc::test::SetExecutablePath(argv[0]);
- webrtc::test::InitFieldTrialsFromString(FLAG_force_fieldtrials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(FLAG_force_fieldtrials);
#if defined(WEBRTC_WIN)
if (!FLAG_default_error_handlers) {
diff --git a/rtc_tools/BUILD.gn b/rtc_tools/BUILD.gn
index 976fa06..2cdf9b5 100644
--- a/rtc_tools/BUILD.gn
+++ b/rtc_tools/BUILD.gn
@@ -267,6 +267,7 @@
"../logging:rtc_event_log_parser",
"../rtc_base:protobuf_utils",
"../rtc_base:rtc_base_approved",
+ "../system_wrappers:field_trial_default",
"../test:field_trial",
"../test:test_support",
]
diff --git a/rtc_tools/event_log_visualizer/main.cc b/rtc_tools/event_log_visualizer/main.cc
index b38f1a4..4a6ec64 100644
--- a/rtc_tools/event_log_visualizer/main.cc
+++ b/rtc_tools/event_log_visualizer/main.cc
@@ -15,6 +15,7 @@
#include "rtc_tools/event_log_visualizer/analyzer.h"
#include "rtc_tools/event_log_visualizer/plot_base.h"
#include "rtc_tools/event_log_visualizer/plot_python.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "test/field_trial.h"
#include "test/testsupport/fileutils.h"
@@ -195,7 +196,10 @@
}
webrtc::test::SetExecutablePath(argv[0]);
- webrtc::test::InitFieldTrialsFromString(FLAG_force_fieldtrials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(FLAG_force_fieldtrials);
std::string filename = argv[1];
diff --git a/test/BUILD.gn b/test/BUILD.gn
index bfc521d..72bc203 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -227,6 +227,7 @@
":field_trial",
":perf_test",
"../rtc_base:rtc_base_approved",
+ "../system_wrappers:field_trial_default",
"../system_wrappers:metrics_default",
"../system_wrappers:runtime_enabled_features_default",
"//testing/gtest",
diff --git a/test/field_trial.cc b/test/field_trial.cc
index 00e5b69..9be36c5 100644
--- a/test/field_trial.cc
+++ b/test/field_trial.cc
@@ -26,9 +26,8 @@
} // namespace
namespace test {
-// Note: this code is copied from src/base/metrics/field_trial.cc since the aim
-// is to mimic chromium --force-fieldtrials.
-void InitFieldTrialsFromString(const std::string& trials_string) {
+
+void ValidateFieldTrialsStringOrDie(const std::string& trials_string) {
static const char kPersistentStringSeparator = '/';
// Catch an error if this is called more than once.
@@ -63,7 +62,7 @@
// Successfully parsed all field trials from the string.
if (next_item == trials_string.length()) {
- webrtc::field_trial::InitFieldTrialsFromString(trials_string.c_str());
+ // webrtc::field_trial::InitFieldTrialsFromString(trials_string.c_str());
return;
}
}
@@ -79,7 +78,8 @@
assert(field_trials_initiated_);
field_trials_initiated_ = false;
current_field_trials_ = config;
- InitFieldTrialsFromString(current_field_trials_);
+ ValidateFieldTrialsStringOrDie(current_field_trials_);
+ webrtc::field_trial::InitFieldTrialsFromString(current_field_trials_.c_str());
}
ScopedFieldTrials::~ScopedFieldTrials() {
diff --git a/test/field_trial.h b/test/field_trial.h
index 99ace4e..40f7b1b 100644
--- a/test/field_trial.h
+++ b/test/field_trial.h
@@ -30,7 +30,7 @@
//
// Note: This method crashes with an error message if an invalid config is
// passed to it. That can be used to find out if a binary is parsing the flags.
-void InitFieldTrialsFromString(const std::string& config);
+void ValidateFieldTrialsStringOrDie(const std::string& config);
// This class is used to override field-trial configs within specific tests.
// After this class goes out of scope previous field trials will be restored.
diff --git a/test/test_main.cc b/test/test_main.cc
index f86dd1f..962c677 100644
--- a/test/test_main.cc
+++ b/test/test_main.cc
@@ -10,6 +10,7 @@
#include "rtc_base/flags.h"
#include "rtc_base/logging.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "system_wrappers/include/metrics_default.h"
#include "test/field_trial.h"
#include "test/gmock.h"
@@ -70,8 +71,10 @@
}
webrtc::test::SetExecutablePath(argv[0]);
- std::string fieldtrials = FLAG_force_fieldtrials;
- webrtc::test::InitFieldTrialsFromString(fieldtrials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(FLAG_force_fieldtrials);
webrtc::metrics::Enable();
diff --git a/video/BUILD.gn b/video/BUILD.gn
index 593e2b9..009b889 100644
--- a/video/BUILD.gn
+++ b/video/BUILD.gn
@@ -168,6 +168,7 @@
deps = [
":video_quality_test",
"../rtc_base:rtc_base_approved",
+ "../system_wrappers:field_trial_default",
"../system_wrappers:metrics_default",
"../system_wrappers:runtime_enabled_features_default",
"../test:field_trial",
@@ -193,6 +194,7 @@
deps = [
":video_quality_test",
"../rtc_base:rtc_base_approved",
+ "../system_wrappers:field_trial_default",
"../system_wrappers:metrics_default",
"../system_wrappers:runtime_enabled_features_default",
"../test:field_trial",
@@ -217,6 +219,7 @@
deps = [
":video_quality_test",
"../rtc_base:rtc_base_approved",
+ "../system_wrappers:field_trial_default",
"../system_wrappers:metrics_default",
"../system_wrappers:runtime_enabled_features_default",
"../test:field_trial",
diff --git a/video/screenshare_loopback.cc b/video/screenshare_loopback.cc
index 8a0b247..3157537 100644
--- a/video/screenshare_loopback.cc
+++ b/video/screenshare_loopback.cc
@@ -12,6 +12,7 @@
#include "rtc_base/flags.h"
#include "rtc_base/stringencode.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/run_test.h"
@@ -330,10 +331,12 @@
return 0;
}
- // InitFieldTrialsFromString needs a reference to an std::string instance,
- // with a scope that outlives the test.
- std::string field_trials = webrtc::flags::FLAG_force_fieldtrials;
- webrtc::test::InitFieldTrialsFromString(field_trials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(
+ webrtc::flags::FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(
+ webrtc::flags::FLAG_force_fieldtrials);
webrtc::test::RunTest(webrtc::Loopback);
return 0;
diff --git a/video/sv_loopback.cc b/video/sv_loopback.cc
index 24d3bed..c8e0d74 100644
--- a/video/sv_loopback.cc
+++ b/video/sv_loopback.cc
@@ -12,6 +12,7 @@
#include "rtc_base/flags.h"
#include "rtc_base/stringencode.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/run_test.h"
@@ -568,10 +569,12 @@
return 0;
}
- // InitFieldTrialsFromString needs a reference to an std::string instance,
- // with a scope that outlives the test.
- std::string field_trials = webrtc::flags::FLAG_force_fieldtrials;
- webrtc::test::InitFieldTrialsFromString(field_trials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(
+ webrtc::flags::FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(
+ webrtc::flags::FLAG_force_fieldtrials);
webrtc::test::RunTest(webrtc::Loopback);
return 0;
diff --git a/video/video_loopback.cc b/video/video_loopback.cc
index 37793cc..d195ceb 100644
--- a/video/video_loopback.cc
+++ b/video/video_loopback.cc
@@ -11,6 +11,7 @@
#include <stdio.h>
#include "rtc_base/flags.h"
+#include "system_wrappers/include/field_trial_default.h"
#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/run_test.h"
@@ -328,10 +329,12 @@
return 0;
}
- // InitFieldTrialsFromString needs a reference to an std::string instance,
- // with a scope that outlives the test.
- std::string field_trials = webrtc::flags::FLAG_force_fieldtrials;
- webrtc::test::InitFieldTrialsFromString(field_trials);
+ webrtc::test::ValidateFieldTrialsStringOrDie(
+ webrtc::flags::FLAG_force_fieldtrials);
+ // InitFieldTrialsFromString stores the char*, so the char array must outlive
+ // the application.
+ webrtc::field_trial::InitFieldTrialsFromString(
+ webrtc::flags::FLAG_force_fieldtrials);
webrtc::test::RunTest(webrtc::Loopback);
return 0;