Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(292)

Side by Side Diff: media/PRESUBMIT.py

Issue 148553003: Clean up histogram'd media enum max values. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: address comments Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 # Copyright 2013 The Chromium Authors. All rights reserved. 1 # Copyright 2013 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Top-level presubmit script for Chromium media component. 5 """Top-level presubmit script for Chromium media component.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into gcl. 8 for more details about the presubmit API built into gcl.
9 """ 9 """
10 10
(...skipping 69 matching lines...) Expand 10 before | Expand all | Expand 10 after
80 if problems: 80 if problems:
81 return [output_api.PresubmitError( 81 return [output_api.PresubmitError(
82 'MessageLoopProxy should only be used for accessing the current loop.\n' 82 'MessageLoopProxy should only be used for accessing the current loop.\n'
83 'Use the TaskRunner interfaces instead as they are more explicit about\n' 83 'Use the TaskRunner interfaces instead as they are more explicit about\n'
84 'the run-time characteristics. In most cases, SingleThreadTaskRunner\n' 84 'the run-time characteristics. In most cases, SingleThreadTaskRunner\n'
85 'is a drop-in replacement for MessageLoopProxy.', problems)] 85 'is a drop-in replacement for MessageLoopProxy.', problems)]
86 86
87 return [] 87 return []
88 88
89 89
90 def _CheckForHistogramOffByOne(input_api, output_api):
91 """Make sure histogram enum maxes are used properly"""
92
93 # A general-purpose chunk of regex to match whitespace and/or comments
94 # that may be interspersed with the code we're interested in:
95 comment = r'/\*.*?\*/|//[^\n]*'
96 whitespace = r'(?:[\n\t ]|(?:' + comment + r'))*'
97
98 # The name is assumed to be a literal string.
99 histogram_name = r'"[^"]*"'
100
101 # This can be an arbitrary expression, so just ensure it isn't a ; to prevent
102 # matching past the end of this statement.
103 histogram_value = r'[^;]*'
104
105 # In parens so we can retrieve it for further checks.
106 histogram_max = r'([^;,]*)'
107
108 # This should match a uma histogram enumeration macro expression.
109 uma_macro_re = input_api.re.compile(
110 r'\bUMA_HISTOGRAM_ENUMERATION\(' + whitespace + histogram_name + r',' +
111 whitespace + histogram_value + r',' + whitespace + histogram_max +
112 whitespace + r'\)' + whitespace + r';(?:' + whitespace +
113 r'\/\/ (PRESUBMIT_IGNORE_UMA_MAX))?')
114
115 uma_max_re = input_api.re.compile(r'.*(?:Max|MAX).* \+ 1')
116
117 problems = []
118
119 for f in input_api.AffectedSourceFiles(_FilterFile):
120 contents = input_api.ReadFile(f)
121
122 # We want to match across lines, but still report a line number, so we keep
123 # track of the line we're on as we search through the file.
124 line_number = 1
125
126 # We search the entire file, then check if any violations are in the changed
127 # areas, this is inefficient, but simple. A UMA_HISTOGRAM_ENUMERATION call
128 # will often span multiple lines, so finding a match looking just at the
129 # deltas line-by-line won't catch problems.
130 match = uma_macro_re.search(contents)
131 while match:
132 line_number += contents.count('\n', 0, match.start())
133 max_arg = match.group(1) # The third argument.
134
135 if (not uma_max_re.match(max_arg) and match.group(2) !=
136 'PRESUBMIT_IGNORE_UMA_MAX'):
137 uma_range = range(match.start(), match.end() + 1)
138 # Check if any part of the match is in the changed lines:
139 for num, line in f.ChangedContents():
140 if line_number <= num <= line_number + match.group().count('\n'):
141 problems.append('%s:%d' % (f, line_number))
142 break
143
144 # Strip off the file contents up to the end of the match and update the
145 # line number.
146 contents = contents[match.end():]
147 line_number += match.group().count('\n')
148 match = uma_macro_re.search(contents)
149
150 if problems:
151 return [output_api.PresubmitError(
152 'UMA_HISTOGRAM_ENUMERATION reports in src/media/ are expected to adhere\n'
153 'to the following guidelines:\n'
154 ' - The max value (3rd argument) should be an enum value equal to the\n'
155 ' last valid value, e.g. FOO_MAX = LAST_VALID_FOO.\n'
156 ' - 1 must be added to that max value.\n'
157 'Contact rileya@chromium.org if you have questions.' , problems)]
158
159 return []
160
161
90 def _CheckChange(input_api, output_api): 162 def _CheckChange(input_api, output_api):
91 results = [] 163 results = []
92 results.extend(_CheckForUseOfWrongClock(input_api, output_api)) 164 results.extend(_CheckForUseOfWrongClock(input_api, output_api))
93 results.extend(_CheckForMessageLoopProxy(input_api, output_api)) 165 results.extend(_CheckForMessageLoopProxy(input_api, output_api))
166 results.extend(_CheckForHistogramOffByOne(input_api, output_api))
94 return results 167 return results
95 168
96 169
97 def CheckChangeOnUpload(input_api, output_api): 170 def CheckChangeOnUpload(input_api, output_api):
98 return _CheckChange(input_api, output_api) 171 return _CheckChange(input_api, output_api)
99 172
100 173
101 def CheckChangeOnCommit(input_api, output_api): 174 def CheckChangeOnCommit(input_api, output_api):
102 return _CheckChange(input_api, output_api) 175 return _CheckChange(input_api, output_api)
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698