From 97ae8295dfda25dc394112cb84741df9949a65f9 Mon Sep 17 00:00:00 2001 From: Francois Doray Date: Tue, 23 Feb 2016 18:50:30 -0500 Subject: Use a valid /prefetch argument when launching a process. The /prefetch:# argument that is currently added to the command line of processes launched with content::StartSandboxedProcess is ignored by Windows because # doesn't respect the [1, 8] constraint. This CL ensures that a valid /prefetch:# argument is used for every process launch (except when the feature is disabled by the PreRead field trial). BUG=577698 Review URL: https://codereview.chromium.org/1612663002 Cr-Commit-Position: refs/heads/master@{#373241} (cherry picked from commit 343068c41c299e65a87d935654f0b81a9935250f) Conflicts: chrome/service/DEPS TBR=gab@chromium.org Review URL: https://codereview.chromium.org/1727983002 . Cr-Commit-Position: refs/branch-heads/2623@{#484} Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907} --- components/crash.gypi | 7 ++++ components/crash/content/app/BUILD.gn | 7 ++++ components/crash/content/app/DEPS | 5 +++ components/crash/content/app/crashpad_win.cc | 8 +++++ components/nacl.gyp | 7 ++++ components/nacl/browser/BUILD.gn | 8 +++++ components/nacl/browser/DEPS | 3 ++ components/nacl/browser/nacl_process_host.cc | 8 ++++- .../common/pre_read_field_trial_utils_win.cc | 40 ++++++++++------------ .../common/pre_read_field_trial_utils_win.h | 7 ++-- 10 files changed, 75 insertions(+), 25 deletions(-) (limited to 'components') diff --git a/components/crash.gypi b/components/crash.gypi index 00ee69c..8ef86ad 100644 --- a/components/crash.gypi +++ b/components/crash.gypi @@ -217,6 +217,13 @@ ], 'defines': ['CRASH_IMPLEMENTATION'], 'conditions': [ + ['OS=="win"', { + 'dependencies': [ + # TODO(fdoray): Remove this once the PreRead field trial has + # expired. crbug.com/577698 + '<(DEPTH)/components/components.gyp:startup_metric_utils_common', + ], + }], ['OS=="mac" or OS=="win"', { 'dependencies': [ '../third_party/crashpad/crashpad/client/client.gyp:crashpad_client', diff --git a/components/crash/content/app/BUILD.gn b/components/crash/content/app/BUILD.gn index 52140ce..3def677 100644 --- a/components/crash/content/app/BUILD.gn +++ b/components/crash/content/app/BUILD.gn @@ -49,6 +49,13 @@ source_set("app") { ] deps += [ ":lib" ] + if (is_win) { + deps += [ + # TODO(fdoray): Remove this once the PreRead field trial has expired. + # crbug.com/577698 + "//components/startup_metric_utils/common", + ] + } if (is_mac || is_win) { deps += [ "//third_party/crashpad/crashpad/client" ] } diff --git a/components/crash/content/app/DEPS b/components/crash/content/app/DEPS index 80bc21a..01c710b 100644 --- a/components/crash/content/app/DEPS +++ b/components/crash/content/app/DEPS @@ -3,6 +3,11 @@ include_rules = [ "+content/public/common/content_descriptors.h", "+content/public/common/result_codes.h", + + # TODO(fdoray): Remove this once the PreRead field trial has expired. + # crbug.com/577698 + "+components/startup_metric_utils/common/pre_read_field_trial_utils_win.h", + "+third_party/crashpad", "+third_party/kasko", "+third_party/lss/linux_syscall_support.h", diff --git a/components/crash/content/app/crashpad_win.cc b/components/crash/content/app/crashpad_win.cc index 1c241ca..95e53e2 100644 --- a/components/crash/content/app/crashpad_win.cc +++ b/components/crash/content/app/crashpad_win.cc @@ -16,6 +16,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "components/crash/content/app/crash_reporter_client.h" +#include "components/startup_metric_utils/common/pre_read_field_trial_utils_win.h" #include "third_party/crashpad/crashpad/client/crashpad_client.h" #include "third_party/crashpad/crashpad/client/crashpad_info.h" #include "third_party/crashpad/crashpad/client/simulate_crash_win.h" @@ -75,6 +76,13 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client, exe_file = exe_dir.Append(FILE_PATH_LITERAL("crashpad_handler.exe")); } else { arguments.push_back("--type=crashpad-handler"); + + if (startup_metric_utils::GetPreReadOptions().use_prefetch_argument) { + // The prefetch argument added here has to be documented in + // chrome_switches.cc, below the kPrefetchArgument* constants. A + // constant can't be used here because crashpad can't depend on Chrome. + arguments.push_back("/prefetch:7"); + } } // TODO(scottmg): See https://crashpad.chromium.org/bug/23. diff --git a/components/nacl.gyp b/components/nacl.gyp index a9ed69c..2fd7aba 100644 --- a/components/nacl.gyp +++ b/components/nacl.gyp @@ -139,6 +139,13 @@ '../sandbox/sandbox.gyp:sandbox_services', ] }], + ['OS=="win"', { + 'dependencies': [ + # TODO(fdoray): Remove this once the PreRead field trial has + # expired. crbug.com/577698 + '../components/components.gyp:startup_metric_utils_common', + ] + }], ], }, { diff --git a/components/nacl/browser/BUILD.gn b/components/nacl/browser/BUILD.gn index 63c2c9d..9b247f4 100644 --- a/components/nacl/browser/BUILD.gn +++ b/components/nacl/browser/BUILD.gn @@ -63,6 +63,14 @@ source_set("browser") { data_deps += [ "//components/nacl/loader:helper_nonsfi" ] } + if (is_win) { + deps += [ + # TODO(fdoray): Remove this once the PreRead field trial has expired. + # crbug.com/577698 + "//components/startup_metric_utils/common", + ] + } + if (is_win && current_cpu == "x86") { data_deps += [ "//components/nacl/broker:nacl64" ] } diff --git a/components/nacl/browser/DEPS b/components/nacl/browser/DEPS index efbdb131..2e1a96b 100644 --- a/components/nacl/browser/DEPS +++ b/components/nacl/browser/DEPS @@ -1,4 +1,7 @@ include_rules = [ + # TODO(fdoray): Remove this once the PreRead field trial has expired. + # crbug.com/577698 + "+components/startup_metric_utils/common", "+components/url_formatter", "+content/public/browser", "+content/public/test", diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc index aa4f4c5..2ade0f7 100644 --- a/components/nacl/browser/nacl_process_host.cc +++ b/components/nacl/browser/nacl_process_host.cc @@ -76,6 +76,7 @@ #include "base/win/scoped_handle.h" #include "components/nacl/browser/nacl_broker_service_win.h" #include "components/nacl/common/nacl_debug_exception_handler_win.h" +#include "components/startup_metric_utils/common/pre_read_field_trial_utils_win.h" #include "content/public/common/sandbox_init.h" #endif @@ -625,7 +626,12 @@ bool NaClProcessHost::LaunchSelLdr() { if (NaClBrowser::GetDelegate()->DialogsAreSuppressed()) cmd_line->AppendSwitch(switches::kNoErrorDialogs); - // On Windows we might need to start the broker process to launch a new loader +#if defined(OS_WIN) + if (startup_metric_utils::GetPreReadOptions().use_prefetch_argument) + cmd_line->AppendArg(switches::kPrefetchArgumentOther); +#endif // defined(OS_WIN) + +// On Windows we might need to start the broker process to launch a new loader #if defined(OS_WIN) if (RunningOnWOW64()) { if (!NaClBrokerService::GetInstance()->LaunchLoader( diff --git a/components/startup_metric_utils/common/pre_read_field_trial_utils_win.cc b/components/startup_metric_utils/common/pre_read_field_trial_utils_win.cc index a75a81c..dbb32bf 100644 --- a/components/startup_metric_utils/common/pre_read_field_trial_utils_win.cc +++ b/components/startup_metric_utils/common/pre_read_field_trial_utils_win.cc @@ -25,12 +25,16 @@ const char kPreReadFieldTrialName[] = "PreRead"; // annotated with the pre-read group that is actually used during this startup. const char kPreReadSyntheticFieldTrialName[] = "SyntheticPreRead"; -// Variation names for the PreRead field trial. +// Variation names for the PreRead field trial. All variations change the +// default behavior, i.e. the default is the inverse of the variation. Thus, +// variations that cancel something that is done by default have a negative +// name. const base::char16 kNoPreReadVariationName[] = L"NoPreRead"; const base::char16 kHighPriorityVariationName[] = L"HighPriority"; const base::char16 kOnlyIfColdVariationName[] = L"OnlyIfCold"; const base::char16 kPrefetchVirtualMemoryVariationName[] = L"PrefetchVirtualMemory"; +const base::char16 kNoPrefetchArgumentVariationName[] = L"NoPrefetchArgument"; // Registry key in which the PreRead field trial group is stored. const base::char16 kPreReadFieldTrialRegistryKey[] = L"\\PreReadFieldTrial"; @@ -45,6 +49,12 @@ base::string16 GetPreReadRegistryPath( return product_registry_path + kPreReadFieldTrialRegistryKey; } +// Returns true if |key| has a value named |name| which is not zero. +bool ReadBool(const base::win::RegKey& key, const base::char16* name) { + DWORD value = 0; + return key.ReadValueDW(name, &value) == ERROR_SUCCESS && value != 0; +} + } // namespace void InitializePreReadOptions(const base::string16& product_registry_path) { @@ -57,27 +67,13 @@ void InitializePreReadOptions(const base::string16& product_registry_path) { KEY_QUERY_VALUE); // Set the PreRead field trial's options. - struct { - const base::char16* name; - bool* option; - } const variations_mappings[] = { - {kNoPreReadVariationName, &g_pre_read_options.no_pre_read}, - {kHighPriorityVariationName, &g_pre_read_options.high_priority}, - {kOnlyIfColdVariationName, &g_pre_read_options.only_if_cold}, - {kPrefetchVirtualMemoryVariationName, - &g_pre_read_options.prefetch_virtual_memory}, - }; - - for (const auto& mapping : variations_mappings) { - // Set the option variable to true if the corresponding value is found in - // the registry. Set to false otherwise (default behavior). - DWORD value = 0; - if (key.ReadValueDW(mapping.name, &value) == ERROR_SUCCESS) { - DCHECK_EQ(1U, value); - DCHECK(!*mapping.option); - *mapping.option = true; - } - } + g_pre_read_options.pre_read = !ReadBool(key, kNoPreReadVariationName); + g_pre_read_options.high_priority = ReadBool(key, kHighPriorityVariationName); + g_pre_read_options.only_if_cold = ReadBool(key, kOnlyIfColdVariationName); + g_pre_read_options.prefetch_virtual_memory = + ReadBool(key, kPrefetchVirtualMemoryVariationName); + g_pre_read_options.use_prefetch_argument = + !ReadBool(key, kNoPrefetchArgumentVariationName); } PreReadOptions GetPreReadOptions() { diff --git a/components/startup_metric_utils/common/pre_read_field_trial_utils_win.h b/components/startup_metric_utils/common/pre_read_field_trial_utils_win.h index cacf4b3..a2aaf4a 100644 --- a/components/startup_metric_utils/common/pre_read_field_trial_utils_win.h +++ b/components/startup_metric_utils/common/pre_read_field_trial_utils_win.h @@ -21,8 +21,8 @@ using RegisterPreReadSyntheticFieldTrialCallback = // The options controlled by the PreRead field trial. struct PreReadOptions { - // No explicit DLL pre-reading. - bool no_pre_read; + // Pre-read DLLs explicitly. + bool pre_read; // Pre-read DLLs with a high thread priority. bool high_priority; @@ -32,6 +32,9 @@ struct PreReadOptions { // Pre-read DLLs using the ::PrefetchVirtualMemory function, if available. bool prefetch_virtual_memory; + + // Use a /prefetch argument when launching a process. + bool use_prefetch_argument; }; // Initializes DLL pre-reading options from the registry. -- cgit v1.1