Commit 2c0483fc authored by Wenbin Zhang's avatar Wenbin Zhang Committed by Commit Bot

Reland "Set limit for field trial parameters length."

This is a reland of 1547c775

Original change's description:
> Set limit for field trial parameters length.
>
> Windows has a limit of 32767 on a command line. When testing with field
> trails, the command to launch chrome.exe may exceed the limit if there're
> too many field trial configs passed as parameters. This was causing
> the chrome perf waterfall to completely fail on Windows.
>
> This CL added a presubmit check to see whether the configs under Windows
> will exceed the limit of 31000.
>
> Bug: chromium:1045530
> Change-Id: I9d181783f1bdad58eeba802583c052bfd0321c6f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2021468
> Commit-Queue: Wenbin Zhang <wenbinzhang@google.com>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#735981}

Bug: chromium:1045530
Change-Id: I04b664b6f8a6b21f6202df95d912bd220d669728
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025511
Commit-Queue: Wenbin Zhang <wenbinzhang@google.com>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736166}
parent e5f85ba2
......@@ -18,6 +18,16 @@ VALID_EXPERIMENT_KEYS = [
'//0', '//1', '//2', '//3', '//4', '//5', '//6', '//7', '//8', '//9'
]
FIELDTRIAL_CONFIG_FILE_NAME = 'fieldtrial_testing_config.json'
FIELDTRIAL_CONFIG_TOO_LONG_ERROR_MSG = \
"Contents of %s result in command-line flags that exceed Windows' 32767 "\
"character limit. To add a new entry, please remove existing obsolete "\
"entries. To check if an entry is obsolete, do a code search for its "\
"'enable_features' and 'disable_features' strings and verify there are "\
"no results other than the files related to the testing config. "\
"See crbug.com/1045530 for more details." % FIELDTRIAL_CONFIG_FILE_NAME
def PrettyPrint(contents):
"""Pretty prints a fieldtrial configuration.
......@@ -88,10 +98,12 @@ def PrettyPrint(contents):
ordered_config, sort_keys=False, indent=4, separators=(',', ': ')) + '\n'
def ValidateData(json_data, file_path, message_type):
def ValidateData(input_api, json_data, file_path, message_type):
"""Validates the format of a fieldtrial configuration.
Args:
input_api: An instance passed to presubmit scripts telling info about the
changes.
json_data: Parsed JSON object representing the fieldtrial config.
file_path: String representing the path to the JSON file.
message_type: Type of message from |output_api| to return in the case of
......@@ -106,6 +118,9 @@ def ValidateData(json_data, file_path, message_type):
return _CreateMalformedConfigMessage(message_type, file_path,
message_format, *args)
if not _IsFieldTrialSizeBelowLimitOnWindows(input_api, file_path):
return [message_type(FIELDTRIAL_CONFIG_TOO_LONG_ERROR_MSG)]
if not isinstance(json_data, dict):
return _CreateMessage('Expecting dict')
for (study, experiment_configs) in json_data.iteritems():
......@@ -228,15 +243,54 @@ def CheckPretty(contents, file_path, message_type):
return []
def _IsFieldTrialSizeBelowLimitOnWindows(input_api, file_path):
"""Checks whether the fieldtrial parameters exceeded the Windows cmd limit.
When launching chrome, the fieldtrial related parameters take more than
30,000 characters. Windows has a limit of 32767 characters on command
line and thus it raises parameter error when the parameters are too long.
Before we have a valid fix, we need to limit the number of fieldtrias in
fieldtrial_testing_config.json to 31,500, from the fact that the
non-fieldtrial parameters take roughly 1450 characters.
See crbug.com/1045530 for more details.
Args:
input_api: An instance passed to presubmit scripts telling info about the
changes.
file_path: the absolute path to the json file with the fieldtrial configs.
"""
sys.path.append(input_api.os_path.join(
input_api.PresubmitLocalPath(), '..', '..', 'tools', 'variations'))
import fieldtrial_util
args = fieldtrial_util.GenerateArgs(file_path, 'windows')
total_length = 0
for arg in args:
total_length += len(arg)
return total_length < 31500
def CommonChecks(input_api, output_api):
affected_files = input_api.AffectedFiles(
include_deletes=False,
file_filter=lambda x: x.LocalPath().endswith('.json'))
for f in affected_files:
if not f.LocalPath().endswith(FIELDTRIAL_CONFIG_FILE_NAME):
return [
output_api.PresubmitError(
'%s is the only json file expected in this folder. If new jsons '
'are added, please update the presubmit process with proper '
'validation. ' % FIELDTRIAL_CONFIG_FILE_NAME
)
]
contents = input_api.ReadFile(f)
try:
json_data = input_api.json.loads(contents)
result = ValidateData(json_data, f.LocalPath(), output_api.PresubmitError)
result = ValidateData(
input_api,
json_data,
f.AbsoluteLocalPath(),
output_api.PresubmitError)
if len(result):
return result
result = CheckPretty(contents, f.LocalPath(), output_api.PresubmitError)
......
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