media/PRESUBMIT.py: adding CheckPassByValue for base::Time and derived classes

media/PRESUBMIT.py

 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Top-level presubmit script for Chromium media component.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into depot_tools.
 """
 
+import re
+import string
+
+# Well-defined simple classes containing only <= 4 ints, or <= 2 floats.
+PASS_BY_VALUE_TYPES = ['base::Time',
+                       'base::TimeDelta',
+                       'base::TimeTicks',
+                       ]

[sandersd (OOO until July 31), 2015/10/16 21:52:27]

Can you check that pyformat agrees with this? If not, I prefer:

PASS_BY_VALUE_TYPES = [
    'base::Time',
    'base::TimeDelta',
    'base::TimeTicks',
]

[mcasas, 2015/10/20 00:24:18]

If you're happy like that, I'm happy too. Done.

+
+PASS_BY_VALUE_RE = re.compile(r'\bconst +' + '%s&' %

[sandersd (OOO until July 31), 2015/10/16 21:52:27]

This requires parentheses around %s.

[mcasas, 2015/10/20 00:24:18]

Done.

+                              string.join(PASS_BY_VALUE_TYPES, '|'))
+
 def _FilterFile(affected_file):
   """Return true if the file could contain code requiring a presubmit check."""
   return affected_file.LocalPath().endswith(
       ('.h', '.cc', '.cpp', '.cxx', '.mm'))
 
 
 def _CheckForUseOfWrongClock(input_api, output_api):
   """Make sure new lines of media code don't use a clock susceptible to skew."""
 
   # Regular expression that should detect any explicit references to the
@@ skipping 131 matching lines @@
       'UMA_HISTOGRAM_ENUMERATION reports in src/media/ are expected to adhere\n'
       'to the following guidelines:\n'
       ' - The max value (3rd argument) should be an enum value equal to the\n'
       '   last valid value, e.g. FOO_MAX = LAST_VALID_FOO.\n'
       ' - 1 must be added to that max value.\n'
       'Contact rileya@chromium.org if you have questions.' , problems)]
 
   return []
 
 
+def _CheckPassByValue(input_api, output_api):
+  local_errors = []
+
+  for f in input_api.AffectedSourceFiles(_FilterFile):
+    for line_number, line in f.ChangedContents():
+      if PASS_BY_VALUE_RE.search(line.strip()):

[sandersd (OOO until July 31), 2015/10/16 21:52:27]

No need to strip the line here.

[mcasas, 2015/10/20 00:24:18]

Done.

+        local_errors.append(output_api.PresubmitError(
+            '  %s:%d passes argument by const ref instead of by value:\n   %s' %

[sandersd (OOO until July 31), 2015/10/16 21:52:27]

I'm not too happy with this message because it doesn't actually tell you
precisely what to fix to make it happy. I'd rather include the name of the class
and rephrase as an instruction to fix it.

[mcasas, 2015/10/20 00:24:18]

Rephrase to make it look more like the neighbours.
Please suggest alternative otherwise...

+            (f.LocalPath(), line_number, line.strip())))
+  return local_errors
+
+
 def _CheckChange(input_api, output_api):
   results = []
   results.extend(_CheckForUseOfWrongClock(input_api, output_api))
   results.extend(_CheckForMessageLoopProxy(input_api, output_api))
+  results.extend(_CheckPassByValue(input_api, output_api))
   results.extend(_CheckForHistogramOffByOne(input_api, output_api))
   return results
 
 
 def CheckChangeOnUpload(input_api, output_api):
   return _CheckChange(input_api, output_api)
 
 
 def CheckChangeOnCommit(input_api, output_api):
   return _CheckChange(input_api, output_api)