JNI refactor: @NativeMethods conversion (HttpNegotiateAuthenticator).

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: Ife3b8bee26e0da433341b138e7c1837d20cc4a60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832623
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Auto-Submit: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701583}

net/android/java/src/org/chromium/net/HttpNegotiateAuthenticator.java

@@ -29,6 +29,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.io.IOException;
 
@@ -101,7 +102,8 @@ public class HttpNegotiateAuthenticator {
                 accounts = future.getResult();
             } catch (OperationCanceledException | AuthenticatorException | IOException e) {
                 Log.w(TAG, "ERR_UNEXPECTED: Error while attempting to retrieve accounts.", e);
-                nativeSetResult(mRequestData.nativeResultObject, NetError.ERR_UNEXPECTED, null);
+                HttpNegotiateAuthenticatorJni.get().setResult(mRequestData.nativeResultObject,
+                        HttpNegotiateAuthenticator.this, NetError.ERR_UNEXPECTED, null);
                 return;
             }
 
@@ -109,8 +111,9 @@ public class HttpNegotiateAuthenticator {
                 Log.w(TAG, "ERR_MISSING_AUTH_CREDENTIALS: No account provided for the kerberos "
                                 + "authentication. Please verify the configuration policies and "
                                 + "that the CONTACTS runtime permission is granted. ");
-                nativeSetResult(mRequestData.nativeResultObject,
-                        NetError.ERR_MISSING_AUTH_CREDENTIALS, null);
+                HttpNegotiateAuthenticatorJni.get().setResult(mRequestData.nativeResultObject,
+                        HttpNegotiateAuthenticator.this, NetError.ERR_MISSING_AUTH_CREDENTIALS,
+                        null);
                 return;
             }
 
@@ -119,8 +122,9 @@ public class HttpNegotiateAuthenticator {
                                 + "kerberos authentication. Please fix the configuration by "
                                 + "providing a single account.",
                         accounts.length);
-                nativeSetResult(mRequestData.nativeResultObject,
-                        NetError.ERR_MISSING_AUTH_CREDENTIALS, null);
+                HttpNegotiateAuthenticatorJni.get().setResult(mRequestData.nativeResultObject,
+                        HttpNegotiateAuthenticator.this, NetError.ERR_MISSING_AUTH_CREDENTIALS,
+                        null);
                 return;
             }
 
