Refactor WebApkServiceConnectionManager.

chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkIdentityServiceClientTest.java

Index: chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkIdentityServiceClientTest.java
diff --git a/chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkIdentityServiceClientTest.java b/chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkIdentityServiceClientTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..7a7a663c7e02813304f85394447f992a7afe47b6
--- /dev/null
+++ b/chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkIdentityServiceClientTest.java
@@ -0,0 +1,160 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.webapk.lib.client;
+
+import static org.mockito.Mockito.when;
+
+import android.content.ComponentName;
+import android.content.Context;
+import android.content.Intent;
+import android.os.Bundle;
+import android.os.IBinder;
+import android.os.RemoteException;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.Shadows;
+import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowApplication;
+
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+import org.chromium.webapk.lib.common.WebApkMetaDataKeys;
+import org.chromium.webapk.lib.common.identity_service.IIdentityService;
+import org.chromium.webapk.test.WebApkTestHelper;
+
+/**
+ * Unit tests for {@link org.chromium.webapk.lib.client.WebApkIdentityServiceClient}.
+ */
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class WebApkIdentityServiceClientTest {
+    private static final String WEBAPK_PACKAGE_NAME = "org.chromium.webapk.test_package";
+    private static final String RUNTIME_HOST_IN_METADATA = "browser.in.webapk.metadata";
+    private static final String REAL_RUNTIME_HOST = "real.runtime.host.browser";
+    private ShadowApplication mShadowApplication;
+
+    @Before
+    public void setUp() {
+        mShadowApplication = Shadows.shadowOf(RuntimeEnvironment.application);
+    }
+
+    /**
+     * Tests that for WebAPKs with shell APK version lower than the
+     * {@link WebApkIdentityServiceClient#SHELL_APK_VERSION_SUPPORTING_SWITCH_RUNTIME_HOST},
+     * the backs WebAPK check returns true only if the browser matches the WebAPK's runtime host
+     * specified in the metaData.
+     */
+    @Test
+    public void testBacksWebApkCheckBeforeIntroducingHostBrowserSwitchLogic() {
+        String webApkPackageName = WEBAPK_PACKAGE_NAME;
+        String webApkRuntimeHost = RUNTIME_HOST_IN_METADATA;
+        int shellApkVersion =
+                WebApkIdentityServiceClient.SHELL_APK_VERSION_SUPPORTING_SWITCH_RUNTIME_HOST - 1;
+        registerWebApk(webApkPackageName, webApkRuntimeHost, shellApkVersion);
+
+        String browserPackageName = RUNTIME_HOST_IN_METADATA;
+        mockBrowserContext(browserPackageName);
+        checkBacksWebApk(webApkPackageName, true);
+
+        browserPackageName = "another.browser";
+        mockBrowserContext(browserPackageName);
+        checkBacksWebApk(webApkPackageName, false);
+    }
+
+    /**
+     * Tests that for WebAPKs with shell APK version equals or higher than the

[pkotwicz, 2017/07/20 01:32:05]

Nit: equals -> equal

[Xi Han, 2017/07/21 20:36:35]

Done.

+     * {@link WebApkIdentityServiceClient#SHELL_APK_VERSION_SUPPORTING_SWITCH_RUNTIME_HOST} but
+     * doesn't have Identity Service, the backs WebAPK check returns null.

[pkotwicz, 2017/07/20 01:32:05]

You are testing that the callback returns false?

[Xi Han, 2017/07/21 20:36:35]

Done.

+     */
+    @Test
+    public void testBacksWebApkCheckForWebApkWithHostBrowserSwitchLogicButWithoutIdentityService() {
+        String webApkPackageName = WEBAPK_PACKAGE_NAME;
+        String webApkRuntimeHost = RUNTIME_HOST_IN_METADATA;
+        int shellApkVersion =
+                WebApkIdentityServiceClient.SHELL_APK_VERSION_SUPPORTING_SWITCH_RUNTIME_HOST;
+        registerWebApk(webApkPackageName, webApkRuntimeHost, shellApkVersion);
+
+        String browserPackageName = RUNTIME_HOST_IN_METADATA;
+        mockBrowserContext(browserPackageName);
+        checkBacksWebApk(webApkPackageName, false);
+    }
+
+    /**
+     * Tests that for WebAPKs with Identity service, the backs WebAPK check returns true if the

[pkotwicz, 2017/07/20 01:32:05]

Nit: Add '-' to "backs WebAPK check" to improve readability ->
"backs-WebAPK-check"

(here and elsewhere)

[Xi Han, 2017/07/21 20:36:35]

Done.

+     * package name of the browser matches the one provided by the Identity service.
+     */
+    @Test
+    public void testBacksWebApkCheckForWebApkWithIdentityService() {
+        String webApkPackageName = WEBAPK_PACKAGE_NAME;
+        String webApkSpecifiedRuntimeHost = RUNTIME_HOST_IN_METADATA;
+        String realRuntimeHost = REAL_RUNTIME_HOST;
+        // The shell APK version doesn't matter as long as the WebAPK has an Identity service.
+        registerWebApk(webApkPackageName, webApkSpecifiedRuntimeHost, 0 /*shellApkVersion*/);
+        mockIdentityService(webApkPackageName, realRuntimeHost);
+
+        String browserPackageName = REAL_RUNTIME_HOST;
+        mockBrowserContext(browserPackageName);
+        checkBacksWebApk(webApkPackageName, true);
+
+        browserPackageName = RUNTIME_HOST_IN_METADATA;
+        mockBrowserContext(browserPackageName);
+        checkBacksWebApk(webApkPackageName, false);
+    }
+
+    /** Registers a WebAPK with the runtime host and the shell APK version in its metadata. */
+    private void registerWebApk(String webApkPackageName, String runtimeHost, int shellApkVersion) {
+        Bundle bundle = new Bundle();
+        bundle.putString(WebApkMetaDataKeys.RUNTIME_HOST, runtimeHost);
+        bundle.putInt(WebApkMetaDataKeys.SHELL_APK_VERSION, shellApkVersion);
+        WebApkTestHelper.registerWebApkWithMetaData(webApkPackageName, bundle);
+    }
+
+    // Mocks the browser context for the given browser package name.
+    private void mockBrowserContext(String browserPackageName) {
+        Context context = Mockito.spy(mShadowApplication.getApplicationContext());
+        when(context.getPackageName()).thenReturn(browserPackageName);

[pkotwicz, 2017/07/20 01:32:05]

Can you set the package name via @Config like we do in WebApkUtilsTest.java?

You can call registerWebApk() multiple times in a test case instead of calling
mockBrowserContext() multiple times

[Xi Han, 2017/07/21 20:36:35]

Done.

+    }
+
+    /** Checks whether backs WebAPK check matches the expected result. */
+    private void checkBacksWebApk(String webApkPackageName, final boolean expectedResult) {
+        WebApkIdentityServiceClient.CheckBrowserBacksWebApkCallback callback =
+                new WebApkIdentityServiceClient.CheckBrowserBacksWebApkCallback() {
+                    @Override
+                    public void onChecked(boolean doesBrowserBackWebApk) {
+                        Assert.assertEquals(expectedResult, doesBrowserBackWebApk);
+                    }
+                };
+        WebApkIdentityServiceClient.getInstance().checkBrowserBacksWebApkAsync(
+                mShadowApplication.getApplicationContext(), webApkPackageName, callback);

[pkotwicz, 2017/07/20 01:32:05]

- Does WebApkIdentityServiceClient#checkBrowserBacksWebApkAsync() block till the
check is done?
  You can probably fix this by calling
Robolectric#runUiThreadTasksIncludingDelayedTasks() and checking that the
callback was indeed called.
- RuntimeEnvironment.application is the same as
ShadowApplication#getApplicationContext()

[Xi Han, 2017/07/21 20:36:35]

Done.

+    }
+
+    /**
+     * Mocks an Identity service for the WebAPK.
+     * @param webApkPackageName The webAPK's package name.
+     * @param runtimeHost The real runtime host of the WebAPK.
+     */
+    private void mockIdentityService(String webApkPackageName, String runtimeHost) {
+        Intent intent = WebApkIdentityServiceClient.getInstance()
+                                .getConnectionManagerForTesting()
+                                .createConnectIntent(webApkPackageName);
+
+        IBinder service = Mockito.mock(IBinder.class);
+        IIdentityService identityService = Mockito.mock(IIdentityService.class);
+        try {
+            Mockito.when(identityService.getRuntimeHostBrowserPackageName())
+                    .thenReturn(runtimeHost);
+        } catch (RemoteException e) {
+            Assert.fail();
+        }
+        when(IIdentityService.Stub.asInterface(service)).thenReturn(identityService);

[pkotwicz, 2017/07/20 01:32:05]

Would it be simpler to have a custom implementation which inherits from
IIdentityService.Stub?

[Xi Han, 2017/07/21 20:36:35]

Done.

+
+        mShadowApplication.setComponentNameAndServiceForBindServiceForIntent(

[pkotwicz, 2017/07/20 01:32:05]

Why did you choose to use the 3 argument version of
setComponentNameAndServiceForBindService() instead of the 2 argument version?
(This test class cares about the IdentityService and nothing else. It does not
matter if it is impossible to connect a service which is not the
IdentityService)

[Xi Han, 2017/07/21 20:36:35]

Done.

+                intent, new ComponentName(webApkPackageName, ""), service);
+    }
+}