Index: base/metrics/field_trial.cc |
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc |
index 78602f1e613d39f00020dea6f1f18f093bcfa72f..b4196d8c1f55acf674588d57692020f534f0a934 100644 |
--- a/base/metrics/field_trial.cc |
+++ b/base/metrics/field_trial.cc |
@@ -208,25 +208,6 @@ void AddFeatureAndFieldTrialFlags(const char* enable_features_switch, |
} |
} |
-#if defined(OS_WIN) |
-HANDLE CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) { |
- HANDLE src = allocator->shared_memory()->handle().GetHandle(); |
- ProcessHandle process = GetCurrentProcess(); |
- DWORD access = SECTION_MAP_READ | SECTION_QUERY; |
- HANDLE dst; |
- if (!::DuplicateHandle(process, src, process, &dst, access, true, 0)) |
- return kInvalidPlatformFile; |
- return dst; |
-} |
-#endif |
- |
-#if defined(OS_POSIX) && !defined(OS_NACL) |
-int CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) { |
- SharedMemoryHandle handle = allocator->shared_memory()->GetReadOnlyHandle(); |
- return SharedMemory::GetFdFromSharedMemoryHandle(handle); |
-} |
-#endif |
- |
void OnOutOfMemory(size_t size) { |
#if defined(OS_NACL) |
NOTREACHED(); |
@@ -775,15 +756,15 @@ bool FieldTrialList::CreateTrialsFromString( |
// static |
void FieldTrialList::CreateTrialsFromCommandLine( |
const CommandLine& cmd_line, |
- const char* field_trial_handle_switch, |
+ const char* field_trial_switch_value, |
int fd_key) { |
global_->create_trials_from_command_line_called_ = true; |
#if defined(OS_WIN) |
- if (cmd_line.HasSwitch(field_trial_handle_switch)) { |
- std::string handle_switch = |
- cmd_line.GetSwitchValueASCII(field_trial_handle_switch); |
- bool result = CreateTrialsFromHandleSwitch(handle_switch); |
+ if (cmd_line.HasSwitch(field_trial_switch_value)) { |
+ std::string switch_value = |
+ cmd_line.GetSwitchValueASCII(field_trial_switch_value); |
+ bool result = CreateTrialsFromSwitchValue(switch_value); |
DCHECK(result); |
} |
#endif |
@@ -792,8 +773,10 @@ void FieldTrialList::CreateTrialsFromCommandLine( |
// On POSIX, we check if the handle is valid by seeing if the browser process |
// sent over the switch (we don't care about the value). Invalid handles |
// occur in some browser tests which don't initialize the allocator. |
- if (cmd_line.HasSwitch(field_trial_handle_switch)) { |
- bool result = CreateTrialsFromDescriptor(fd_key); |
+ if (cmd_line.HasSwitch(field_trial_switch_value)) { |
+ std::string switch_value = |
+ cmd_line.GetSwitchValueASCII(field_trial_switch_value); |
+ bool result = CreateTrialsFromDescriptor(fd_key, switch_value); |
DCHECK(result); |
} |
#endif |
@@ -832,27 +815,27 @@ void FieldTrialList::AppendFieldTrialHandleIfNeeded( |
return; |
if (kUseSharedMemoryForFieldTrials) { |
InstantiateFieldTrialAllocatorIfNeeded(); |
- if (global_->readonly_allocator_handle_) |
- handles->push_back(global_->readonly_allocator_handle_); |
+ if (global_->readonly_allocator_handle_.IsValid()) |
+ handles->push_back(global_->readonly_allocator_handle_.GetHandle()); |
} |
} |
#endif |
#if defined(OS_POSIX) && !defined(OS_NACL) |
// static |
-int FieldTrialList::GetFieldTrialHandle() { |
+SharedMemoryHandle FieldTrialList::GetFieldTrialHandle() { |
if (global_ && kUseSharedMemoryForFieldTrials) { |
InstantiateFieldTrialAllocatorIfNeeded(); |
// We check for an invalid handle where this gets called. |
return global_->readonly_allocator_handle_; |
} |
- return kInvalidPlatformFile; |
+ return SharedMemoryHandle(); |
} |
#endif |
// static |
void FieldTrialList::CopyFieldTrialStateToFlags( |
- const char* field_trial_handle_switch, |
+ const char* field_trial_switch_value, |
Alexei Svitkine (slow)
2017/05/05 18:02:16
It's not the switch value - it's the switch name..
erikchen
2017/05/05 19:41:51
sed error.
|
const char* enable_features_switch, |
const char* disable_features_switch, |
CommandLine* cmd_line) { |
@@ -873,37 +856,16 @@ void FieldTrialList::CopyFieldTrialStateToFlags( |
InstantiateFieldTrialAllocatorIfNeeded(); |
// If the readonly handle didn't get duplicated properly, then fallback to |
// original behavior. |
- if (global_->readonly_allocator_handle_ == kInvalidPlatformFile) { |
+ if (!global_->readonly_allocator_handle_.IsValid()) { |
AddFeatureAndFieldTrialFlags(enable_features_switch, |
disable_features_switch, cmd_line); |
return; |
} |
global_->field_trial_allocator_->UpdateTrackingHistograms(); |
- |
-#if defined(OS_WIN) |
- // We need to pass a named anonymous handle to shared memory over the |
- // command line on Windows, since the child doesn't know which of the |
- // handles it inherited it should open. |
- // PlatformFile is typedef'd to HANDLE which is typedef'd to void *. We |
- // basically cast the handle into an int (uintptr_t, to be exact), stringify |
- // the int, and pass it as a command-line flag. The child process will do |
- // the reverse conversions to retrieve the handle. See |
- // http://stackoverflow.com/a/153077 |
- auto uintptr_handle = |
- reinterpret_cast<uintptr_t>(global_->readonly_allocator_handle_); |
- std::string field_trial_handle = std::to_string(uintptr_handle); |
- cmd_line->AppendSwitchASCII(field_trial_handle_switch, field_trial_handle); |
-#elif defined(OS_POSIX) |
- // On POSIX, we dup the fd into a fixed fd kFieldTrialDescriptor, so we |
- // don't have to pass over the handle (it's not even the right handle |
- // anyways). But some browser tests don't create the allocator, so we need |
- // to be able to distinguish valid and invalid handles. We do that by just |
- // checking that the flag is set with a dummy value. |
- cmd_line->AppendSwitchASCII(field_trial_handle_switch, "1"); |
-#else |
-#error Unsupported OS |
-#endif |
+ std::string switch_value = SerializeSharedMemoryHandleMetadata( |
+ global_->readonly_allocator_handle_); |
+ cmd_line->AppendSwitchASCII(field_trial_switch_value, switch_value); |
return; |
} |
@@ -1132,22 +1094,83 @@ FieldTrialList::GetAllFieldTrialsFromPersistentAllocator( |
return entries; |
} |
+// static |
+std::string FieldTrialList::SerializeSharedMemoryHandleMetadata( |
+ const SharedMemoryHandle& shm) { |
+ std::stringstream ss; |
+#if defined(OS_WIN) |
+ // Tell the child process the name of the inherited HANDLE. |
+ uintptr_t uintptr_handle = reinterpret_cast<uintptr_t>(shm.GetHandle()); |
+ ss << uintptr_handle << ","; |
+#elif !defined(OS_POSIX) |
+#error Unsupported OS |
+#endif |
+ |
+ base::UnguessableToken guid = shm.GetGUID(); |
+ ss << guid.GetHighForSerialization() << "," << guid.GetLowForSerialization(); |
+ return ss.str(); |
+} |
+ |
#if defined(OS_WIN) |
// static |
-bool FieldTrialList::CreateTrialsFromHandleSwitch( |
- const std::string& handle_switch) { |
- int field_trial_handle = std::stoi(handle_switch); |
+SharedMemoryHandle FieldTrialList::DeserializeSharedMemoryHandleMetadata( |
+ const std::string& switch_value) { |
+ std::istringstream iss(switch_value); |
+ std::string token; |
+ |
+ std::getline(iss, token, ','); |
Avi (use Gerrit)
2017/05/05 04:30:12
You use base::StringToInt, why not base::SplitStri
Alexei Svitkine (slow)
2017/05/05 18:02:16
+1 - let's use SplitString()
erikchen
2017/05/05 19:41:51
Done.
|
+ int field_trial_handle = 0; |
+ if (!base::StringToInt(token, &field_trial_handle)) |
+ return SharedMemoryHandle(); |
HANDLE handle = reinterpret_cast<HANDLE>(field_trial_handle); |
- // TODO(erikchen): Plumb a GUID for this SharedMemoryHandle. |
- // https://crbug.com/713763. |
- SharedMemoryHandle shm_handle(handle, base::UnguessableToken::Create()); |
- return FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm_handle); |
+ |
+ std::getline(iss, token, ','); |
+ uint64_t high = 0; |
+ if (!base::StringToUint64(token, &high)) |
+ return SharedMemoryHandle(); |
+ std::getline(iss, token, ','); |
+ uint64_t low = 0; |
+ if (!base::StringToUint64(token, &low)) |
+ return SharedMemoryHandle(); |
+ |
+ base::UnguessableToken guid = base::UnguessableToken::Deserialize(high, low); |
+ return SharedMemoryHandle(handle, guid); |
+} |
+ |
+// static |
+bool FieldTrialList::CreateTrialsFromSwitchValue( |
+ const std::string& switch_value) { |
+ SharedMemoryHandle shm = DeserializeSharedMemoryHandleMetadata(switch_value); |
+ if (!shm.IsValid()) |
+ return false; |
+ return FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm); |
} |
#endif |
#if defined(OS_POSIX) && !defined(OS_NACL) |
// static |
-bool FieldTrialList::CreateTrialsFromDescriptor(int fd_key) { |
+SharedMemoryHandle FieldTrialList::DeserializeSharedMemoryHandleMetadata( |
+ int fd, |
+ const std::string& switch_value) { |
Alexei Svitkine (slow)
2017/05/05 18:02:16
How about unifying the code for the windows and po
erikchen
2017/05/05 19:41:51
I thought about this - there needs to be another i
|
+ std::istringstream iss(switch_value); |
+ std::string token; |
+ std::getline(iss, token, ','); |
+ uint64_t high = 0; |
+ if (!base::StringToUint64(token, &high)) |
+ return SharedMemoryHandle(); |
+ std::getline(iss, token, ','); |
+ uint64_t low = 0; |
+ if (!base::StringToUint64(token, &low)) |
+ return SharedMemoryHandle(); |
+ |
+ base::UnguessableToken guid = base::UnguessableToken::Deserialize(high, low); |
+ return SharedMemoryHandle(FileDescriptor(fd, true), guid); |
+} |
+ |
+// static |
+bool FieldTrialList::CreateTrialsFromDescriptor( |
+ int fd_key, |
+ const std::string& switch_value) { |
if (!kUseSharedMemoryForFieldTrials) |
return false; |
@@ -1158,12 +1181,12 @@ bool FieldTrialList::CreateTrialsFromDescriptor(int fd_key) { |
if (fd == -1) |
return false; |
- // TODO(erikchen): Plumb a GUID for this SharedMemoryHandle. |
- // https://crbug.com/713763. |
- SharedMemoryHandle shm_handle(FileDescriptor(fd, true), |
- base::UnguessableToken::Create()); |
+ SharedMemoryHandle shm = |
+ DeserializeSharedMemoryHandleMetadata(fd, switch_value); |
+ if (!shm.IsValid()) |
+ return false; |
- bool result = FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm_handle); |
+ bool result = FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm); |
DCHECK(result); |
return true; |
} |
@@ -1254,7 +1277,7 @@ void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() { |
// Set |readonly_allocator_handle_| so we can pass it to be inherited and |
// via the command line. |
global_->readonly_allocator_handle_ = |
- CreateReadOnlyHandle(global_->field_trial_allocator_.get()); |
+ global_->field_trial_allocator_->shared_memory()->GetReadOnlyHandle(); |
#endif |
} |