Android: Fix JNI headers not being regenerated.

Previously, moving Java files used by JNI to new locations could cause
build failures. This CL addressed stale generated files by deleting
stale files from the output directory before creating new ones.

Also merge the two generate_jni() templates since they're pretty much
the same.

Bug: 951701
Change-Id: Id066d8e74a52ed0c4c6fd29e92108c2e0bbfe96b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610749
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660651}

base/android/jni_generator/jni_generator.py

@@ -12,6 +12,7 @@ import base64
 import collections
 import errno
 import hashlib
+import itertools
 import optparse
 import os
 import re
@@ -1503,35 +1504,6 @@ def WrapOutput(output):
   return '\n'.join(ret)
 
 
-def ExtractJarInputFile(jar_file, input_file, out_dir):
-  """Extracts input file from jar and returns the filename.
-
-  The input file is extracted to the same directory that the generated jni
-  headers will be placed in.  This is passed as an argument to script.
-
-  Args:
-    jar_file: the jar file containing the input files to extract.
-    input_files: the list of files to extract from the jar file.
-    out_dir: the name of the directories to extract to.
-
-  Returns:
-    the name of extracted input file.
-  """
-  jar_file = zipfile.ZipFile(jar_file)
-
-  out_dir = os.path.join(out_dir, os.path.dirname(input_file))
-  try:
-    os.makedirs(out_dir)
-  except OSError as e:
-    if e.errno != errno.EEXIST:
-      raise
-  extracted_file_name = os.path.join(out_dir, os.path.basename(input_file))
-  with open(extracted_file_name, 'wb') as outfile:
-    outfile.write(jar_file.read(input_file))
-
-  return extracted_file_name
-
-
 def GenerateJNIHeader(input_file, output_file, options):
   try:
     if os.path.splitext(input_file)[1] == '.class':
@@ -1579,13 +1551,13 @@ See SampleForTests.java for more details.
                            help='Uses as a namespace in the generated header '
                            'instead of the javap class name, or when there is '
                            'no JNINamespace annotation in the java source.')
-  option_parser.add_option('--input_file',
-                           help='Single input file name. The output file name '
-                           'will be derived from it. Must be used with '
-                           '--output_dir.')
+  option_parser.add_option('--input_files',
+                           help='Input file names, or paths within a .jar if '
+                           '--jar-file is used.')
+  option_parser.add_option('--output_files',
+                           help='Output file names.')
   option_parser.add_option('--output_dir',
-                           help='The output directory. Must be used with '
-                           '--input')
+                           help='The output directory.')
   option_parser.add_option('--script_name', default=GetScriptName(),
                            help='The name of this script in the generated '
                            'header.')
@@ -1614,20 +1586,23 @@ See SampleForTests.java for more details.
       'in @JniNatives interface. And uses a shorter name and package'
       ' than GEN_JNI.')
   options, args = option_parser.parse_args(argv)
+  input_files = build_utils.ParseGnList(options.input_files)
+  output_files = build_utils.ParseGnList(options.output_files)
+
+  # Prevent stale file issues when moving file locations (crbug.com/951701).
+  if options.output_dir and os.path.exists(options.output_dir):
+    build_utils.DeleteDirectory(options.output_dir)
+    build_utils.MakeDirectory(options.output_dir)
+
+  with build_utils.TempDir() as temp_dir:
     if options.jar_file:
-    input_file = ExtractJarInputFile(options.jar_file, options.input_file,
-                                     options.output_dir)
-  elif options.input_file:
-    input_file = options.input_file
-  else:
-    option_parser.print_help()
-    print('\nError: Must specify --jar_file or --input_file.')
-    return 1
-  output_file = None
-  if options.output_dir:
-    root_name = os.path.splitext(os.path.basename(input_file))[0]
-    output_file = os.path.join(options.output_dir, root_name) + '_jni.h'
-  GenerateJNIHeader(input_file, output_file, options)
+      with zipfile.ZipFile(options.jar_file) as z:
+        z.extractall(temp_dir, input_files)
+      input_files = [os.path.join(temp_dir, f) for f in input_files]
+
+    for java_path, header_path in itertools.izip_longest(
+        input_files, output_files):
+      GenerateJNIHeader(java_path, header_path, options)
 
 
 if __name__ == '__main__':

