Skip to content

GitLab

T
tangled
Commit 4428eafa authored by Alexei Svitkine's avatar Alexei Svitkine Committed by Commit Bot
Browse files
Options

[Cleanup] Remove kUseSharedMemoryForFieldTrials in field_trial.c. 

This constant was only set to false on OS_FUCHSIA and has been
shipping as true for several years on all other platforms.

Cleans up the code to not need this constant in most places and
adds a OS_FUCHSIA ifdef around InstantiateFieldTrialAllocatorIfNeeded()
which is the only place that needed custom logic to disable it for
Fuchsia.

Bug: 867558
Change-Id: Idbf49ce571a50d67debeafe2cdacee086ebca537
Reviewed-on: https://chromium-review.googlesource.com/c/1343020
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609922}
parent c5d65c1e
Show whitespace changes
Inline Side-by-side
Showing with 36 additions and 72 deletions
base/metrics/field_trial.cc
View file @ 4428eafa
......@@ -45,20 +45,6 @@ const char kPersistentStringSeparator = '/'; // Currently a slash.
// command line which forces its activation.
const char kActivationMarker = '*';
// Use shared memory to communicate field trial (experiment) state. Set to false
// for now while the implementation is fleshed out (e.g. data format, single
// shared memory segment). See https://codereview.chromium.org/2365273004/ and
// crbug.com/653874
// The browser is the only process that has write access to the shared memory.
// This is safe from race conditions because MakeIterable is a release operation
// and GetNextOfType is an acquire operation, so memory writes before
// MakeIterable happen before memory reads after GetNextOfType.
#if defined(OS_FUCHSIA) // TODO(752368): Not yet supported on Fuchsia.
const bool kUseSharedMemoryForFieldTrials = false;
#else
const bool kUseSharedMemoryForFieldTrials = true;
#endif
// Constants for the field trial allocator.
const char kAllocatorName[] = "FieldTrialAllocator";
......@@ -242,10 +228,10 @@ SharedMemoryHandle GetSharedMemoryReadOnlyHandle(SharedMemory* shared_memory) {
// writable mapping attempts, but the original one in |shm| survives
// and is still usable in the current process.
result.SetRegionReadOnly();
#endif // OS_ANDROID
#endif // defined(OS_ANDROID)
return result;
}
#endif // !OS_NACL
#endif // !defined(OS_NACL)
} // namespace
......@@ -459,7 +445,7 @@ void FieldTrial::FinalizeGroupChoiceImpl(bool is_locked) {
SetGroupChoice(default_group_name_, kDefaultGroupNumber);
// Add the field trial to shared memory.
if (kUseSharedMemoryForFieldTrials && trial_registered_)
if (trial_registered_)
FieldTrialList::OnGroupFinalized(is_locked, this);
}
......@@ -872,8 +858,7 @@ void FieldTrialList::CreateFeaturesFromCommandLine(
const char* disable_features_switch,
FeatureList* feature_list) {
// Fallback to command line if not using shared memory.
if (!kUseSharedMemoryForFieldTrials ||
!global_->field_trial_allocator_.get()) {
if (!global_->field_trial_allocator_.get()) {
return feature_list->InitializeFromCommandLine(
command_line.GetSwitchValueASCII(enable_features_switch),
command_line.GetSwitchValueASCII(disable_features_switch));
......@@ -889,18 +874,16 @@ void FieldTrialList::AppendFieldTrialHandleIfNeeded(
HandlesToInheritVector* handles) {
if (!global_)
return;
if (kUseSharedMemoryForFieldTrials) {
InstantiateFieldTrialAllocatorIfNeeded();
if (global_->readonly_allocator_handle_.IsValid())
handles->push_back(global_->readonly_allocator_handle_.GetHandle());
}
}
#elif defined(OS_FUCHSIA)
// TODO(fuchsia): Implement shared-memory configuration (crbug.com/752368).
#elif defined(OS_MACOSX) && !defined(OS_IOS)
// static
FieldTrialMemoryServer* FieldTrialList::GetFieldTrialMemoryServer() {
if (global_ && kUseSharedMemoryForFieldTrials) {
if (global_) {
InstantiateFieldTrialAllocatorIfNeeded();
return global_->field_trial_server_.get();
}
......@@ -909,7 +892,7 @@ FieldTrialMemoryServer* FieldTrialList::GetFieldTrialMemoryServer() {
#elif defined(OS_POSIX) && !defined(OS_NACL)
// static
SharedMemoryHandle FieldTrialList::GetFieldTrialHandle() {
if (global_ && kUseSharedMemoryForFieldTrials) {
if (global_) {
InstantiateFieldTrialAllocatorIfNeeded();
// We check for an invalid handle where this gets called.
return global_->readonly_allocator_handle_;
......@@ -924,32 +907,22 @@ void FieldTrialList::CopyFieldTrialStateToFlags(
const char* enable_features_switch,
const char* disable_features_switch,
CommandLine* cmd_line) {
// TODO(lawrencewu): Ideally, having the global would be guaranteed. However,
// content browser tests currently don't create a FieldTrialList because they
// don't run ChromeBrowserMainParts code where it's done for Chrome.
// Some tests depend on the enable and disable features flag switch, though,
// so we can still add those even though AllStatesToString() will be a no-op.
if (!global_) {
AddFeatureAndFieldTrialFlags(enable_features_switch,
disable_features_switch, cmd_line);
return;
}
// Use shared memory to pass the state if the feature is enabled, otherwise
// fallback to passing it via the command line as a string.
if (kUseSharedMemoryForFieldTrials) {
#if !defined(OS_FUCHSIA) // TODO(752368): Not yet supported on Fuchsia.
// Use shared memory to communicate field trial state to child processes.
// The browser is the only process that has write access to the shared memory.
InstantiateFieldTrialAllocatorIfNeeded();
// If the readonly handle didn't get duplicated properly, then fallback to
// original behavior.
if (!global_->readonly_allocator_handle_.IsValid()) {
#endif // !defined(OS_FUCHSIA)
// If the readonly handle did not get created, fall back to flags.
if (!global_ || !global_->readonly_allocator_handle_.IsValid()) {
AddFeatureAndFieldTrialFlags(enable_features_switch,
disable_features_switch, cmd_line);
return;
}
global_->field_trial_allocator_->UpdateTrackingHistograms();
std::string switch_value = SerializeSharedMemoryHandleMetadata(
global_->readonly_allocator_handle_);
std::string switch_value =
SerializeSharedMemoryHandleMetadata(global_->readonly_allocator_handle_);
cmd_line->AppendSwitchASCII(field_trial_handle_switch, switch_value);
// Append --enable-features and --disable-features switches corresponding
......@@ -965,12 +938,6 @@ void FieldTrialList::CopyFieldTrialStateToFlags(
cmd_line->AppendSwitchASCII(enable_features_switch, enabled_features);
if (!disabled_features.empty())
cmd_line->AppendSwitchASCII(disable_features_switch, disabled_features);
return;
}
AddFeatureAndFieldTrialFlags(enable_features_switch, disable_features_switch,
cmd_line);
}
// static
......@@ -1054,7 +1021,6 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
if (!field_trial->enable_field_trial_)
return;
if (kUseSharedMemoryForFieldTrials)
ActivateFieldTrialEntryWhileLocked(field_trial);
}
......@@ -1274,7 +1240,7 @@ SharedMemoryHandle FieldTrialList::DeserializeSharedMemoryHandleMetadata(
FieldTrialMemoryClient::AcquireMemoryObject();
if (scoped_handle == MACH_PORT_NULL)
return SharedMemoryHandle();
#endif // defined(OS_WIN)
#endif
base::UnguessableToken guid;
if (!DeserializeGUIDFromStringPieces(tokens[1], tokens[2], &guid))
......@@ -1333,9 +1299,6 @@ bool FieldTrialList::CreateTrialsFromSwitchValue(
bool FieldTrialList::CreateTrialsFromDescriptor(
int fd_key,
const std::string& switch_value) {
if (!kUseSharedMemoryForFieldTrials)
return false;
if (fd_key == -1)
return false;
......@@ -1402,6 +1365,7 @@ bool FieldTrialList::CreateTrialsFromSharedMemory(
void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
if (!global_)
return;
AutoLock auto_lock(global_->lock_);
// Create the allocator if not already created and add all existing trials.
if (global_->field_trial_allocator_ != nullptr)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment