Make AccountCapabilities mapping immutable.

Moves the capabilities creation to the constructor to ensure that this
mapping is immutable. This removes the need to check for incorrect
updates to the capability state (e.g., overriding a known value with
an exception).

Bug: 1298465
Change-Id: I79a8ecbdec8d24cfe8885a647ced6f88f9bffbf5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3500839
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985244}

chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerImplTest.java

@@ -46,6 +46,7 @@
 import org.chromium.components.signin.metrics.SignoutReason;
 import org.chromium.components.sync.ModelType;
 
+import java.util.HashMap;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -57,7 +58,7 @@ public class SigninManagerImplTest {
     private static final long NATIVE_IDENTITY_MANAGER = 10002L;
     private static final AccountInfo ACCOUNT_INFO =
             new AccountInfo(new CoreAccountId("gaia-id-user"), "user@domain.com", "gaia-id-user",
-                    "full name", "given name", null, new AccountCapabilities());
+                    "full name", "given name", null, new AccountCapabilities(new HashMap<>()));
 
     @Rule
     public final JniMocker mocker = new JniMocker();

chrome/browser/signin/services/android/java/src/org/chromium/chrome/browser/signin/services/ProfileDataCacheRenderTest.java

@@ -62,6 +62,7 @@
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 
 /**
@@ -107,9 +108,9 @@ public ProfileDataCacheRenderTest(int imageSize) {
     @Mock
     private IdentityManager.Natives mIdentityManagerNativeMock;
 
-    private final AccountInfo mAccountInfoWithAvatar =
-            new AccountInfo(new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test",
-                    "full name", "given name", createAvatar(), new AccountCapabilities());
+    private final AccountInfo mAccountInfoWithAvatar = new AccountInfo(
+            new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test", "full name",
+            "given name", createAvatar(), new AccountCapabilities(new HashMap<>()));
 
     private FrameLayout mContentView;
     private ImageView mImageView;
@@ -193,7 +194,7 @@ public void testNoProfileDataRemovedWithEmptyAccountInfo() throws IOException {
             mIdentityManager.onExtendedAccountInfoUpdated(mAccountInfoWithAvatar);
             final AccountInfo emptyAccountInfo = new AccountInfo(mAccountInfoWithAvatar.getId(),
                     mAccountInfoWithAvatar.getEmail(), mAccountInfoWithAvatar.getGaiaId(), null,
-                    null, null, new AccountCapabilities());
+                    null, null, new AccountCapabilities(new HashMap<>()));
             mIdentityManager.onExtendedAccountInfoUpdated(emptyAccountInfo);
             checkImageIsScaled(mAccountInfoWithAvatar.getEmail());
         });

chrome/browser/signin/services/android/junit/src/org/chromium/chrome/browser/signin/services/ProfileDataCacheUnitTest.java

@@ -29,6 +29,8 @@
 import org.chromium.components.signin.identitymanager.IdentityManager;
 import org.chromium.components.signin.identitymanager.IdentityManagerJni;
 
+import java.util.HashMap;
+
 /**
  * Unit tests for {@link ProfileDataCache}
  */