@@ -131,7 +135,8 @@ public class HttpNegotiateAuthenticator {
                 // API >= 23 USE_CREDENTIALS permission is removed
                 Log.e(TAG, "ERR_MISCONFIGURED_AUTH_ENVIRONMENT: USE_CREDENTIALS permission not "
                                 + "granted. Aborting authentication.");
-                nativeSetResult(mRequestData.nativeResultObject,
+                HttpNegotiateAuthenticatorJni.get().setResult(mRequestData.nativeResultObject,
+                        HttpNegotiateAuthenticator.this,
                         NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT, null);
                 return;
             }
@@ -157,7 +162,8 @@ public class HttpNegotiateAuthenticator {
                 result = future.getResult();
             } catch (OperationCanceledException | AuthenticatorException | IOException e) {
                 Log.w(TAG, "ERR_UNEXPECTED: Error while attempting to obtain a token.", e);
-                nativeSetResult(mRequestData.nativeResultObject, NetError.ERR_UNEXPECTED, null);
+                HttpNegotiateAuthenticatorJni.get().setResult(mRequestData.nativeResultObject,
+                        HttpNegotiateAuthenticator.this, NetError.ERR_UNEXPECTED, null);
                 return;
             }
 
@@ -206,7 +212,8 @@ public class HttpNegotiateAuthenticator {
 
     /**
      * @param nativeResultObject The C++ object used to return the result. For correct C++ memory
-     *            management we must call nativeSetResult precisely once with this object.
+     *            management we must call HttpNegotiateAuthenticatorJni.get().setResult precisely
+     * once with this object.
      * @param principal The principal (must be host based).
      * @param authToken The incoming auth token.
      * @param canDelegate True if we can delegate.
@@ -286,7 +293,8 @@ public class HttpNegotiateAuthenticator {
             default:
                 status = NetError.ERR_UNEXPECTED;
         }
-        nativeSetResult(requestData.nativeResultObject, status,
+        HttpNegotiateAuthenticatorJni.get().setResult(requestData.nativeResultObject,
+                HttpNegotiateAuthenticator.this, status,
                 result.getString(AccountManager.KEY_AUTHTOKEN));
     }
 
@@ -310,8 +318,9 @@ public class HttpNegotiateAuthenticator {
             //           don't require it on M+
             Log.e(TAG, "ERR_MISCONFIGURED_AUTH_ENVIRONMENT: GET_ACCOUNTS permission not "
                             + "granted. Aborting authentication.");
-            nativeSetResult(requestData.nativeResultObject,
-                    NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT, null);
+            HttpNegotiateAuthenticatorJni.get().setResult(requestData.nativeResultObject,
+                    HttpNegotiateAuthenticator.this, NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT,
+                    null);
             return;
         }
         requestData.accountManager.getAccountsByTypeAndFeatures(mAccountType, features,
@@ -349,8 +358,9 @@ public class HttpNegotiateAuthenticator {
         if (lacksPermission(ctx, permission, isPreM)) {
             Log.e(TAG, "ERR_MISCONFIGURED_AUTH_ENVIRONMENT: %s permission not granted. "
                        + "Aborting authentication", permission);
-            nativeSetResult(requestData.nativeResultObject,
-                    NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT, null);
+            HttpNegotiateAuthenticatorJni.get().setResult(requestData.nativeResultObject,
+                    HttpNegotiateAuthenticator.this, NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT,
+                    null);
             return;
         }
 
@@ -372,7 +382,9 @@ public class HttpNegotiateAuthenticator {
         return permissionResult != PackageManager.PERMISSION_GRANTED;
     }
 
-    @VisibleForTesting
-    native void nativeSetResult(
-            long nativeJavaNegotiateResultWrapper, int status, String authToken);
+    @NativeMethods
+    interface Natives {
+        void setResult(long nativeJavaNegotiateResultWrapper, HttpNegotiateAuthenticator caller,
+                int status, String authToken);
+    }
 }

net/android/junit/src/org/chromium/net/HttpNegotiateAuthenticatorTest.java

@@ -9,17 +9,11 @@ import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
-import static org.mockito.AdditionalMatchers.or;
 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.eq;
 import static org.mockito.ArgumentMatchers.isNull;
-import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -41,6 +35,7 @@ import android.os.Handler;
 
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -58,6 +53,7 @@ import org.robolectric.shadows.multidex.ShadowMultiDex;
 
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
 import org.chromium.net.HttpNegotiateAuthenticator.GetAccountsCallback;
 import org.chromium.net.HttpNegotiateAuthenticator.RequestData;
 
@@ -84,8 +80,12 @@ public class HttpNegotiateAuthenticatorTest {
         }
     }
 
+    @Rule
+    public JniMocker mocker = new JniMocker();
     @Mock
     private static AccountManager sMockAccountManager;
+    @Mock
+    private HttpNegotiateAuthenticator.Natives mAuthenticatorJniMock;
     @Captor
     private ArgumentCaptor<AccountManagerCallback<Bundle>> mBundleCallbackCaptor;
     @Captor
@@ -96,6 +96,7 @@ public class HttpNegotiateAuthenticatorTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+        mocker.mock(HttpNegotiateAuthenticatorJni.TEST_HOOKS, mAuthenticatorJniMock);
     }
 
     /**
@@ -104,7 +105,7 @@ public class HttpNegotiateAuthenticatorTest {
     @Test
     public void testGetNextAuthToken() {
         final String accountType = "Dummy_Account";
-        HttpNegotiateAuthenticator authenticator = createWithoutNative(accountType);
+        HttpNegotiateAuthenticator authenticator = createAuthenticator(accountType);
         Robolectric.buildActivity(Activity.class).create().start().resume().visible();
 
         authenticator.getNextAuthToken(0, "test_principal", "", true);
@@ -138,7 +139,7 @@ public class HttpNegotiateAuthenticatorTest {
     public void testGetNextAuthTokenWithoutActivity() {
         final String accountType = "Dummy_Account";
         final Account[] returnedAccount = {new Account("name", accountType)};
-        HttpNegotiateAuthenticator authenticator = createWithoutNative(accountType);
+        HttpNegotiateAuthenticator authenticator = createAuthenticator(accountType);
 
         authenticator.getNextAuthToken(1234, "test_principal", "", true);
 
@@ -174,7 +175,7 @@ public class HttpNegotiateAuthenticatorTest {
     @Test
     public void testGetAccountCallback() {
         String type = "Dummy_Account";
-        HttpNegotiateAuthenticator authenticator = createWithoutNative(type);
+        HttpNegotiateAuthenticator authenticator = createAuthenticator(type);
         RequestData requestData = new RequestData();
         requestData.nativeResultObject = 42;
         requestData.accountManager = sMockAccountManager;
@@ -182,9 +183,9 @@ public class HttpNegotiateAuthenticatorTest {
 
         // Should fail because there are no accounts
         callback.run(makeFuture(new Account[]{}));
-        verify(authenticator)
-                .nativeSetResult(
-                        eq(42L), eq(NetError.ERR_MISSING_AUTH_CREDENTIALS), (String) isNull());
+        verify(mAuthenticatorJniMock)
+                .setResult(eq(42L), eq(authenticator), eq(NetError.ERR_MISSING_AUTH_CREDENTIALS),
+                        (String) isNull());
 
         // Should succeed, for a single account we use it for the AccountManager#getAuthToken call.
         Account testAccount = new Account("a", type);
@@ -195,9 +196,9 @@ public class HttpNegotiateAuthenticatorTest {
 
         // Should fail because there is more than one account
         callback.run(makeFuture(new Account[]{new Account("a", type), new Account("b", type)}));
-        verify(authenticator, times(2))
-                .nativeSetResult(
-                        eq(42L), eq(NetError.ERR_MISSING_AUTH_CREDENTIALS), (String) isNull());
+        verify(mAuthenticatorJniMock, times(2))
+                .setResult(eq(42L), eq(authenticator), eq(NetError.ERR_MISSING_AUTH_CREDENTIALS),
+                        (String) isNull());
     }
 
     /**
@@ -207,7 +208,7 @@ public class HttpNegotiateAuthenticatorTest {
     @Test
     public void testGetTokenCallbackWithIntent() {
         String type = "Dummy_Account";
-        HttpNegotiateAuthenticator authenticator = createWithoutNative(type);
+        HttpNegotiateAuthenticator authenticator = createAuthenticator(type);
         RequestData requestData = new RequestData();
         requestData.nativeResultObject = 42;
         requestData.authTokenType = "foo";
@@ -240,7 +241,7 @@ public class HttpNegotiateAuthenticatorTest {
      */
     @Test
     public void testAccountManagerCallbackRun() {
-        HttpNegotiateAuthenticator authenticator = createWithoutNative("Dummy_Account");
+        HttpNegotiateAuthenticator authenticator = createAuthenticator("Dummy_Account");
 
         Robolectric.buildActivity(Activity.class).create().start().resume().visible();
 
@@ -258,7 +259,7 @@ public class HttpNegotiateAuthenticatorTest {
         resultBundle.putBundle(HttpNegotiateConstants.KEY_SPNEGO_CONTEXT, context);
         resultBundle.putString(AccountManager.KEY_AUTHTOKEN, "output_token");
         mBundleCallbackCaptor.getValue().run(makeFuture(resultBundle));
-        verify(authenticator).nativeSetResult(1234, 0, "output_token");
+        verify(mAuthenticatorJniMock).setResult(1234, authenticator, 0, "output_token");
 
         // Check that the next call to getNextAuthToken uses the correct context
         authenticator.getNextAuthToken(5678, "test_principal", "", true);
@@ -274,22 +275,18 @@ public class HttpNegotiateAuthenticatorTest {
         // Test exception path
         mBundleCallbackCaptor.getValue().run(
                 this.<Bundle>makeFuture(new OperationCanceledException()));
-        verify(authenticator).nativeSetResult(5678, NetError.ERR_UNEXPECTED, null);
+        verify(mAuthenticatorJniMock).setResult(5678, authenticator, NetError.ERR_UNEXPECTED, null);
     }
 
     @Test
     public void testPermissionDenied() {
         Robolectric.buildActivity(Activity.class).create().start().resume().visible();
-        HttpNegotiateAuthenticator authenticator = createWithoutNative("Dummy_Account");
-
-        doReturn(true)
-                .when(authenticator)
-                .lacksPermission(any(Context.class), any(String.class), anyBoolean());
+        HttpNegotiateAuthenticator authenticator = createAuthenticator("Dummy_Account", true);
 
         authenticator.getNextAuthToken(1234, "test_principal", "", true);
-        verify(authenticator)
-                .nativeSetResult(anyLong(), eq(NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT),
-                        (String) isNull());
+        verify(mAuthenticatorJniMock)
+                .setResult(anyLong(), eq(authenticator),
+                        eq(NetError.ERR_MISCONFIGURED_AUTH_ENVIRONMENT), (String) isNull());
     }
 
     @Test
@@ -367,7 +364,7 @@ public class HttpNegotiateAuthenticatorTest {
     }
 
     private void checkErrorReturn(Integer spnegoError, int expectedError) {
-        HttpNegotiateAuthenticator authenticator = createWithoutNative("Dummy_Account");
+        HttpNegotiateAuthenticator authenticator = createAuthenticator("Dummy_Account");
 
         // Call getNextAuthToken to get the callback
         authenticator.getNextAuthToken(1234, "test_principal", "", true);
@@ -381,7 +378,8 @@ public class HttpNegotiateAuthenticatorTest {
             resultBundle.putInt(HttpNegotiateConstants.KEY_SPNEGO_RESULT, spnegoError);
         }
         mBundleCallbackCaptor.getValue().run(makeFuture(resultBundle));
-        verify(authenticator).nativeSetResult(anyLong(), eq(expectedError), (String) isNull());
+        verify(mAuthenticatorJniMock)
+                .setResult(anyLong(), eq(authenticator), eq(expectedError), (String) isNull());
     }
 
     /**
@@ -421,18 +419,19 @@ public class HttpNegotiateAuthenticatorTest {
     }
 
     /**
-     * Returns a new authenticator as a spy so that we can override and intercept the native method
-     * calls.
+     * Returns a new authenticator with an overridden lacksPermission method.
      */
-    private HttpNegotiateAuthenticator createWithoutNative(String accountType) {
-        HttpNegotiateAuthenticator authenticator =
-                spy(HttpNegotiateAuthenticator.create(accountType));
-        doNothing()
-                .when(authenticator)
-                .nativeSetResult(anyLong(), anyInt(), or(any(String.class), (String) isNull()));
-        doReturn(false)
-                .when(authenticator)
-                .lacksPermission(any(Context.class), any(String.class), anyBoolean());
-        return authenticator;
+    private HttpNegotiateAuthenticator createAuthenticator(
+            String accountType, boolean lacksPermission) {
+        return new HttpNegotiateAuthenticator(accountType) {
+            @Override
+            boolean lacksPermission(Context context, String permission, boolean onlyPreM) {
+                return lacksPermission;
+            }
+        };
+    }
+
+    private HttpNegotiateAuthenticator createAuthenticator(String accountType) {
+        return createAuthenticator(accountType, false);
     }
 }