build/config/android/rules.gni

@@ -189,30 +189,14 @@ if (enable_java_templates) {
   import("//build/config/sanitizers/sanitizers.gni")
   import("//tools/grit/grit_rule.gni")
 
-  # Declare a jni target
-  #
-  # This target generates the native jni bindings for a set of .java files.
-  #
-  # See base/android/jni_generator/jni_generator.py for more info about the
-  # format of generating JNI bindings.
-  #
-  # Variables
-  #   sources: list of .java files to generate jni for
-  #   jni_package: subdirectory path for generated bindings
-  #
-  # Example
-  #   generate_jni("foo_jni") {
-  #     sources = [
-  #       "android/java/src/org/chromium/foo/Foo.java",
-  #       "android/java/src/org/chromium/foo/FooUtil.java",
-  #     ]
-  #     jni_package = "foo"
-  #   }
-  template("generate_jni") {
+  # JNI target implementation. See generate_jni or generate_jar_jni for usage.
+  template("generate_jni_impl") {
     set_sources_assignment_filter([])
     forward_variables_from(invoker, [ "testonly" ])
 
-    _base_output_dir = "${target_gen_dir}/${target_name}"
+    # TODO(crbug.com/963471): Change include paths to be relative to
+    # $root_gen_dir to prevent issues when moving JNI targets.
+    _base_output_dir = "${target_gen_dir}/${target_name}_"
     _package_output_dir = "${_base_output_dir}/${invoker.jni_package}"
     _jni_output_dir = "${_package_output_dir}/jni"
 
@@ -232,16 +216,13 @@ if (enable_java_templates) {
       ]
     }
 
-    _foreach_target_name = "${target_name}__jni_gen"
-    action_foreach_with_pydeps(_foreach_target_name) {
+    _jni_target_name = "${target_name}__jni"
+    action_with_pydeps(_jni_target_name) {
+      # The sources aren't compiled so don't check their dependencies.
+      check_includes = false
       script = "//base/android/jni_generator/jni_generator.py"
-      sources = invoker.sources
-      outputs = [
-        "${_jni_output_dir}/{{source_name_part}}_jni.h",
-      ]
-
+      inputs = []
       args = [
-        "--input_file={{source}}",
         "--ptr_type=long",
         "--output_dir",
         rebase_path(_jni_output_dir, root_build_dir),
@@ -249,23 +230,54 @@ if (enable_java_templates) {
         rebase_path(_jni_generator_include, _jni_output_dir),
       ]
 
+      if (defined(invoker.classes)) {
+        if (defined(invoker.jar_file)) {
+          _jar_file = invoker.jar_file
+        } else {
+          _jar_file = android_sdk_jar
+        }
+        inputs += [ _jar_file ]
+        args += [
+          "--jar_file",
+          rebase_path(_jar_file, root_build_dir),
+          "--input_files=${invoker.classes}",
+        ]
+        _input_names = invoker.classes
+        if (defined(invoker.always_mangle) && invoker.always_mangle) {
+          args += [ "--always_mangle" ]
+        }
+      } else {
+        assert(defined(invoker.sources))
+        inputs += invoker.sources
+        _rebased_sources = rebase_path(invoker.sources, root_build_dir)
+        args += [ "--input_files=$_rebased_sources" ]
+        _input_names = invoker.sources
         if (use_hashed_jni_names) {
           args += [ "--use_proxy_hash" ]
         }
+        if (defined(invoker.namespace)) {
+          args += [ "-n ${invoker.namespace}" ]
+        }
+      }
+
+      outputs = []
+      foreach(_name, _input_names) {
+        _name_part = get_path_info(_name, "name")
+        outputs += [ "${_jni_output_dir}/${_name_part}_jni.h" ]
+      }
+      _rebased_outputs = rebase_path(outputs, root_build_dir)
+      args += [ "--output_files=$_rebased_outputs" ]
 
       if (enable_profiling) {
         args += [ "--enable_profiling" ]
       }
-      if (defined(invoker.namespace)) {
-        args += [ "-n ${invoker.namespace}" ]
-      }
       if (enable_jni_tracing) {
         args += [ "--enable_tracing" ]
       }
     }
 
     config("jni_includes_${target_name}") {
-      # TODO(cjhopman): #includes should probably all be relative to
+      # TODO: #includes should probably all be relative to
       # _base_output_dir. Remove that from this config once the includes are
       # updated.
       include_dirs = [
@@ -284,12 +296,38 @@ if (enable_java_templates) {
       if (!defined(public_deps)) {
         public_deps = []
       }
-      public_deps += [ ":$_foreach_target_name" ]
+      public_deps += [ ":$_jni_target_name" ]
       public_deps += _jni_generator_include_deps
       public_configs = [ ":jni_includes_${target_name}" ]
     }
   }
 
+  # Declare a jni target
+  #
+  # This target generates the native jni bindings for a set of .java files.
+  #
+  # See base/android/jni_generator/jni_generator.py for more info about the
+  # format of generating JNI bindings.
+  #
+  # Variables
+  #   sources: list of .java files to generate jni for
+  #   jni_package: subdirectory path for generated bindings
+  #   namespace: Specify the namespace for the generated header file.
+  #
+  # Example
+  #   generate_jni("foo_jni") {
+  #     sources = [
+  #       "android/java/src/org/chromium/foo/Foo.java",
+  #       "android/java/src/org/chromium/foo/FooUtil.java",
+  #     ]
+  #     jni_package = "foo"
+  #   }
+  template("generate_jni") {
+    generate_jni_impl(target_name) {
+      forward_variables_from(invoker, "*")
+    }
+  }
+
   # Declare a jni target for a prebuilt jar
   #
   # This target generates the native jni bindings for a set of classes in a .jar.
@@ -316,82 +354,8 @@ if (enable_java_templates) {
   #     jni_package = "foo"
   #   }
   template("generate_jar_jni") {
-    forward_variables_from(invoker, [ "testonly" ])
-
-    if (defined(invoker.jar_file)) {
-      _jar_file = invoker.jar_file
-    } else {
-      _jar_file = android_sdk_jar
-    }
-
-    _always_mangle = defined(invoker.always_mangle) && invoker.always_mangle
-
-    _base_output_dir = "${target_gen_dir}/${target_name}/${invoker.jni_package}"
-    _jni_output_dir = "${_base_output_dir}/jni"
-
-    if (defined(invoker.jni_generator_include)) {
-      _jni_generator_include = invoker.jni_generator_include
-    } else {
-      _jni_generator_include =
-          "//base/android/jni_generator/jni_generator_helper.h"
-    }
-
-    # TODO(cjhopman): make jni_generator.py support generating jni for multiple
-    # .class files from a .jar.
-    _jni_actions = []
-    foreach(_class, invoker.classes) {
-      _classname = get_path_info(_class, "name")
-      _jni_target_name = "${target_name}__jni_${_classname}"
-      _jni_actions += [ ":$_jni_target_name" ]
-      action_with_pydeps(_jni_target_name) {
-        # The sources aren't compiled so don't check their dependencies.
-        check_includes = false
-        script = "//base/android/jni_generator/jni_generator.py"
-        inputs = [
-          _jar_file,
-        ]
-        outputs = [
-          "${_jni_output_dir}/${_classname}_jni.h",
-        ]
-
-        args = [
-          "--jar_file",
-          rebase_path(_jar_file, root_build_dir),
-          "--input_file",
-          _class,
-          "--ptr_type=long",
-          "--output_dir",
-          rebase_path(_jni_output_dir, root_build_dir),
-          "--includes",
-          rebase_path(_jni_generator_include, _jni_output_dir),
-        ]
-
-        if (enable_profiling) {
-          args += [ "--enable_profiling" ]
-        }
-        if (enable_jni_tracing) {
-          args += [ "--enable_tracing" ]
-        }
-        if (_always_mangle) {
-          args += [ "--always_mangle" ]
-        }
-      }
-    }
-
-    config("jni_includes_${target_name}") {
-      include_dirs = [ _base_output_dir ]
-    }
-
-    group(target_name) {
-      public_deps = []
-      forward_variables_from(invoker,
-                             [
-                               "deps",
-                               "public_deps",
-                               "visibility",
-                             ])
-      public_deps += _jni_actions
-      public_configs = [ ":jni_includes_${target_name}" ]
+    generate_jni_impl(target_name) {
+      forward_variables_from(invoker, "*")
     }
   }