Add size checking script for android-binary-size trybot

And fix diagnose_bloat.py regexp that extracts summary
from supersize output

Bug: 702625
Change-Id: Ib8941a2cb84c18d02dd980e4725f44ea6ed4fcb7
Reviewed-on: https://chromium-review.googlesource.com/1071708
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562557}

tools/binary_size/BUILD.gn

@@ -10,5 +10,6 @@ python_library("binary_size_trybot_py") {
   pydeps_file = "supersize.pydeps"
   data = [
     "diagnose_bloat.py",
+    "trybot_commit_size_checker.py",
   ]
 }

tools/binary_size/diagnose_bloat.py

@@ -62,7 +62,8 @@ class BaseDiff(object):
 
   @property
   def summary_stat(self):
-    return None
+    """Returns a tuple of (name, value, units) for the most important metric."""
+    raise NotImplementedError()
 
   def Summary(self):
     """A short description that summarizes the source of binary size bloat."""
@@ -85,7 +86,7 @@ class BaseDiff(object):
 class NativeDiff(BaseDiff):
   # E.g.: Section Sizes (Total=1.2 kb (1222 bytes)):
   _RE_SUMMARY_STAT = re.compile(
-      r'Section Sizes \(Total=(?P<value>\d+) ?(?P<units>\w+)')
+      r'Section Sizes \(Total=(?P<value>-?[0-9\.]+) ?(?P<units>\w+)')
   _SUMMARY_STAT_NAME = 'Native Library Delta'
 
   def __init__(self, size_name, supersize_path):
@@ -100,7 +101,7 @@ class NativeDiff(BaseDiff):
     if m:
       return _DiffResult(
           NativeDiff._SUMMARY_STAT_NAME, m.group('value'), m.group('units'))
-    return None
+    raise Exception('Could not extract total from:\n' + self._diff)
 
   def DetailedResults(self):
     return self._diff.splitlines()
@@ -135,7 +136,7 @@ class ResourceSizesDiff(BaseDiff):
         if 'normalized' in subsection_name:
           full_name = '{} {}'.format(section_name, subsection_name)
           return _DiffResult(full_name, value, units)
-    return None
+    raise Exception('Could not find "normalized" in: ' + repr(self._diff))
 
   def DetailedResults(self):
     return self._ResultLines()
@@ -833,17 +834,13 @@ def _DiffMain(args):
     supersize_path = os.path.join(_BINARY_SIZE_DIR, 'supersize')
     diff = NativeDiff(args.apk_name + '.size', supersize_path)
   else:
-    diff = ResourceSizesDiff(args.apk_name, args.apk_name + '.json')
+    diff = ResourceSizesDiff(args.apk_name)
 
   diff.ProduceDiff(args.before_dir, args.after_dir)
   with open(args.diff_output, 'w') as f:
     f.writelines(l + '\n' for l in diff.DetailedResults())
-
-  stat = diff.summary_stat
-  if stat:
-    print 'Summary: {} {} {}'.format(*stat)
-  else:
-    print 'Missing Summary!'
+    stat = diff.summary_stat
+    f.write('{}={}\n'.format(*stat[:2]))
 
 
 def main():

tools/binary_size/trybot_commit_size_checker.py

+#!/usr/bin/env python
+# Copyright 2018 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.
+
+"""Fails if a try job increases binary size unexpectedly."""
+
+import argparse
+import sys
+
+
+_MAX_UNNOTICED_INCREASE = 16 * 1024
+
+
+def main():
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--author', help='CL author')
+  parser.add_argument('--resource-sizes-diff',
+                      help='Path to resource sizes diff produced by '
+                           '"diagnose_bloat.py diff sizes".')
+  args = parser.parse_args()
+
+  # Last line looks like:
+  # MonochromePublic.apk_Specifics normalized apk size=1234
+  with open(args.resource_sizes_diff) as f:
+    last_line = f.readlines()[-1]
+    size_delta = int(last_line.partition('=')[2])
+
+  is_roller = '-autoroll' in args.author
+
+  # Useful for bot debugging to have these printed out:
+  print 'Is Roller:', is_roller
+  print 'Increase:', size_delta
+
+  if size_delta > _MAX_UNNOTICED_INCREASE and not is_roller:
+    failure_message = """
+Binary size increase is non-trivial (where "non-trivial" means the normalized \
+size increased by more than {} bytes).
+
+Please look at the symbol diffs from the "Show Resource Sizes Diff" and the \
+"Show Supersize Diff" bot steps. Try and understand the growth and see if it \
+can be mitigated. There is guidance at:
+
+https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase
+
+If the growth is expected / justified, then you can bypass this bot failure by \
+adding "Binary-Size: $JUSTIFICATION" to your commit description. Here are some \
+examples:
+
+Binary-Size: Increase is due to translations and so cannot be avoided.
+Binary-Size: Increase is due to new images, which are already optimally encoded.
+Binary-Size: Increase is temporary due to a "new way" / "old way" refactoring.
+    It should go away once the "old way" is removed.
+Binary-Size: Increase is temporary and will be reverted before next branch cut.
+Binary-Size: Increase needed to reduce RAM of a common user flow.
+Binary-Size: Increase needed to reduce runtime of a common user flow.
+Binary-Size: Increase needed to implement a feature, and I've already spent a
+    non-trivial amount of time trying to reduce its size.
+""".format(_MAX_UNNOTICED_INCREASE)
+    sys.exit(failure_message)
+
+
+if __name__ == '__main__':
+  main()