[Cronet] Enforce Cronet API never modified, only grown

Enforce that the Cronet API classes and methods are never modified or
removed, only added.  Modifying or removing an API class or method
breaks backwards compatibility.  This is enforced by:
1. Keep an updated dump of the Cronet API in components/cronet/android/api.txt
2. When building check that the Cronet API matches that in api.txt
3. Provide a script for updating api.txt; this script also enforces that
   only additions are made to the API.
This change also maintains a version number for the Cronet API.  Cronet
can use this to enforce, for example that the Cronet implementation is at
least as new as the Cronet API being used.

BUG=629299
R=kapishnikov
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2544043002
Cr-Commit-Position: refs/heads/master@{#441455}

components/cronet/android/BUILD.gn

@@ -1435,6 +1435,13 @@ action("api_static_checks") {
     ":cronet_impl_native_java",
     ":cronet_impl_platform_java",
   ]
+  inputs = [
+    "//components/cronet/tools/update_api.py",
+  ]
+  sources = [
+    "//components/cronet/android/api.txt",
+    "//components/cronet/android/api_version.txt",
+  ]
 }
 
 group("cronet_package") {

components/cronet/android/api_version.txt

@@ -0,0 +1 @@
+1

components/cronet/tools/api_static_checks.py

@@ -3,9 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""api_static_checks.py - Check Cronet implementation does not call through
-API classes.
-"""
+"""api_static_checks.py - Enforce Cronet API requirements."""
 
 import argparse
 import os
@@ -20,9 +18,13 @@
 sys.path.append(os.path.join(REPOSITORY_ROOT, 'build/android/gyp/util'))
 import build_utils
 
+sys.path.append(os.path.join(REPOSITORY_ROOT, 'components'))
+from cronet.tools import update_api
+
+
 # These regular expressions catch the beginning of lines that declare classes
 # and methods.  The first group returned by a match is the class or method name.
-CLASS_RE = re.compile(r'.*class ([^ ]*) .*\{')
+from cronet.tools.update_api import CLASS_RE
 METHOD_RE = re.compile(r'.* ([^ ]*)\(.*\);')
 
 # Allowed exceptions.  Adding anything to this list is dangerous and should be
@@ -104,24 +106,9 @@ def find_api_calls(dump, api_classes, bad_calls):
         bad_calls += [bad_call]
 
 
-def main(args):
+def check_api_calls(opts):
   # Returns True if no calls through API classes in implementation.
 
-  parser = argparse.ArgumentParser(
-      description='Check modules do not contain ARM Neon instructions.')
-  parser.add_argument('--api_jar',
-                      help='Path to API jar (i.e. cronet_api.jar)',
-                      required=True,
-                      metavar='path/to/cronet_api.jar')
-  parser.add_argument('--impl_jar',
-                      help='Path to implementation jar '
-                          '(i.e. cronet_impl_native_java.jar)',
-                      required=True,
-                      metavar='path/to/cronet_impl_native_java.jar',
-                      action='append')
-  parser.add_argument('--stamp', help='Path to touch on success.')
-  opts = parser.parse_args(args)
-
   temp_dir = tempfile.mkdtemp()
 
   # Extract API class files from jar
@@ -174,10 +161,40 @@ def main(args):
     print '       does not contain newer methods.  Please call through a'
     print '       wrapper class from VersionSafeCallbacks.'
     print '\n'.join(bad_api_calls)
+  return not bad_api_calls
+
 
-  if not bad_api_calls and opts.stamp:
+def check_api_version(opts):
+  if update_api.check_up_to_date(opts.api_jar):
+    return True
+  print 'ERROR: API file out of date.  Please run this command:'
+  print '       components/cronet/tools/update_api.py --api_jar %s' % (
+      os.path.abspath(opts.api_jar))
+  return False
+
+
+def main(args):
+  parser = argparse.ArgumentParser(
+      description='Enforce Cronet API requirements.')
+  parser.add_argument('--api_jar',
+                      help='Path to API jar (i.e. cronet_api.jar)',
+                      required=True,
+                      metavar='path/to/cronet_api.jar')
+  parser.add_argument('--impl_jar',
+                      help='Path to implementation jar '
+                          '(i.e. cronet_impl_native_java.jar)',
+                      required=True,
+                      metavar='path/to/cronet_impl_native_java.jar',
+                      action='append')
+  parser.add_argument('--stamp', help='Path to touch on success.')
+  opts = parser.parse_args(args)
+
+  ret = True
+  ret = check_api_calls(opts) and ret
+  ret = check_api_version(opts) and ret
+  if ret and opts.stamp:
     build_utils.Touch(opts.stamp)
-  return not bad_api_calls
+  return ret
 
 
 if __name__ == '__main__':

components/cronet/tools/api_static_checks_unittest.py

@@ -14,17 +14,43 @@
 import tempfile
 import unittest
 
-from tools import api_static_checks
+REPOSITORY_ROOT = os.path.abspath(os.path.join(
+    os.path.dirname(__file__), '..', '..', '..'))
 
+sys.path.append(os.path.join(REPOSITORY_ROOT, 'components'))
+from cronet.tools import api_static_checks
 
-ERROR_PREFIX = (
+
+ERROR_PREFIX_CHECK_API_CALLS = (
 """ERROR: Found the following calls from implementation classes through
        API classes.  These could fail if older API is used that
        does not contain newer methods.  Please call through a
        wrapper class from VersionSafeCallbacks.
 """)
 
 
+ERROR_PREFIX_UPDATE_API = (
+"""ERROR: This API was modified or removed:
+             """)
+
+
+ERROR_SUFFIX_UPDATE_API = (
+"""
+
+       Cronet API methods and classes cannot be modified.
+""")
+
+
+CHECK_API_VERSION_PREFIX = (
+"""DO NOT EDIT THIS FILE, USE update_api.py TO UPDATE IT
+
+""")
+
+
+API_FILENAME = './android/api.txt'
+API_VERSION_FILENAME = './android/api_version.txt'
+
+
 @contextlib.contextmanager
 def capture_output():
   # A contextmanger that collects the stdout and stderr of wrapped code
@@ -44,6 +70,12 @@ class ApiStaticCheckUnitTest(unittest.TestCase):
   def setUp(self):
     self.temp_dir = tempfile.mkdtemp()
     os.chdir(self.temp_dir)
+    os.mkdir('android')
+    with open(API_VERSION_FILENAME, 'w') as api_version_file:
+      api_version_file.write('0')
+    with open(API_FILENAME, 'w') as api_file:
+      api_file.write('}\n')
+    shutil.copytree(os.path.dirname(__file__), 'tools')
 
 
   def tearDown(self):
@@ -55,41 +87,149 @@ def make_jar(self, java, class_name):
     # return jar filename.
 
     java_filename = class_name + '.java'
-    class_filename = class_name + '.class'
+    class_filenames = class_name + '*.class'
     jar_filename = class_name + '.jar'
 
     with open(java_filename, 'w') as java_file:
       java_file.write('public class %s {' % class_name)
       java_file.write(java)
       java_file.write('}')
     os.system('javac %s' % java_filename)
-    os.system('jar cf %s %s' % (jar_filename, class_filename))
+    os.system('jar cf %s %s' % (jar_filename, class_filenames))
     return jar_filename
 
 
-  def run_test(self, api_java, impl_java):
-    api_jar = self.make_jar(api_java, 'Api')
-    impl_jar = self.make_jar(impl_java, 'Impl')
+  def run_check_api_calls(self, api_java, impl_java):
+    test = self
+    class MockOpts(object):
+      def __init__(self):
+        self.api_jar = test.make_jar(api_java, 'Api')
+        self.impl_jar = [test.make_jar(impl_java, 'Impl')]
+    opts = MockOpts()
     with capture_output() as return_output:
-      return_code = api_static_checks.main(
-          ['--api_jar', api_jar, '--impl_jar', impl_jar])
+      return_code = api_static_checks.check_api_calls(opts)
     return [return_code, return_output[0]]
 
 
-  def test_success(self):
+  def test_check_api_calls_success(self):
     # Test simple classes with functions
-    self.assertEqual(self.run_test('void a(){}', 'void b(){}'), [True, ''])
+    self.assertEqual(self.run_check_api_calls(
+        'void a(){}', 'void b(){}'), [True, ''])
     # Test simple classes with functions calling themselves
-    self.assertEqual(self.run_test(
+    self.assertEqual(self.run_check_api_calls(
         'void a(){} void b(){a();}', 'void c(){} void d(){c();}'), [True, ''])
 
 
-  def test_failure(self):
+  def test_check_api_calls_failure(self):
     # Test static call
-    self.assertEqual(self.run_test(
+    self.assertEqual(self.run_check_api_calls(
         'public static void a(){}', 'void b(){Api.a();}'),
-        [False, ERROR_PREFIX + 'Impl/b -> Api/a:()V\n'])
+        [False, ERROR_PREFIX_CHECK_API_CALLS + 'Impl/b -> Api/a:()V\n'])
     # Test virtual call
-    self.assertEqual(self.run_test(
+    self.assertEqual(self.run_check_api_calls(
         'public void a(){}', 'void b(){new Api().a();}'),
-        [False, ERROR_PREFIX + 'Impl/b -> Api/a:()V\n'])
+        [False, ERROR_PREFIX_CHECK_API_CALLS + 'Impl/b -> Api/a:()V\n'])
+
+
+  def run_check_api_version(self, java):
+    OUT_FILENAME = 'out.txt'
+    return_code = os.system('./tools/update_api.py --api_jar %s > %s' %
+        (self.make_jar(java, 'Api'), OUT_FILENAME))
+    with open(API_FILENAME, 'r') as api_file:
+      api = api_file.read()
+    with open(API_VERSION_FILENAME, 'r') as api_version_file:
+      api_version = api_version_file.read()
+    with open(OUT_FILENAME, 'r') as out_file:
+      output = out_file.read()
+    return [return_code == 0, output, api, api_version]
+
+
+  def test_update_api_success(self):
+    # Test simple new API
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+}
+""", '1'])
+    # Test version number not increased when API not changed
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+}
+""", '1'])
+    # Test acceptable API method addition
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){} public void b(){}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+  public void b();
+}
+""", '2'])
+    # Test version number not increased when API not changed
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){} public void b(){}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+  public void b();
+}
+""", '2'])
+    # Test acceptable API class addition
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){} public void b(){} public class C {}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api$C {
+  public Api$C(Api);
+}
+public class Api {
+  public Api();
+  public void a();
+  public void b();
+}
+""", '3'])
+    # Test version number not increased when API not changed
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){} public void b(){} public class C {}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api$C {
+  public Api$C(Api);
+}
+public class Api {
+  public Api();
+  public void a();
+  public void b();
+}
+""", '3'])
+
+
+  def test_update_api_failure(self):
+    # Create a simple new API
+    self.assertEqual(self.run_check_api_version(
+        'public void a(){}'),
+        [True, '', CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+}
+""", '1'])
+    # Test removing API method not allowed
+    self.assertEqual(self.run_check_api_version(''),
+        [False, ERROR_PREFIX_UPDATE_API + 'public void a();'
+            + ERROR_SUFFIX_UPDATE_API,
+            CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+}
+""", '1'])
+    # Test modifying API method not allowed
+    self.assertEqual(self.run_check_api_version(
+        'public void a(int x){}'),
+        [False, ERROR_PREFIX_UPDATE_API + 'public void a();'
+            + ERROR_SUFFIX_UPDATE_API,
+            CHECK_API_VERSION_PREFIX + """public class Api {
+  public Api();
+  public void a();
+}
+""", '1'])