Make EnterpriseInfo into a singleton and add tests

Convert the static class EnterpriseInfo into a singleton.

Add unit tests.

Bug: 1085168, 1099271
Change-Id: Ia073bcea30ba84e5453e3bbae2079a71f495c119
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2272866
Commit-Queue: Steven Bingler <bingler@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783969}

chrome/android/chrome_junit_test_java_sources.gni

@@ -191,6 +191,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediatorTest.java",
   "junit/src/org/chromium/chrome/browser/photo_picker/FileEnumWorkerTaskTest.java",
   "junit/src/org/chromium/chrome/browser/photo_picker/PickerBitmapViewTest.java",
+  "junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java",
   "junit/src/org/chromium/chrome/browser/preferences/PrefServiceBridgeTest.java",
   "junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerTest.java",
   "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java",

chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java

@@ -436,7 +436,8 @@ public class ProcessInitializationHandler {
         deferredStartupHandler.addDeferredTask(
                 () -> SharedClipboardShareActivity.updateComponentEnabledState());
         deferredStartupHandler.addDeferredTask(() -> ExploreOfflineStatusProvider.getInstance());
-        deferredStartupHandler.addDeferredTask(() -> EnterpriseInfo.logDeviceEnterpriseInfo());
+        deferredStartupHandler.addDeferredTask(
+                () -> EnterpriseInfo.getInstance().logDeviceEnterpriseInfo());
     }
 
     private void initChannelsAsync() {

chrome/android/java/src/org/chromium/chrome/browser/policy/EnterpriseInfo.java

@@ -10,6 +10,8 @@ import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.os.SystemClock;
 
+import androidx.annotation.VisibleForTesting;
+
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
@@ -24,20 +26,21 @@ import java.util.LinkedList;
 import java.util.Queue;
 import java.util.concurrent.RejectedExecutionException;
 
-// TODO: This class needs tests. https://crbug.com/1099271
-
 /**
  * Provide the enterprise information for the current device and profile.
  */
 public final class EnterpriseInfo {
     private static final String TAG = "EnterpriseInfo";
 
+    private static EnterpriseInfo sInstance;
+
     // Only ever read/written on the UI thread.
-    private static OwnedState sOwnedState = null;
-    private static Queue<Callback<OwnedState>> sCallbackList =
-            new LinkedList<Callback<OwnedState>>();
+    private OwnedState mOwnedState = null;
+    private Queue<Callback<OwnedState>> mCallbackList;
 
-    private static class OwnedState {
+    private boolean mSkipAsyncCheckForTesting = false;
+
+    static class OwnedState {
         boolean mDeviceOwned;
         boolean mProfileOwned;
 
@@ -45,54 +48,50 @@ public final class EnterpriseInfo {
             mDeviceOwned = isDeviceOwned;
             mProfileOwned = isProfileOwned;
         }
-    }
 
-    /**
-     * Returns, via callback, whether the device has a device owner or a profile owner for native.
-     */
-    @CalledByNative
-    public static void getManagedStateForNative() {
-        Callback<OwnedState> callback = (result) -> {
-            if (result == null) {
-                // Unable to determine the owned state, assume it's not owned.
-                EnterpriseInfoJni.get().updateNativeOwnedState(false, false);
-            }
+        @Override
+        public boolean equals(Object other) {
+            if (this == other) return true;
+            if (other == null) return false;
+            if (!(other instanceof OwnedState)) return false;
 
-            EnterpriseInfoJni.get().updateNativeOwnedState(
-                    result.mDeviceOwned, result.mProfileOwned);
-        };
+            OwnedState otherOwnedState = (OwnedState) other;
 
-        getDeviceEnterpriseInfo(callback);
+            return this.mDeviceOwned == otherOwnedState.mDeviceOwned
+                    && this.mProfileOwned == otherOwnedState.mProfileOwned;
+        }
     }
 
-    /**
-     * Records metrics regarding whether the device has a device owner or a profile owner.
-     */
-    public static void logDeviceEnterpriseInfo() {
-        Callback<OwnedState> callback = (result) -> {
-            recordManagementHistograms(result);
-        };
+    public static EnterpriseInfo getInstance() {
+        ThreadUtils.assertOnUiThread();
 
-        getDeviceEnterpriseInfo(callback);
+        if (sInstance == null) sInstance = new EnterpriseInfo();
+
+        return sInstance;
     }
 
-    private static void getDeviceEnterpriseInfo(Callback<OwnedState> callback) {
+    /**
+     * Returns, via callback, whether the device has a device owner or a profile owner.
+     */
+    public void getDeviceEnterpriseInfo(Callback<OwnedState> callback) {
         // AsyncTask requires being called from UI thread.
         ThreadUtils.assertOnUiThread();
         assert callback != null;
 
-        if (sOwnedState != null) {
-            callback.onResult(sOwnedState);
+        if (mOwnedState != null) {
+            callback.onResult(mOwnedState);
             return;
         }
 
-        sCallbackList.add(callback);
+        mCallbackList.add(callback);
 
-        if (sCallbackList.size() > 1) {
+        if (mCallbackList.size() > 1) {
             // A pending callback is already being worked on, no need to start up a new thread.
             return;
         }
 
+        if (mSkipAsyncCheckForTesting) return;
+
         // This is the first request, spin up a thread.
         try {
             new AsyncTask<OwnedState>() {
@@ -133,14 +132,7 @@ public final class EnterpriseInfo {
 
                 @Override
                 protected void onPostExecute(OwnedState result) {
-                    // This is run on the UI thread.
-                    assert result != null;
-
-                    sOwnedState = result;
-                    // Notify every waiting callback.
-                    while (sCallbackList.size() > 0) {
-                        sCallbackList.remove().onResult(sOwnedState);
-                    }
+                    onEnterpriseInfoResult(result);
                 }
             }.executeWithTaskTraits(TaskTraits.USER_VISIBLE);
         } catch (RejectedExecutionException e) {
@@ -150,11 +142,44 @@ public final class EnterpriseInfo {
 
             // There will only ever be a single item in the queue as we only try()/catch() on the
             // first item.
-            sCallbackList.remove().onResult(null);
+            mCallbackList.remove().onResult(null);
         }
     }
 
-    private static void recordManagementHistograms(OwnedState state) {
+    /**
+     * Records metrics regarding whether the device has a device owner or a profile owner.
+     */
+    public void logDeviceEnterpriseInfo() {
+        Callback<OwnedState> callback = (result) -> {
+            recordManagementHistograms(result);
+        };
+
+        getDeviceEnterpriseInfo(callback);
+    }
+
+    private EnterpriseInfo() {
+        mOwnedState = null;
+        mCallbackList = new LinkedList<Callback<OwnedState>>();
+    }
+
+    @VisibleForTesting
+    void onEnterpriseInfoResult(OwnedState result) {
+        ThreadUtils.assertOnUiThread();
+        assert result != null;
+
+        // Set the cached value.
+        mOwnedState = result;
+
+        // Service every waiting callback.
+        while (mCallbackList.size() > 0) {
+            // This implementation assumes that ever future call to getDeviceEnterpriseInfo(), from
+            // this point, will result in the cached value being returned immediately. This means we
+            // can ignore the issue of re-entrant callbacks.
+            mCallbackList.remove().onResult(mOwnedState);
+        }
+    }
+
+    private void recordManagementHistograms(OwnedState state) {
         if (state == null) return;
 
         RecordHistogram.recordBooleanHistogram("EnterpriseCheck.IsManaged2", state.mProfileOwned);
@@ -162,6 +187,34 @@ public final class EnterpriseInfo {
                 "EnterpriseCheck.IsFullyManaged2", state.mDeviceOwned);
     }
 
+    @VisibleForTesting
+    void setSkipAsyncCheckForTesting(boolean skip) {
+        mSkipAsyncCheckForTesting = skip;
+    }
+
+    @VisibleForTesting
+    static void reset() {
+        sInstance = null;
+    }
+
+    /**
+     * Returns, via callback, the ownded state for native's AndroidEnterpriseInfo.
+     */
+    @CalledByNative
+    public static void getManagedStateForNative() {
+        Callback<OwnedState> callback = (result) -> {
+            if (result == null) {
+                // Unable to determine the owned state, assume it's not owned.
+                EnterpriseInfoJni.get().updateNativeOwnedState(false, false);
+            }
+
+            EnterpriseInfoJni.get().updateNativeOwnedState(
+                    result.mDeviceOwned, result.mProfileOwned);
+        };
+
+        EnterpriseInfo.getInstance().getDeviceEnterpriseInfo(callback);
+    }
+
     @NativeMethods
     interface Natives {
         void updateNativeOwnedState(boolean hasProfileOwnerApp, boolean hasDeviceOwnerApp);

chrome/android/junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java

+// Copyright 2020 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.chrome.browser.policy;
+
+import android.support.test.filters.SmallTest;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.Callback;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.CallbackHelper;
+
+/**
+ * Tests EnterpriseInfo.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+public class EnterpriseInfoTest {
+    @Before
+    public void setUp() {
+        EnterpriseInfo.reset();
+        // Skip the AsyncTask, we don't actually want to query the device, just enqueue callbacks.
+        EnterpriseInfo.getInstance().setSkipAsyncCheckForTesting(true);
+    }
+
+    /**
+     * Tests that the callback is called with the correct result.
+     * Tests both the first computation and the cached value.
+     */
+    @Test
+    @SmallTest
+    public void testCallbacksGetResultValue() {
+        EnterpriseInfo instance = EnterpriseInfo.getInstance();
+
+        EnterpriseInfo.OwnedState stateIn = new EnterpriseInfo.OwnedState(false, true);
+
+        class CallbackWithResult implements Callback<EnterpriseInfo.OwnedState> {
+            public EnterpriseInfo.OwnedState result = null;
+
+            @Override
+            public void onResult(EnterpriseInfo.OwnedState result) {
+                this.result = result;
+            }
+        };
+        CallbackWithResult callback = new CallbackWithResult();
+        CallbackWithResult callback2 = new CallbackWithResult();
+
+        // Make the request and service the callbacks.
+        instance.getDeviceEnterpriseInfo(callback);
+        instance.getDeviceEnterpriseInfo(callback2);
+        instance.onEnterpriseInfoResult(stateIn);
+
+        // Results should be the same for all callbacks.
+        Assert.assertEquals("Callback doesn't match the expected result on servicing.",
+                callback.result, stateIn);
+        Assert.assertEquals("Callback doesn't match the expected result on servicing.",
+                callback2.result, stateIn);
+
+        // Reset the callbacks.
+        callback.result = null;
+        callback2.result = null;
+        Assert.assertNotEquals("Callback wasn't reset properly.", callback.result, stateIn);
+        Assert.assertNotEquals("Callback wasn't reset properly.", callback2.result, stateIn);
+
+        // Check the cached value is returned correctly.
+        instance.getDeviceEnterpriseInfo(callback);
+        instance.getDeviceEnterpriseInfo(callback2);
+        // This should happen immediately, see testCallbackServicedWhenResultCached.
+        Assert.assertEquals(
+                "Callback doesn't match the expected cached result.", callback.result, stateIn);
+        Assert.assertEquals(
+                "Callback doesn't match the expected cached result.", callback2.result, stateIn);
+    }
+
+    /**
+     * Test that if multiple callbacks get queued up that they're all serviced.
+     */
+    @Test
+    @SmallTest
+    public void testMultipleCallbacksServiced() {
+        EnterpriseInfo instance = EnterpriseInfo.getInstance();
+        CallbackHelper helper = new CallbackHelper();
+
+        Callback<EnterpriseInfo.OwnedState> callback = (result) -> {
+            // We don't care about the result in this test.
+            helper.notifyCalled();
+        };
+
+        // Load up requests
+        final int count = 5;
+        for (int i = 0; i < count; i++) {
+            instance.getDeviceEnterpriseInfo(callback);
+        }
+
+        // Nothing should be called yet.
+        Assert.assertEquals(
+                "Callbacks were serviced before they were meant to be.", 0, helper.getCallCount());
+
+        // Do it. The result value here is irrelevant, put anything.
+        instance.onEnterpriseInfoResult(new EnterpriseInfo.OwnedState(true, true));
+
+        Assert.assertEquals(
+                "The wrong number of callbacks were serviced.", count, helper.getCallCount());
+    }
+
+    /**
+     * Tests that a callback is only serviced immediately if there is a cached result.
+     */
+    @Test
+    @SmallTest
+    public void testCallbackServicedWhenResultCached() {
+        EnterpriseInfo instance = EnterpriseInfo.getInstance();
+        CallbackHelper helper = new CallbackHelper();
+
+        Callback<EnterpriseInfo.OwnedState> callback = (result) -> {
+            // We don't care about the result in this test.
+            helper.notifyCalled();
+        };
+
+        // First: Callback should not be serviced if nothing is cached.
+        instance.getDeviceEnterpriseInfo(callback);
+        Assert.assertEquals(
+                "Callbacks were serviced before they were meant to be.", 0, helper.getCallCount());
+
+        // Set a cached result (of any value) and process the callback.
+        instance.onEnterpriseInfoResult(new EnterpriseInfo.OwnedState(false, false));
+        Assert.assertEquals("Only a single callback should have been serviced at this point.", 1,
+                helper.getCallCount());
+
+        // Second: Callback should now be serviced immediately.
+        instance.getDeviceEnterpriseInfo(callback);
+        Assert.assertEquals(
+                "Callback wasn't serviced by the cached result.", 2, helper.getCallCount());
+    }
+
+    /**
+     * Tests that OwnedStates's overridden equals() works as expected.
+     */
+    @Test
+    @SmallTest
+    public void testOwnedStateEquals() {
+        // Two references to the same object are equal. Values don't matter here.
+        EnterpriseInfo.OwnedState ref = new EnterpriseInfo.OwnedState(true, true);
+        EnterpriseInfo.OwnedState sameRef = ref;
+        Assert.assertEquals("Same reference check failed.", ref, sameRef);
+
+        // This is also true for null.
+        EnterpriseInfo.OwnedState nullRef = null;
+        EnterpriseInfo.OwnedState nullRef2 = null;
+        Assert.assertEquals("Null reference check failed.", nullRef, nullRef2);
+
+        // A valid object and null should not be equal.
+
+        Assert.assertNotEquals("Valid obj == null check failed.", ref, nullRef);
+
+        // A different type of, valid, object shouldn't be equal.
+        Object obj = new Object();
+        Assert.assertNotEquals("Wrong object type check failed.", ref, obj);
+
+        // Two valid owned states should only be equal when their member variables are all the same.
+        EnterpriseInfo.OwnedState trueTrue = ref;
+        EnterpriseInfo.OwnedState falseFalse = new EnterpriseInfo.OwnedState(false, false);
+        EnterpriseInfo.OwnedState trueFalse = new EnterpriseInfo.OwnedState(true, false);
+        EnterpriseInfo.OwnedState falseTrue = new EnterpriseInfo.OwnedState(false, true);
+        EnterpriseInfo.OwnedState trueTrue2 = new EnterpriseInfo.OwnedState(true, true);
+
+        Assert.assertNotEquals("Wrong value check failed.", trueTrue, falseFalse);
+        Assert.assertNotEquals("Wrong value check failed.", trueTrue, trueFalse);
+        Assert.assertNotEquals("Wrong value check failed.", trueTrue, falseTrue);
+        Assert.assertEquals("Correct value check failed.", trueTrue, trueTrue2);
+    }
+}