@@ -79,8 +81,9 @@ public void tearDown() {
     @Test
     public void accountInfoIsUpdatedWithOnlyFullName() {
         final String fullName = "full name1";
-        final AccountInfo accountInfo = new AccountInfo(new CoreAccountId("gaia-id-test"),
-                ACCOUNT_EMAIL, "gaia-id-test", fullName, null, null, new AccountCapabilities());
+        final AccountInfo accountInfo =
+                new AccountInfo(new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test",
+                        fullName, null, null, new AccountCapabilities(new HashMap<>()));
         mProfileDataCache.addObserver(mObserverMock);
         Assert.assertFalse(mProfileDataCache.hasProfileData(ACCOUNT_EMAIL));
         Assert.assertNull(mProfileDataCache.getProfileDataOrDefault(ACCOUNT_EMAIL).getFullName());
@@ -95,8 +98,9 @@ public void accountInfoIsUpdatedWithOnlyFullName() {
     @Test
     public void accountInfoIsUpdatedWithOnlyGivenName() {
         final String givenName = "given name1";
-        final AccountInfo accountInfo = new AccountInfo(new CoreAccountId("gaia-id-test"),
-                ACCOUNT_EMAIL, "gaia-id-test", null, givenName, null, new AccountCapabilities());
+        final AccountInfo accountInfo =
+                new AccountInfo(new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test",
+                        null, givenName, null, new AccountCapabilities(new HashMap<>()));
         mProfileDataCache.addObserver(mObserverMock);
         Assert.assertFalse(mProfileDataCache.hasProfileData(ACCOUNT_EMAIL));
         Assert.assertNull(mProfileDataCache.getProfileDataOrDefault(ACCOUNT_EMAIL).getGivenName());
@@ -111,8 +115,9 @@ public void accountInfoIsUpdatedWithOnlyGivenName() {
     @Test
     public void accountInfoIsUpdatedWithOnlyBadgeConfig() {
         mProfileDataCache.setBadge(R.drawable.ic_sync_badge_error_20dp);
-        final AccountInfo accountInfo = new AccountInfo(new CoreAccountId("gaia-id-test"),
-                ACCOUNT_EMAIL, "gaia-id-test", null, null, null, new AccountCapabilities());
+        final AccountInfo accountInfo =
+                new AccountInfo(new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test",
+                        null, null, null, new AccountCapabilities(new HashMap<>()));
         mProfileDataCache.addObserver(mObserverMock);
         Assert.assertFalse(mProfileDataCache.hasProfileData(ACCOUNT_EMAIL));
 

components/signin/public/android/java/src/org/chromium/components/signin/base/AccountCapabilities.java

@@ -5,6 +5,7 @@
 package org.chromium.components.signin.base;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.components.signin.AccountCapabilitiesConstants;
@@ -19,10 +20,11 @@
  * This class has a native counterpart.
  */
 public class AccountCapabilities {
-    private final Map<String, Boolean> mAccountCapabilities = new HashMap<>();
+    private final Map<String, Boolean> mAccountCapabilities;
 
     @CalledByNative
     public AccountCapabilities(String[] capabilityNames, boolean[] capabilityValues) {
+        mAccountCapabilities = new HashMap<>();
         assert capabilityNames.length == capabilityValues.length;
         for (int i = 0; i < capabilityNames.length; i += 1) {
             final String capabilityName = capabilityNames[i];
@@ -34,46 +36,31 @@ public AccountCapabilities(String[] capabilityNames, boolean[] capabilityValues)
         }
     }
 
-    public AccountCapabilities() {}
+    @VisibleForTesting
+    public AccountCapabilities(HashMap<String, Boolean> accountCapabilities) {
+        mAccountCapabilities = accountCapabilities;
+    }
 
     /**
-     * @param account the given account to retrieve capabilities from.
-     * @param managerDelegate the manager used to query capability responses.
+     * @param capabilityResponses the mapping from capability name to value.
      * @return the supported account capabilities values.
      */
     public static AccountCapabilities parseFromCapabilitiesResponse(
             Map<String, Integer> capabilityResponses) {
         assert capabilityResponses.size()
                 == AccountCapabilitiesConstants.SUPPORTED_ACCOUNT_CAPABILITY_NAMES.size();
-        AccountCapabilities capabilities = new AccountCapabilities();
+        HashMap<String, Boolean> capabilities = new HashMap<>();
         for (String capabilityName :
                 AccountCapabilitiesConstants.SUPPORTED_ACCOUNT_CAPABILITY_NAMES) {
             assert capabilityResponses.containsKey(capabilityName);
             @AccountManagerDelegate.CapabilityResponse
             int hasCapability = capabilityResponses.get(capabilityName);
-            capabilities.setAccountCapability(capabilityName, hasCapability);
-        }
-        return capabilities;
-    }
-
-    /**
-     * Stores the Capability Value for the given capability name.
-     * @param capabilityName One of the supported capability names {@link
-     *         #AccountCapabilitiesConstants.SUPPORTED_ACCOUNT_CAPABILITY_NAMES}.
-     * @param hasCapability Capability Response for this capability.
-     */
-    public void setAccountCapability(@NonNull String capabilityName,
-            @AccountManagerDelegate.CapabilityResponse int hasCapability) {
-        assert AccountCapabilitiesConstants.SUPPORTED_ACCOUNT_CAPABILITY_NAMES.contains(
-                capabilityName)
-            : "Capability name not supported: "
-                + capabilityName;
-        if (hasCapability == AccountManagerDelegate.CapabilityResponse.EXCEPTION) {
-            /* The value of the capability is unknown, no need to change the map of capabilities. */
-            return;
+            if (hasCapability != AccountManagerDelegate.CapabilityResponse.EXCEPTION) {
+                capabilities.put(capabilityName,
+                        hasCapability == AccountManagerDelegate.CapabilityResponse.YES);
+            }
         }
-        mAccountCapabilities.put(
-                capabilityName, hasCapability == AccountManagerDelegate.CapabilityResponse.YES);
+        return new AccountCapabilities(capabilities);
     }
 
     /**

components/signin/public/android/java/src/org/chromium/components/signin/test/util/FakeAccountInfoService.java

@@ -73,7 +73,7 @@ public void addAccountInfo(
                 email, FakeAccountManagerFacade.toGaiaId(email));
         final AccountInfo accountInfo = new AccountInfo(coreAccountInfo.getId(),
                 coreAccountInfo.getEmail(), coreAccountInfo.getGaiaId(), fullName, givenName,
-                avatar, new AccountCapabilities());
+                avatar, new AccountCapabilities(new HashMap<>()));
         mAccountInfos.put(email, accountInfo);
 
         ThreadUtils.runOnUiThreadBlocking(() -> {

components/signin/public/android/java/src/org/chromium/components/signin/test/util/FakeAccountManagerFacade.java

@@ -24,6 +24,7 @@
 import org.chromium.components.signin.base.AccountCapabilities;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -113,7 +114,7 @@ public void checkChildAccountStatus(Account account, ChildAccountStatusListener
 
     @Override
     public Promise<AccountCapabilities> getAccountCapabilities(Account account) {
-        return Promise.fulfilled(new AccountCapabilities());
+        return Promise.fulfilled(new AccountCapabilities(new HashMap<>()));
     }
 
     @Override

components/signin/public/android/junit/src/org/chromium/components/signin/base/AccountCapabilitiesTest.java

@@ -117,40 +117,25 @@ public void setUp() {
     @Test
     @ParameterAnnotations.UseMethodParameter(CapabilitiesTestParams.class)
     public void testCapabilityResponseException(String capabilityName) {
-        AccountCapabilities capabilities = new AccountCapabilities();
-        capabilities.setAccountCapability(
-                capabilityName, AccountManagerDelegate.CapabilityResponse.EXCEPTION);
+        AccountCapabilities capabilities = new AccountCapabilities(new HashMap<>());
         Assert.assertEquals(getCapability(capabilityName, capabilities), Tribool.UNKNOWN);
     }
 
     @Test
     @ParameterAnnotations.UseMethodParameter(CapabilitiesTestParams.class)
     public void testCapabilityResponseYes(String capabilityName) {
-        AccountCapabilities capabilities = new AccountCapabilities();
-        capabilities.setAccountCapability(
-                capabilityName, AccountManagerDelegate.CapabilityResponse.YES);
+        AccountCapabilities capabilities = new AccountCapabilities(new HashMap<String, Boolean>() {
+            { put(capabilityName, true); }
+        });
         Assert.assertEquals(getCapability(capabilityName, capabilities), Tribool.TRUE);
     }
 
     @Test
     @ParameterAnnotations.UseMethodParameter(CapabilitiesTestParams.class)
     public void testCapabilityResponseNo(String capabilityName) {
-        AccountCapabilities capabilities = new AccountCapabilities();
-        capabilities.setAccountCapability(
-                capabilityName, AccountManagerDelegate.CapabilityResponse.NO);
-        Assert.assertEquals(getCapability(capabilityName, capabilities), Tribool.FALSE);
-    }
-
-    @Test
-    @ParameterAnnotations.UseMethodParameter(CapabilitiesTestParams.class)
-    public void testCapabilityResponseFalseAfterException(String capabilityName) {
-        AccountCapabilities capabilities = new AccountCapabilities();
-        capabilities.setAccountCapability(
-                capabilityName, AccountManagerDelegate.CapabilityResponse.NO);
-        Assert.assertEquals(getCapability(capabilityName, capabilities), Tribool.FALSE);
-
-        capabilities.setAccountCapability(
-                capabilityName, AccountManagerDelegate.CapabilityResponse.EXCEPTION);
+        AccountCapabilities capabilities = new AccountCapabilities(new HashMap<String, Boolean>() {
+            { put(capabilityName, false); }
+        });
         Assert.assertEquals(getCapability(capabilityName, capabilities), Tribool.FALSE);
     }
 

components/signin/public/android/junit/src/org/chromium/components/signin/identitymanager/AccountInfoServiceImplTest.java

@@ -26,6 +26,8 @@
 import org.chromium.components.signin.base.AccountInfo;
 import org.chromium.components.signin.base.CoreAccountId;
 
+import java.util.HashMap;
+
 /**
  * Unit tests for {@link AccountInfoServiceImpl}.
  */
@@ -45,9 +47,9 @@ public class AccountInfoServiceImplTest {
     @Mock
     private AccountInfoService.Observer mObserverMock;
 
-    private final AccountInfo mAccountInfoWithAvatar =
-            new AccountInfo(new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test",
-                    "full name", "given name", mock(Bitmap.class), new AccountCapabilities());
+    private final AccountInfo mAccountInfoWithAvatar = new AccountInfo(
+            new CoreAccountId("gaia-id-test"), ACCOUNT_EMAIL, "gaia-id-test", "full name",
+            "given name", mock(Bitmap.class), new AccountCapabilities(new HashMap<>()));
 
     private AccountInfoServiceImpl mService;