JNI refactor: @NativeMethods conversion (//components/policy).

This CL was partially created by
//base/android/jni_generator/jni_refactorer.py.

The conversion also required converting unit tests to use the new JNI
mocking approach provided by JniMocker.

Bug: 929661
Change-Id: If47b5ba988f61e4b179d483fb0073e11f0d50992
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831416Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701307}

components/policy/android/java/src/org/chromium/policy/CombinedPolicyProvider.java

@@ -10,6 +10,7 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.NativeMethods;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -92,7 +93,7 @@ public class CombinedPolicyProvider {
                 mPolicyConverter.setPolicy(key, settings.get(key));
             }
         }
-        nativeFlushPolicies(mNativeCombinedPolicyProvider);
+        CombinedPolicyProviderJni.get().flushPolicies(mNativeCombinedPolicyProvider, get());
     }
 
     void terminateIncognitoSession() {
@@ -132,14 +133,12 @@ public class CombinedPolicyProvider {
         void terminateIncognitoSession();
     }
 
-    @VisibleForTesting
-    public static void set(CombinedPolicyProvider p) {
+    static void setForTesting(CombinedPolicyProvider p) {
         sInstance = p;
     }
 
-    @VisibleForTesting
-    CombinedPolicyProvider() {}
-
-    @VisibleForTesting
-    protected native void nativeFlushPolicies(long nativeAndroidCombinedPolicyProvider);
+    @NativeMethods
+    interface Natives {
+        void flushPolicies(long nativeAndroidCombinedPolicyProvider, CombinedPolicyProvider caller);
+    }
 }

components/policy/android/java/src/org/chromium/policy/PolicyConverter.java

@@ -16,6 +16,7 @@ import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.NativeMethods;
 
 import java.util.Arrays;
 import java.util.Set;
@@ -44,19 +45,23 @@ public class PolicyConverter {
         assert mNativePolicyConverter != 0;
 
         if (value instanceof Boolean) {
-            nativeSetPolicyBoolean(mNativePolicyConverter, key, (Boolean) value);
+            PolicyConverterJni.get().setPolicyBoolean(
+                    mNativePolicyConverter, PolicyConverter.this, key, (Boolean) value);
             return;
         }
         if (value instanceof String) {
-            nativeSetPolicyString(mNativePolicyConverter, key, (String) value);
+            PolicyConverterJni.get().setPolicyString(
+                    mNativePolicyConverter, PolicyConverter.this, key, (String) value);
             return;
         }
         if (value instanceof Integer) {
-            nativeSetPolicyInteger(mNativePolicyConverter, key, (Integer) value);
+            PolicyConverterJni.get().setPolicyInteger(
+                    mNativePolicyConverter, PolicyConverter.this, key, (Integer) value);
             return;
         }
         if (value instanceof String[]) {
-            nativeSetPolicyStringArray(mNativePolicyConverter, key, (String[]) value);
+            PolicyConverterJni.get().setPolicyStringArray(
+                    mNativePolicyConverter, PolicyConverter.this, key, (String[]) value);
             return;
         }
         // App restrictions can only contain bundles and bundle arrays on Android M, however our
@@ -68,8 +73,8 @@ public class PolicyConverter {
                 // JNI can't take a Bundle argument without a lot of extra work, but the native code
                 // already accepts arbitrary JSON strings, so convert to JSON.
                 try {
-                    nativeSetPolicyString(
-                            mNativePolicyConverter, key, convertBundleToJson(bundle).toString());
+                    PolicyConverterJni.get().setPolicyString(mNativePolicyConverter,
+                            PolicyConverter.this, key, convertBundleToJson(bundle).toString());
                 } catch (JSONException e) {
                     // Chrome requires all policies to be expressible as JSON, so this can't be a
                     // valid policy.
@@ -83,7 +88,8 @@ public class PolicyConverter {
                 // JNI can't take a Bundle[] argument without a lot of extra work, but the native
                 // code already accepts arbitrary JSON strings, so convert to JSON.
                 try {
-                    nativeSetPolicyString(mNativePolicyConverter, key,
+                    PolicyConverterJni.get().setPolicyString(mNativePolicyConverter,
+                            PolicyConverter.this, key,
                             convertBundleArrayToJson(bundleArray).toString());
                 } catch (JSONException e) {
                     // Chrome requires all policies to be expressible as JSON, so this can't be a
@@ -130,13 +136,15 @@ public class PolicyConverter {
         mNativePolicyConverter = 0;
     }
 
-    @VisibleForTesting
-    native void nativeSetPolicyBoolean(long nativePolicyConverter, String policyKey, boolean value);
-    @VisibleForTesting
-    native void nativeSetPolicyInteger(long nativePolicyConverter, String policyKey, int value);
-    @VisibleForTesting
-    native void nativeSetPolicyString(long nativePolicyConverter, String policyKey, String value);
-    @VisibleForTesting
-    native void nativeSetPolicyStringArray(
-            long nativePolicyConverter, String policyKey, String[] value);
+    @NativeMethods
+    interface Natives {
+        void setPolicyBoolean(long nativePolicyConverter, PolicyConverter caller, String policyKey,
+                boolean value);
+        void setPolicyInteger(
+                long nativePolicyConverter, PolicyConverter caller, String policyKey, int value);
+        void setPolicyString(
+                long nativePolicyConverter, PolicyConverter caller, String policyKey, String value);
+        void setPolicyStringArray(long nativePolicyConverter, PolicyConverter caller,
+                String policyKey, String[] value);
+    }
 }

components/policy/android/junit/src/org/chromium/policy/CombinedPolicyProviderTest.java

@@ -8,9 +8,7 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -20,11 +18,15 @@ import static org.mockito.Mockito.verify;
 import android.os.Bundle;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
 
 /**
  * Robolectric tests for CombinedPolicyProvider
@@ -32,16 +34,18 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
 public class CombinedPolicyProviderTest {
+    @Rule
+    public JniMocker mocker = new JniMocker();
+    @Mock
     private PolicyConverter mPolicyConverter;
+    @Mock
+    private CombinedPolicyProvider.Natives mCombinedPolicyConverterJniMock;
 
     @Before
     public void setup() {
-        // Stub out the native calls
-        CombinedPolicyProvider provider = spy(new CombinedPolicyProvider());
-        mPolicyConverter = mock(PolicyConverter.class);
-        doNothing().when(mPolicyConverter).setPolicy(anyString(), any());
-        doNothing().when(provider).nativeFlushPolicies(anyLong());
-        CombinedPolicyProvider.set(provider);
+        MockitoAnnotations.initMocks(this);
+        mocker.mock(CombinedPolicyProviderJni.TEST_HOOKS, mCombinedPolicyConverterJniMock);
+        CombinedPolicyProvider.setForTesting(new CombinedPolicyProvider());
     }
 
     /**
@@ -85,7 +89,7 @@ public class CombinedPolicyProviderTest {
         b.putBoolean("BoolPolicy", true);
         CombinedPolicyProvider.get().onSettingsAvailable(0, b);
         verify(mPolicyConverter, never()).setPolicy(anyString(), any());
-        verify(CombinedPolicyProvider.get(), never()).nativeFlushPolicies(anyInt());
+        verify(mCombinedPolicyConverterJniMock, never()).flushPolicies(anyInt(), any());
     }
 
     @Test
@@ -104,7 +108,7 @@ public class CombinedPolicyProviderTest {
         verify(mPolicyConverter).setPolicy("StringPolicy", "A string");
         verify(mPolicyConverter)
                 .setPolicy("StringArrayPolicy", new String[] {"String1", "String2"});
-        verify(CombinedPolicyProvider.get()).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock).flushPolicies(1234, CombinedPolicyProvider.get());
     }
 
     @Test
@@ -114,7 +118,7 @@ public class CombinedPolicyProviderTest {
         CombinedPolicyProvider.get().registerProvider(provider);
         Bundle b = new Bundle();
         CombinedPolicyProvider.get().onSettingsAvailable(0, b);
-        verify(CombinedPolicyProvider.get()).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock).flushPolicies(1234, CombinedPolicyProvider.get());
 
         // Second policy provider registered but no settings.
         PolicyProvider provider2 = new DummyPolicyProvider();
@@ -126,14 +130,15 @@ public class CombinedPolicyProviderTest {
         // Second call should have been ignored, so nothing should have been set
         verify(mPolicyConverter, never()).setPolicy(anyString(), anyBoolean());
         // and flush should have been called precisely once.
-        verify(CombinedPolicyProvider.get()).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock).flushPolicies(1234, CombinedPolicyProvider.get());
 
         // Empty but valid bundle from second policy provider should set the policy and push it
         // to the native code
         b = new Bundle();
         CombinedPolicyProvider.get().onSettingsAvailable(1, b);
         verify(mPolicyConverter).setPolicy("BoolPolicy", true);
-        verify(CombinedPolicyProvider.get(), times(2)).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock, times(2))
+                .flushPolicies(1234, CombinedPolicyProvider.get());
     }
 
     @Test
@@ -147,19 +152,22 @@ public class CombinedPolicyProviderTest {
         b.putBoolean("BoolPolicy", true);
         CombinedPolicyProvider.get().onSettingsAvailable(0, b);
         CombinedPolicyProvider.get().onSettingsAvailable(1, b);
-        verify(CombinedPolicyProvider.get(), times(1)).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock, times(1))
+                .flushPolicies(1234, CombinedPolicyProvider.get());
 
         CombinedPolicyProvider.get().refreshPolicies();
         // This should have cleared the cached policies, so onSettingsAvailable should now do
         // nothing until both providers have settings.
         CombinedPolicyProvider.get().onSettingsAvailable(0, b);
         // Still only one call.
-        verify(CombinedPolicyProvider.get(), times(1)).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock, times(1))
+                .flushPolicies(1234, CombinedPolicyProvider.get());
         b = new Bundle();
         b.putBoolean("BoolPolicy", false);
         CombinedPolicyProvider.get().onSettingsAvailable(1, b);
         // That should have caused the second flush.
-        verify(CombinedPolicyProvider.get(), times(2)).nativeFlushPolicies(1234);
+        verify(mCombinedPolicyConverterJniMock, times(2))
+                .flushPolicies(1234, CombinedPolicyProvider.get());
         // And the policy should have been set to the new value.
         verify(mPolicyConverter).setPolicy("BoolPolicy", false);
     }

components/policy/android/junit/src/org/chromium/policy/PolicyConverterTest.java

@@ -4,23 +4,21 @@
 
 package org.chromium.policy;
 
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
-import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
 import android.os.Build;
 import android.os.Bundle;
 
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
 
 /**
  * Robolectric test for AbstractAppRestrictionsProvider.
@@ -28,6 +26,18 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE, sdk = Build.VERSION_CODES.LOLLIPOP)
 public class PolicyConverterTest {
+    @Rule
+    public JniMocker mocker = new JniMocker();
+
+    @Mock
+    private PolicyConverter.Natives mPolicyConverterJniMock;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+        mocker.mock(PolicyConverterJni.TEST_HOOKS, mPolicyConverterJniMock);
+    }
+
     /**
      * Test method for
      * {@link org.chromium.policy.PolicyConverter#setPolicy(java.lang.String, java.lang.Object)}.
@@ -35,27 +45,18 @@ public class PolicyConverterTest {
     @Test
     public void testSetPolicy() {
         // Stub out the native methods.
-        PolicyConverter policyConverter = spy(PolicyConverter.create(1234));
-        doNothing()
-                .when(policyConverter)
-                .nativeSetPolicyBoolean(anyLong(), anyString(), anyBoolean());
-        doNothing().when(policyConverter).nativeSetPolicyInteger(anyLong(), anyString(), anyInt());
-        doNothing()
-                .when(policyConverter)
-                .nativeSetPolicyString(anyLong(), anyString(), anyString());
-        doNothing()
-                .when(policyConverter)
-                .nativeSetPolicyStringArray(anyLong(), anyString(), any(String[].class));
+        PolicyConverter policyConverter = PolicyConverter.create(1234);
 
         policyConverter.setPolicy("p1", true);
-        verify(policyConverter).nativeSetPolicyBoolean(1234, "p1", true);
+        verify(mPolicyConverterJniMock).setPolicyBoolean(1234, policyConverter, "p1", true);
         policyConverter.setPolicy("p1", 5678);
-        verify(policyConverter).nativeSetPolicyInteger(1234, "p1", 5678);
+        verify(mPolicyConverterJniMock).setPolicyInteger(1234, policyConverter, "p1", 5678);
         policyConverter.setPolicy("p1", "hello");
-        verify(policyConverter).nativeSetPolicyString(1234, "p1", "hello");
+        verify(mPolicyConverterJniMock).setPolicyString(1234, policyConverter, "p1", "hello");
         policyConverter.setPolicy("p1", new String[] {"hello", "goodbye"});
-        verify(policyConverter)
-                .nativeSetPolicyStringArray(1234, "p1", new String[] {"hello", "goodbye"});
+        verify(mPolicyConverterJniMock)
+                .setPolicyStringArray(
+                        1234, policyConverter, "p1", new String[] {"hello", "goodbye"});
         Bundle b1 = new Bundle();
         b1.putInt("i1", 23);
         b1.putString("s1", "a string");
@@ -65,11 +66,13 @@ public class PolicyConverterTest {
         ba[0].putString("ba1s", "another string");
         b1.putParcelableArray("b1b", ba);
         policyConverter.setPolicy("p1", b1);
-        verify(policyConverter)
-                .nativeSetPolicyString(1234, "p1", "{\"i1\":23,\"s1\":\"a string\","
+        verify(mPolicyConverterJniMock)
+                .setPolicyString(1234, policyConverter, "p1",
+                        "{\"i1\":23,\"s1\":\"a string\","
                                 + "\"b1b\":[{\"ba1b\":true,\"ba1s\":\"another string\"}]}");
         policyConverter.setPolicy("p1", ba);
-        verify(policyConverter)
-                .nativeSetPolicyString(1234, "p1", "[{\"ba1b\":true,\"ba1s\":\"another string\"}]");
+        verify(mPolicyConverterJniMock)
+                .setPolicyString(1234, policyConverter, "p1",
+                        "[{\"ba1b\":true,\"ba1s\":\"another string\"}]");
     }
 }