[android] Make findbugs analysis errors fatal.

This CL also:
 - updates an outdated ANDROID_SDK_VERSION constant
 - fixes findbugs errors that had crept into the build

Bug: 755685
Change-Id: I46783270cec8fae51add80c69117e2e12434eea0
Reviewed-on: https://chromium-review.googlesource.com/619038
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496354}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 27eb8dc48592c57bb94268a45985c956ffa2fbfa

android/findbugs_diff.py

@@ -39,6 +39,9 @@ def main():
 
   parser.add_argument(
       '-v', '--verbose', action='count', help='Enable verbose logging.')
+  parser.add_argument(
+      '--system-jar', default=None, dest='system_jars', action='append',
+      help='System JAR for analysis.')
   parser.add_argument(
       '-a', '--auxclasspath', default=None, dest='auxclasspath',
       help='Set aux classpath for analysis.')
@@ -88,21 +91,29 @@ def main():
     if not args.exclude:
       args.exclude = os.path.join(args.base_dir, 'findbugs_exclude.xml')
 
-  findbugs_command, findbugs_warnings = findbugs.Run(
+  findbugs_command, findbugs_errors, findbugs_warnings = findbugs.Run(
-      args.exclude, args.only_analyze, args.auxclasspath,
+      args.exclude, args.only_analyze, args.system_jars, args.auxclasspath,
       args.output_file, args.findbug_args, args.jar_paths)
 
-  if findbugs_warnings:
+  if findbugs_warnings or findbugs_errors:
     print
     print '*' * 80
     print 'FindBugs run via:'
     print findbugs_command
-    print
+    if findbugs_errors:
-    print 'FindBugs reported the following issues:'
+      print
-    for warning in sorted(findbugs_warnings):
+      print 'FindBugs encountered the following errors:'
-      print str(warning)
+      for error in sorted(findbugs_errors):
-    print '*' * 80
+        print str(error)
-    print
+      print '*' * 80
+      print
+    if findbugs_warnings:
+      print
+      print 'FindBugs reported the following issues:'
+      for warning in sorted(findbugs_warnings):
+        print str(warning)
+      print '*' * 80
+      print
   else:
     if args.depfile:
       deps = args.auxclasspath + args.jar_paths
@@ -110,7 +121,7 @@ def main():
     if args.stamp:
       build_utils.Touch(args.stamp)
 
-  return len(findbugs_warnings)
+  return len(findbugs_errors) + len(findbugs_warnings)
 
 
 if __name__ == '__main__':

android/pylib/constants/__init__.py

@@ -95,7 +95,7 @@ DEVICE_PERF_OUTPUT_DIR = (
 
 SCREENSHOTS_DIR = os.path.join(DIR_SOURCE_ROOT, 'out_screenshots')
 
-ANDROID_SDK_VERSION = version_codes.MARSHMALLOW
+ANDROID_SDK_VERSION = version_codes.O
 ANDROID_SDK_BUILD_TOOLS_VERSION = '26.0.0'
 ANDROID_SDK_ROOT = os.path.join(DIR_SOURCE_ROOT,
                                 'third_party', 'android_tools', 'sdk')

android/pylib/utils/findbugs.py

@@ -7,7 +7,6 @@ import os
 import xml.dom.minidom
 
 from devil.utils import cmd_helper
-from pylib import constants
 from pylib.constants import host_paths
 
 
@@ -21,12 +20,15 @@ _FINDBUGS_PLUGIN_PATH = os.path.join(
 
 
 def _ParseXmlResults(results_doc):
+  errors = set()
   warnings = set()
   for en in (n for n in results_doc.documentElement.childNodes
              if n.nodeType == xml.dom.Node.ELEMENT_NODE):
+    if en.tagName == 'Errors':
+      errors.update(_ParseErrors(en))
     if en.tagName == 'BugInstance':
       warnings.add(_ParseBugInstance(en))
-  return warnings
+  return errors, warnings
 
 
 def _GetMessage(node):
@@ -39,6 +41,30 @@ def _GetMessage(node):
   return None
 
 
+def _GetTextContent(node):
+  if (len(node.childNodes) == 1
+      and node.childNodes[0].nodeType == xml.dom.Node.TEXT_NODE):
+    return node.childNodes[0].data
+  return ''
+
+
+def _ParseErrors(node):
+  errors = set()
+  for error_node in (n for n in node.childNodes
+                     if n.nodeType == xml.dom.Node.ELEMENT_NODE):
+    error_text = '(unable to determine error text)'
+    if error_node.tagName == 'Error':
+      error_message_nodes = (
+          n for n in error_node.childNodes
+          if (n.nodeType == xml.dom.Node.ELEMENT_NODE
+              and n.tagName == 'ErrorMessage'))
+      text_pieces = [_GetTextContent(n) for n in error_message_nodes]
+      if text_pieces:
+        error_text = ', '.join(t for t in text_pieces if t)
+      errors.add(FindBugsError(error_node.tagName, error_text))
+  return errors
+
+
 def _ParseBugInstance(node):
   bug = FindBugsWarning(node.getAttribute('type'))
   msg_parts = []
@@ -57,13 +83,34 @@ def _ParseBugInstance(node):
       if c.hasAttribute('end'):
         bug.end_line = int(c.getAttribute('end'))
       msg_parts.append(_GetMessage(c))
-    elif (c.tagName == 'ShortMessage' and len(c.childNodes) == 1
+    elif c.tagName == 'ShortMessage':
-          and c.childNodes[0].nodeType == xml.dom.Node.TEXT_NODE):
+      msg_parts.append(_GetTextContent(c))
-      msg_parts.append(c.childNodes[0].data)
   bug.message = tuple(m for m in msg_parts if m)
   return bug
 
 
+class FindBugsError(object):
+  def __init__(self, error_type='', error_val=''):
+    self.error_type = error_type
+    self.error_val = error_val
+
+  def __cmp__(self, other):
+    return (cmp(self.error_type, other.error_type)
+            or cmp(self.error_val, other.error_val))
+
+  def __eq__(self, other):
+    return self.__dict__ == other.__dict__
+
+  def __hash__(self):
+    return hash((self.error_type, self.error_val))
+
+  def __ne__(self, other):
+    return not self == other
+
+  def __str__(self):
+    return '%s: %s' % (self.error_type, self.error_val)
+
+
 class FindBugsWarning(object):
 
   def __init__(self, bug_type='', end_line=0, file_name='', message=None,
@@ -98,26 +145,26 @@ class FindBugsWarning(object):
     return '%s: %s' % (self.bug_type, '\n  '.join(self.message))
 
 
-def Run(exclude, classes_to_analyze, auxiliary_classes, output_file,
+def Run(exclude, classes_to_analyze, system_classes, auxiliary_classes,
-        findbug_args, jars):
+        output_file, findbug_args, jars):
   """Run FindBugs.
 
   Args:
     exclude: the exclude xml file, refer to FindBugs's -exclude command option.
-    classes_to_analyze: the list of classes need to analyze, refer to FindBug's
+    classes_to_analyze: the list of classes need to analyze, refer to FindBugs'
                         -onlyAnalyze command line option.
-    auxiliary_classes: the classes help to analyze, refer to FindBug's
+    system_classes: system classes to help analysis.
+    auxiliary_classes: other classes to help analysis. Refer to FindBugs'
                        -auxclasspath command line option.
     output_file: An optional path to dump XML results to.
     findbug_args: A list of additional command line options to pass to Findbugs.
   """
   # TODO(jbudorick): Get this from the build system.
-  system_classes = [
+  all_aux_classes = []
-    os.path.join(constants.ANDROID_SDK_ROOT, 'platforms',
+  all_aux_classes.extend(system_classes or [])
-                 'android-%s' % constants.ANDROID_SDK_VERSION, 'android.jar')
+  all_aux_classes.extend(
-  ]
+      os.path.abspath(classes)
-  system_classes.extend(os.path.abspath(classes)
+      for classes in auxiliary_classes or [])
-                        for classes in auxiliary_classes or [])
 
   cmd = ['java',
          '-classpath', '%s:' % _FINDBUGS_JAR,
@@ -127,7 +174,7 @@ def Run(exclude, classes_to_analyze, auxiliary_classes, output_file,
          '-textui', '-sortByClass',
          '-pluginList', _FINDBUGS_PLUGIN_PATH, '-xml:withMessages']
   if system_classes:
-    cmd.extend(['-auxclasspath', ':'.join(system_classes)])
+    cmd.extend(['-auxclasspath', ':'.join(all_aux_classes)])
   if classes_to_analyze:
     cmd.extend(['-onlyAnalyze', classes_to_analyze])
   if exclude:
@@ -149,7 +196,7 @@ def Run(exclude, classes_to_analyze, auxiliary_classes, output_file,
   for line in stderr.splitlines():
     logging.debug('  %s', line)
 
-  current_warnings_set = _ParseXmlResults(results_doc)
+  current_errors_set, current_warnings_set = _ParseXmlResults(results_doc)
 
-  return (' '.join(cmd), current_warnings_set)
+  return (' '.join(cmd), current_errors_set, current_warnings_set)
 

config/android/internal_rules.gni

@@ -949,6 +949,8 @@ if (enable_java_templates) {
         rebase_path(depfile, root_build_dir),
         "--exclude",
         rebase_path(_exclusions_file, root_build_dir),
+        "--system-jar",
+        rebase_path(android_sdk_jar, root_build_dir),
         "--auxclasspath-gyp",
         "@FileArg($_rebased_build_config:javac:classpath)",
         "--output-file",