JNI: Add flags to enable/disable/require mocks in GEN_JNI

Adds the REQUIRE_MOCKS flag to GEN_JNI. If both the REQUIRE_MOCKS and
TESTING_ENABLED flags are set, calls to native implementations must
have had a mock instance set first through TEST_HOOKS,
otherwise an UnsupportedOperationException will be thrown.

If only TESTING_ENABLED is set, calls to the native implementation
will use a test mock if it is provided, and otherwise fallback to the
native implementation. This is used for instrumentation tests that
may want to mock out some native implementations but not all.

In the annotation processor version of GEN_JNI both these flags are
mutable and so they can be set by a JNI test rule for unit tests.

Instrumentation tests that want to mock a native implementation
can pass flags to JNI registration generator to set the corresponding
GEN_JNI flags.

Bug: 898261
Change-Id: I6a90e91a29368df692d458f35c9ea92f8898064b
Reviewed-on: https://chromium-review.googlesource.com/c/1344554
Commit-Queue: Aiden Benner <abenner@google.com>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610263}

base/android/jni_generator/HashedSampleForAnnotationProcessorGenJni.golden

@@ -10,6 +10,7 @@ package org.chromium.base.natives;
 
 public class GEN_JNI {
   public static final boolean TESTING_ENABLED = false;
+  public static final boolean REQUIRE_MOCK = false;
 
   public static native void org_chromium_example_jni_1generator_SampleForAnnotationProcessor_foo();
 

base/android/jni_generator/java/src/org/chromium/jni_generator/JniProcessor.java

@@ -65,11 +65,19 @@ public class JniProcessor extends AbstractProcessor {
 
     static final String NATIVE_CLASS_NAME_STR = "GEN_JNI";
     static final String NATIVE_CLASS_PACKAGE_NAME = "org.chromium.base.natives";
+
     static final ClassName NATIVE_CLASS_NAME =
             ClassName.get(NATIVE_CLASS_PACKAGE_NAME, NATIVE_CLASS_NAME_STR);
+
     static final String NATIVE_TEST_FIELD_NAME = "TESTING_ENABLED";
+    static final String NATIVE_REQUIRE_MOCK_FIELD_NAME = "REQUIRE_MOCK";
+
+    // Lets mocks of the Native impl to be set.
     static final boolean TESTING_ENABLED = false;
 
+    // If true, throw an exception if no mock is provided.
+    private static final boolean REQUIRE_MOCK = false;
+
     // Builder for NativeClass which will hold all our native method declarations.
     private TypeSpec.Builder mNativesBuilder;
 
@@ -107,15 +115,24 @@ public class JniProcessor extends AbstractProcessor {
         FieldSpec.Builder testingFlagBuilder =
                 FieldSpec.builder(TypeName.BOOLEAN, NATIVE_TEST_FIELD_NAME)
                         .addModifiers(Modifier.STATIC, Modifier.PUBLIC);
+        FieldSpec.Builder throwFlagBuilder =
+                FieldSpec.builder(TypeName.BOOLEAN, NATIVE_REQUIRE_MOCK_FIELD_NAME)
+                        .addModifiers(Modifier.STATIC, Modifier.PUBLIC);
+
+        // Initialize only if true to avoid NoRedundantFieldInit.
         if (TESTING_ENABLED) {
             testingFlagBuilder.initializer("true");
         }
+        if (REQUIRE_MOCK) {
+            throwFlagBuilder.initializer("true");
+        }
 
         // State of mNativesBuilder needs to be preserved between processing rounds.
         mNativesBuilder = TypeSpec.classBuilder(NATIVE_CLASS_NAME)
                                   .addAnnotation(createGeneratedAnnotation())
                                   .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
-                                  .addField(testingFlagBuilder.build());
+                                  .addField(testingFlagBuilder.build())
+                                  .addField(throwFlagBuilder.build());
 
         try {
             sNativeMethodHashFunction = MessageDigest.getInstance("MD5");
@@ -323,7 +340,6 @@ public class JniProcessor extends AbstractProcessor {
                                            .addSuperinterface(nativeInterfaceType)
                                            .addModifiers(Modifier.FINAL)
                                            .addAnnotation(createGeneratedAnnotation());
-
         if (isPublic) {
             builder.addModifiers(Modifier.PUBLIC);
         }
@@ -356,13 +372,38 @@ public class JniProcessor extends AbstractProcessor {
         builder.addField(testTarget);
 
         // Getter for target or testing instance if flag in GEN_JNI is set.
+        /*
+        {classname}.Natives get() {
+            if (GEN_JNI.TESTING_ENABLED) {
+                if (testInst != null) {
+                    return testInst;
+                }
+                if (GEN_JNI.REQUIRE_MOCK) {
+                    throw new UnsupportedOperationException($noMockExceptionString);
+                }
+            }
+            return new {classname}Jni();
+        }
+         */
+        String noMockExceptionString =
+                String.format("No mock found for the native implementation for %s. "
+                                + "The current configuration requires all native "
+                                + "implementations to have a mock instance.",
+                        nativeInterfaceType);
         MethodSpec instanceGetter =
                 MethodSpec.methodBuilder("get")
                         .addModifiers(Modifier.PUBLIC, Modifier.STATIC)
                         .returns(nativeInterfaceType)
                         .beginControlFlow("if ($T.$N)", NATIVE_CLASS_NAME, NATIVE_TEST_FIELD_NAME)
+                        .beginControlFlow("if ($N != null)", testTarget)
                         .addStatement("return $N", testTarget)
                         .endControlFlow()
+                        .beginControlFlow(
+                                "if ($T.$N)", NATIVE_CLASS_NAME, NATIVE_REQUIRE_MOCK_FIELD_NAME)
+                        .addStatement("throw new UnsupportedOperationException($S)",
+                                noMockExceptionString)
+                        .endControlFlow()
+                        .endControlFlow()
                         .addStatement("return new $N()", name)
                         .build();
 

base/android/jni_generator/jni_generator_tests.py

@@ -338,8 +338,9 @@ class TestGenerator(unittest.TestCase):
                                                     natives, jni_params, False)
     content = TestGenerator._MergeRegistrationForTests([h2.Generate()])
 
+    proxy_opts = jni_registration_generator.ProxyOptions()
     self.assertGoldenTextEquals(
-        jni_registration_generator.CreateProxyJavaFromDict(content),
+        jni_registration_generator.CreateProxyJavaFromDict(content, proxy_opts),
         suffix='Java')
 
     self.assertGoldenTextEquals(
@@ -1115,8 +1116,10 @@ public class java.util.HashSet {
     reg_dict = jni_registration_generator._DictForPath(path)
     reg_dict = self._MergeRegistrationForTests([reg_dict])
 
+    proxy_opts = jni_registration_generator.ProxyOptions()
     self.assertGoldenTextEquals(
-        jni_registration_generator.CreateProxyJavaFromDict(reg_dict),
+        jni_registration_generator.CreateProxyJavaFromDict(reg_dict,
+                                                           proxy_opts),
         golden_file='HashedSampleForAnnotationProcessorGenJni.golden')
 
   def testJniProxyExample(self):

base/android/jni_generator/jni_registration_generator.py

@@ -34,10 +34,9 @@ MERGEABLE_KEYS = [
     'REGISTER_NON_MAIN_DEX_NATIVES',
 ]
 
-
 def _Generate(java_file_paths,
               srcjar_path,
-              use_proxy_hash=False,
+              proxy_opts,
               header_path=None,
               namespace=''):
   """Generates files required to perform JNI registration.
@@ -61,7 +60,7 @@ def _Generate(java_file_paths,
 
   results = []
   for d in pool.imap_unordered(
-      functools.partial(_DictForPath, use_proxy_hash=use_proxy_hash),
+      functools.partial(_DictForPath, use_proxy_hash=proxy_opts.use_hash),
       java_file_paths):
     if d:
       results.append(d)
@@ -87,7 +86,7 @@ def _Generate(java_file_paths,
       build_utils.AddToZipHermetic(
           srcjar,
           'org/chromium/base/natives/GEN_JNI.java',
-          data=CreateProxyJavaFromDict(combined_dict))
+          data=CreateProxyJavaFromDict(combined_dict, proxy_opts))
 
 
 def _DictForPath(path, use_proxy_hash=False):
@@ -181,7 +180,7 @@ JNI_REGISTRATION_EXPORT bool ${REGISTRATION_NAME}(JNIEnv* env) {
   registration_dict['REGISTER_MAIN_DEX_PROXY_NATIVES'] = main_dex_call
 
 
-def CreateProxyJavaFromDict(registration_dict):
+def CreateProxyJavaFromDict(registration_dict, proxy_opts):
   template = string.Template("""\
 // Copyright 2018 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
@@ -194,13 +193,16 @@ package ${PACKAGE};
 // Please do not change its content.
 
 public class ${CLASS_NAME} {
-  public static final boolean TESTING_ENABLED = false;
+  public static final boolean TESTING_ENABLED = ${TESTING_ENABLED};
+  public static final boolean REQUIRE_MOCK = ${REQUIRE_MOCK};
 ${SIGNATURES}
 
 }
 """)
 
   return template.substitute({
+      'TESTING_ENABLED': str(proxy_opts.enable_mocks).lower(),
+      'REQUIRE_MOCK': str(proxy_opts.require_mocks).lower(),
       'CLASS_NAME': jni_generator.NATIVE_PROXY_CLASS_NAME,
       'PACKAGE': jni_generator.NATIVE_PROXY_PACKAGE_NAME.replace('/', '.'),
       'SIGNATURES': registration_dict['PROXY_NATIVE_SIGNATURES']
@@ -503,6 +505,16 @@ def _MakeProxySignature(proxy_native):
   })
 
 
+class ProxyOptions:
+
+  def __init__(self, **kwargs):
+    self.use_hash = kwargs.get('use_hash', False)
+    self.enable_mocks = kwargs.get('enable_mocks', False)
+    self.require_mocks = kwargs.get('require_mocks', False)
+    # Can never require and disable.
+    assert self.enable_mocks or not self.require_mocks
+
+
 def main(argv):
   arg_parser = argparse.ArgumentParser()
   build_utils.AddDepfileOption(arg_parser)
@@ -527,14 +539,38 @@ def main(argv):
                           default='',
                           help='Namespace to wrap the registration functions '
                           'into.')
+  # TODO(crbug.com/898261) hook these flags up to the build config to enable
+  # mocking in instrumentation tests
+  arg_parser.add_argument(
+      '--enable_proxy_mocks',
+      default=False,
+      action='store_true',
+      help='Allows proxy native impls to be mocked through Java.')
+  arg_parser.add_argument(
+      '--require_mocks',
+      default=False,
+      action='store_true',
+      help='Requires all used native implementations to have a mock set when '
+           'called. Otherwise an exception will be thrown.')
   arg_parser.add_argument(
       '--use_proxy_hash',
       action='store_true',
-      help='Enables hashing of the native declaration '
-      'for methods in an @JniNatives interface')
+      help='Enables hashing of the native declaration for methods in '
+           'an @JniNatives interface')
   args = arg_parser.parse_args(build_utils.ExpandFileArgs(argv[1:]))
+
+  if not args.enable_proxy_mocks and args.require_mocks:
+    arg_parser.error(
+        'Invalid arguments: --require_mocks without --enable_proxy_mocks. '
+        'Cannot require mocks if they are not enabled.')
+
   args.sources_files = build_utils.ParseGnList(args.sources_files)
 
+  proxy_opts = ProxyOptions(
+      use_hash=args.use_proxy_hash,
+      require_mocks=args.require_mocks,
+      enable_mocks=args.enable_proxy_mocks)
+
   java_file_paths = []
   for f in args.sources_files:
     # java_file_paths stores each Java file path as a string.
@@ -545,7 +581,7 @@ def main(argv):
   _Generate(
       java_file_paths,
       args.srcjar_path,
-      use_proxy_hash=args.use_proxy_hash,
+      proxy_opts=proxy_opts,
       header_path=args.header_path,
       namespace=args.namespace)
 

base/android/jni_generator/testProxyNativesJava.golden

@@ -10,6 +10,7 @@ package org.chromium.base.natives;
 
 public class GEN_JNI {
   public static final boolean TESTING_ENABLED = false;
+  public static final boolean REQUIRE_MOCK = false;
 
   public static native void org_chromium_example_SampleProxyJni_foo();