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

media/PRESUBMIT.py

Index: media/PRESUBMIT.py
diff --git a/media/PRESUBMIT.py b/media/PRESUBMIT.py
index feba2f719a5c6fe3c35fdb514af8fb5ef61cbbd4..36ee8792885a29b74bddeb6f4eafb68b8e763e1a 100644
--- a/media/PRESUBMIT.py
+++ b/media/PRESUBMIT.py
@@ -8,6 +8,18 @@ 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(
@@ -159,10 +171,23 @@ def _CheckForHistogramOffByOne(input_api, output_api):
   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