Remove ContentSettingExceptionType enum

This enum contains a subset of the values of ContentSettingsType.
As WebsitePermissionFetcher defines which ContentSettingsType are
fetched as ContentSettingsException and which as PermissionInfo, we do
not need this duplicate and the code to convert between types.

Bug: 1103597
Change-Id: I17667e76c5091ed6417795cdba807c78b35a85a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346425Reviewed-by: Ehimare Okoyomon <eokoyomon@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798580}

chrome/android/javatests/src/org/chromium/chrome/browser/site_settings/WebsitePermissionsFetcherTest.java

@@ -536,20 +536,19 @@ public class WebsitePermissionsFetcherTest {
 
             // Check content setting exception types.
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.COOKIE));
+                    site.getContentSettingPermission(ContentSettingsType.COOKIES));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.POPUP));
+                    site.getContentSettingPermission(ContentSettingsType.POPUPS));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.ADS));
+                    site.getContentSettingPermission(ContentSettingsType.ADS));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.JAVASCRIPT));
+                    site.getContentSettingPermission(ContentSettingsType.JAVASCRIPT));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.SOUND));
+                    site.getContentSettingPermission(ContentSettingsType.SOUND));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(ContentSettingException.Type.BACKGROUND_SYNC));
+                    site.getContentSettingPermission(ContentSettingsType.BACKGROUND_SYNC));
             Assert.assertEquals(Integer.valueOf(ContentSettingValues.DEFAULT),
-                    site.getContentSettingPermission(
-                            ContentSettingException.Type.AUTOMATIC_DOWNLOADS));
+                    site.getContentSettingPermission(ContentSettingsType.AUTOMATIC_DOWNLOADS));
 
             // Check storage info.
             ArrayList<StorageInfo> storageInfos = new ArrayList<>(site.getStorageInfo());
@@ -705,26 +704,21 @@ public class WebsitePermissionsFetcherTest {
 
         String googleOrigin = "https://google.com";
         String preferenceSource = "preference";
-        ArrayList<Integer> contentSettingExceptionTypes = new ArrayList<>(Arrays.asList(
-                ContentSettingException.Type.ADS, ContentSettingException.Type.AUTOMATIC_DOWNLOADS,
-                ContentSettingException.Type.BACKGROUND_SYNC,
-                ContentSettingException.Type.BLUETOOTH_SCANNING,
-                ContentSettingException.Type.COOKIE, ContentSettingException.Type.JAVASCRIPT,
-                ContentSettingException.Type.POPUP, ContentSettingException.Type.SOUND));
-        Assert.assertEquals(8, ContentSettingException.Type.NUM_ENTRIES);
+        ArrayList<Integer> contentSettingExceptionTypes = new ArrayList<>(
+                Arrays.asList(ContentSettingsType.ADS, ContentSettingsType.AUTOMATIC_DOWNLOADS,
+                        ContentSettingsType.BACKGROUND_SYNC, ContentSettingsType.BLUETOOTH_SCANNING,
+                        ContentSettingsType.COOKIES, ContentSettingsType.JAVASCRIPT,
+                        ContentSettingsType.POPUPS, ContentSettingsType.SOUND));
 
         for (@ContentSettingsType int type : contentSettingExceptionTypes) {
-            @ContentSettingsType
-            int contentSettingsType = ContentSettingException.getContentSettingsType(type);
             {
-                ContentSettingException fakeContentSettingException =
-                        new ContentSettingException(contentSettingsType, googleOrigin,
-                                ContentSettingValues.DEFAULT, preferenceSource);
+                ContentSettingException fakeContentSettingException = new ContentSettingException(
+                        type, googleOrigin, ContentSettingValues.DEFAULT, preferenceSource);
                 websitePreferenceBridge.addContentSettingException(fakeContentSettingException);
 
                 fetcher.fetchPreferencesForCategory(
                         SiteSettingsCategory.createFromContentSettingsType(
-                                UNUSED_BROWSER_CONTEXT_HANDLE, contentSettingsType),
+                                UNUSED_BROWSER_CONTEXT_HANDLE, type),
                         (sites) -> {
                             Assert.assertEquals(1, sites.size());
 
@@ -736,14 +730,13 @@ public class WebsitePermissionsFetcherTest {
 
             // Make sure that the content setting value is updated.
             {
-                ContentSettingException fakeContentSettingException =
-                        new ContentSettingException(contentSettingsType, googleOrigin,
-                                ContentSettingValues.BLOCK, preferenceSource);
+                ContentSettingException fakeContentSettingException = new ContentSettingException(
+                        type, googleOrigin, ContentSettingValues.BLOCK, preferenceSource);
                 websitePreferenceBridge.addContentSettingException(fakeContentSettingException);
 
                 fetcher.fetchPreferencesForCategory(
                         SiteSettingsCategory.createFromContentSettingsType(
-                                UNUSED_BROWSER_CONTEXT_HANDLE, contentSettingsType),
+                                UNUSED_BROWSER_CONTEXT_HANDLE, type),
                         (sites) -> {
                             Assert.assertEquals(1, sites.size());
 
@@ -766,11 +759,8 @@ public class WebsitePermissionsFetcherTest {
         String mainSite = "https://a.com";
         String thirdPartySite = "https://b.com";
         String preferenceSource = "preference";
-        @ContentSettingException.Type
-        int contentSettingExceptionType = ContentSettingException.Type.COOKIE;
         @ContentSettingsType
-        int contentSettingsType =
-                ContentSettingException.getContentSettingsType(contentSettingExceptionType);
+        int contentSettingsType = ContentSettingsType.COOKIES;
 
         // Test the advanced exception combinations of:
         // b.com on a.com
@@ -797,7 +787,7 @@ public class WebsitePermissionsFetcherTest {
 
                             Website site = sites.iterator().next();
                             assertContentSettingExceptionEquals(fakeContentSettingException,
-                                    site.getContentSettingException(contentSettingExceptionType));
+                                    site.getContentSettingException(contentSettingsType));
                         });
             }
 
@@ -816,7 +806,7 @@ public class WebsitePermissionsFetcherTest {
 
                             Website site = sites.iterator().next();
                             assertContentSettingExceptionEquals(fakeContentSettingException,
-                                    site.getContentSettingException(contentSettingExceptionType));
+                                    site.getContentSettingException(contentSettingsType));
                         });
             }
         }

chrome/android/junit/src/org/chromium/chrome/browser/site_settings/SingleWebsiteSettingsTest.java

@@ -73,8 +73,9 @@ public class SingleWebsiteSettingsTest {
     @Test
     @SmallTest
     public void testCorrectMapOfPreferenceKeyToContentSettingsType() {
+        SingleWebsiteSettings settings = new SingleWebsiteSettings();
         for (String key : SingleWebsiteSettings.PERMISSION_PREFERENCE_KEYS) {
-            Assert.assertEquals(SingleWebsiteSettings.getContentSettingsTypeFromPreferenceKey(key),
+            Assert.assertEquals(settings.getContentSettingsTypeFromPreferenceKey(key),
                     getCorrectContentSettingsTypeForPreferenceKey(key));
         }
     }

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/ContentSettingException.java

@@ -6,42 +6,17 @@ package org.chromium.components.browser_ui.site_settings;
 
 import static org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge.SITE_WILDCARD;
 
-import androidx.annotation.IntDef;
 import androidx.annotation.Nullable;
 
 import org.chromium.components.content_settings.ContentSettingValues;
-import org.chromium.components.content_settings.ContentSettingsType;
 import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
 
 import java.io.Serializable;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 
 /**
  * Exception information for a given origin.
  */
 public class ContentSettingException implements Serializable {
-    @IntDef({Type.ADS, Type.AUTOMATIC_DOWNLOADS, Type.BACKGROUND_SYNC, Type.BLUETOOTH_SCANNING,
-            Type.COOKIE, Type.JAVASCRIPT, Type.POPUP, Type.SOUND})
-    @Retention(RetentionPolicy.SOURCE)
-    public @interface Type {
-        // Values used to address array index - should be enumerated from 0 and can't have gaps.
-        // All updates here must also be reflected in {@link #getContentSettingsType(int)
-        // getContentSettingsType} and {@link SingleWebsiteSettings.PERMISSION_PREFERENCE_KEYS}.
-        int ADS = 0;
-        int AUTOMATIC_DOWNLOADS = 1;
-        int BACKGROUND_SYNC = 2;
-        int BLUETOOTH_SCANNING = 3;
-        int COOKIE = 4;
-        int JAVASCRIPT = 5;
-        int POPUP = 6;
-        int SOUND = 7;
-        /**
-         * Number of handled exceptions used for calculating array sizes.
-         */
-        int NUM_ENTRIES = 8;
-    }
-
     private final int mContentSettingType;
     private final String mPrimaryPattern;
     private final String mSecondaryPattern;
@@ -97,33 +72,9 @@ public class ContentSettingException implements Serializable {
     /**
      * Sets the content setting value for this exception.
      */
-    public void setContentSetting(BrowserContextHandle browserContextHandle,
-            @ContentSettingValues @Nullable Integer value) {
+    public void setContentSetting(
+            BrowserContextHandle browserContextHandle, @ContentSettingValues int value) {
         WebsitePreferenceBridge.setContentSettingForPattern(browserContextHandle,
                 mContentSettingType, mPrimaryPattern, mSecondaryPattern, value);
     }
-
-    public static @ContentSettingsType int getContentSettingsType(@Type int type) {
-        switch (type) {
-            case Type.ADS:
-                return ContentSettingsType.ADS;
-            case Type.AUTOMATIC_DOWNLOADS:
-                return ContentSettingsType.AUTOMATIC_DOWNLOADS;
-            case Type.BACKGROUND_SYNC:
-                return ContentSettingsType.BACKGROUND_SYNC;
-            case Type.BLUETOOTH_SCANNING:
-                return ContentSettingsType.BLUETOOTH_SCANNING;
-            case Type.COOKIE:
-                return ContentSettingsType.COOKIES;
-            case Type.JAVASCRIPT:
-                return ContentSettingsType.JAVASCRIPT;
-            case Type.POPUP:
-                return ContentSettingsType.POPUPS;
-            case Type.SOUND:
-                return ContentSettingsType.SOUND;
-            default:
-                assert false;
-                return ContentSettingsType.DEFAULT;
-        }
-    }
 }

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SingleCategorySettings.java

@@ -210,13 +210,11 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
                 getSiteSettingsClient().getBrowserContextHandle();
         for (@SiteSettingsCategory.Type int i = 0; i < SiteSettingsCategory.Type.NUM_ENTRIES; i++) {
             if (!mCategory.showSites(i)) continue;
-            for (@ContentSettingException.Type int j = 0;
-                    j < ContentSettingException.Type.NUM_ENTRIES; j++) {
-                if (ContentSettingException.getContentSettingsType(j)
-                        == SiteSettingsCategory.contentSettingsType(i)) {
-                    return ContentSettingValues.BLOCK
-                            == website.site().getContentSettingPermission(j);
-                }
+            @ContentSettingValues
+            Integer contentSettingPermission = website.site().getContentSettingPermission(
+                    SiteSettingsCategory.contentSettingsType(i));
+            if (contentSettingPermission != null) {
+                return ContentSettingValues.BLOCK == contentSettingPermission;
             }
             for (@PermissionInfo.Type int j = 0; j < PermissionInfo.Type.NUM_ENTRIES; j++) {
                 if (PermissionInfo.getContentSettingsType(j)

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SiteDataCleaner.java

@@ -24,10 +24,9 @@ public class SiteDataCleaner {
         WebsitePreferenceBridgeJni.get().clearMediaLicenses(browserContextHandle, origin);
 
         // Clear the permissions.
-        for (@ContentSettingException.Type int type = 0;
-                type < ContentSettingException.Type.NUM_ENTRIES; type++) {
-            site.setContentSettingPermission(
-                    browserContextHandle, type, ContentSettingValues.DEFAULT);
+        for (ContentSettingException exception : site.getContentSettingExceptions()) {
+            site.setContentSettingPermission(browserContextHandle,
+                    exception.getContentSettingType(), ContentSettingValues.DEFAULT);
         }
         for (@PermissionInfo.Type int type = 0; type < PermissionInfo.Type.NUM_ENTRIES; type++) {
             site.setPermission(browserContextHandle, type, ContentSettingValues.DEFAULT);

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/Website.java

@@ -17,34 +17,30 @@ import org.chromium.components.embedder_support.browser_context.BrowserContextHa
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Website is a class for storing information about a website and its associated permissions.
  */
-public class Website implements Serializable {
-    static final int INVALID_CAMERA_OR_MICROPHONE_ACCESS = 0;
-    static final int CAMERA_ACCESS_ALLOWED = 1;
-    static final int MICROPHONE_AND_CAMERA_ACCESS_ALLOWED = 2;
-    static final int MICROPHONE_ACCESS_ALLOWED = 3;
-    static final int CAMERA_ACCESS_DENIED = 4;
-    static final int MICROPHONE_AND_CAMERA_ACCESS_DENIED = 5;
-    static final int MICROPHONE_ACCESS_DENIED = 6;
-
+public final class Website implements Serializable {
     private final WebsiteAddress mOrigin;
     private final WebsiteAddress mEmbedder;
 
     /**
-     * Indexed by ContentSettingException.Type.
+     * Indexed by ContentSettingsType.
      */
-    private ContentSettingException mContentSettingException[];
+    private Map<Integer, ContentSettingException> mContentSettingExceptions = new HashMap<>();
+
     /**
      * Indexed by PermissionInfo.Type.
      */
     private PermissionInfo[] mPermissionInfo;
 
     private LocalStorageInfo mLocalStorageInfo;
-    private final List<StorageInfo> mStorageInfo = new ArrayList<StorageInfo>();
+    private final List<StorageInfo> mStorageInfo = new ArrayList<>();
     private int mStorageInfoCallbacksLeft;
 
     // The collection of chooser-based permissions (e.g. USB device access) granted to this site.
@@ -56,8 +52,6 @@ public class Website implements Serializable {
         mOrigin = origin;
         mEmbedder = embedder;
         mPermissionInfo = new PermissionInfo[PermissionInfo.Type.NUM_ENTRIES];
-        mContentSettingException =
-                new ContentSettingException[ContentSettingException.Type.NUM_ENTRIES];
     }
 
     public WebsiteAddress getAddress() {
@@ -164,29 +158,34 @@ public class Website implements Serializable {
         }
     }
 
+    public Collection<ContentSettingException> getContentSettingExceptions() {
+        return mContentSettingExceptions.values();
+    }
+
     /**
      * Returns the exception info for this Website for specified type.
      */
-    public ContentSettingException getContentSettingException(
-            @ContentSettingException.Type int type) {
-        return mContentSettingException[type];
+    public ContentSettingException getContentSettingException(@ContentSettingsType int type) {
+        return mContentSettingExceptions.get(type);
     }
 
     /**
      * Sets the exception info for this Website for specified type.
      */
     public void setContentSettingException(
-            @ContentSettingException.Type int type, ContentSettingException exception) {
-        mContentSettingException[type] = exception;
+            @ContentSettingsType int type, ContentSettingException exception) {
+        mContentSettingExceptions.put(type, exception);
     }
 
     /**
      * Returns what ContentSettingException governs the setting of specified type.
      */
     public @ContentSettingValues @Nullable Integer getContentSettingPermission(
-            @ContentSettingException.Type int type) {
-        return mContentSettingException[type] != null
-                ? mContentSettingException[type].getContentSetting()
+            @ContentSettingsType int type) {
+        // TODO(crbug.com/1103597): Merge with getPermission() when both are keyed by
+        // ContentSettingsType.
+        return getContentSettingException(type) != null
+                ? getContentSettingException(type).getContentSetting()
                 : null;
     }
 
@@ -194,24 +193,28 @@ public class Website implements Serializable {
      * Sets the permission.
      */
     public void setContentSettingPermission(BrowserContextHandle browserContextHandle,
-            @ContentSettingException.Type int type, @ContentSettingValues int value) {
-        if (type == ContentSettingException.Type.ADS) {
+            @ContentSettingsType int type, @ContentSettingValues int value) {
+        // TODO(crbug.com/1103597): Merge with setPermission() when both are keyed by
+        // ContentSettingsType.
+        ContentSettingException exception = getContentSettingException(type);
+        if (type == ContentSettingsType.ADS) {
             // It is possible to set the permission without having an existing exception,
             // because we can show the BLOCK state even when this permission is set to the
             // default. In that case, just set an exception now to BLOCK to enable changing the
             // permission.
-            if (mContentSettingException[type] == null) {
-                mContentSettingException[type] =
-                        new ContentSettingException(ContentSettingsType.ADS,
-                                getAddress().getOrigin(), ContentSettingValues.BLOCK, "");
+            if (exception == null) {
+                exception = new ContentSettingException(ContentSettingsType.ADS,
+                        getAddress().getOrigin(), ContentSettingValues.BLOCK, "");
+                setContentSettingException(type, exception);
             }
-        } else if (type == ContentSettingException.Type.JAVASCRIPT) {
+        } else if (type == ContentSettingsType.JAVASCRIPT) {
             // It is possible to set the permission without having an existing exception,
             // because we show the javascript permission in Site Settings if javascript
             // is blocked by default.
-            if (mContentSettingException[type] == null) {
-                mContentSettingException[type] = new ContentSettingException(
+            if (exception == null) {
+                exception = new ContentSettingException(
                         ContentSettingsType.JAVASCRIPT, getAddress().getHost(), value, "");
+                setContentSettingException(type, exception);
             }
             // It's possible for either action to be emitted. This code path is hit
             // regardless of whether there was an existing permission or not.
@@ -220,12 +223,13 @@ public class Website implements Serializable {
             } else {
                 RecordUserAction.record("JavascriptContentSetting.DisableBy.SiteSettings");
             }
-        } else if (type == ContentSettingException.Type.SOUND) {
+        } else if (type == ContentSettingsType.SOUND) {
             // It is possible to set the permission without having an existing exception,
             // because we always show the sound permission in Site Settings.
-            if (mContentSettingException[type] == null) {
-                mContentSettingException[type] = new ContentSettingException(
+            if (exception == null) {
+                exception = new ContentSettingException(
                         ContentSettingsType.SOUND, getAddress().getHost(), value, "");
+                setContentSettingException(type, exception);
             }
             if (value == ContentSettingValues.BLOCK) {
                 RecordUserAction.record("SoundContentSetting.MuteBy.SiteSettings");
@@ -236,8 +240,8 @@ public class Website implements Serializable {
         // We want to call setContentSetting even after explicitly setting
         // mContentSettingException above because this will trigger the actual change
         // on the PrefServiceBridge.
-        if (mContentSettingException[type] != null) {
-            mContentSettingException[type].setContentSetting(browserContextHandle, value);
+        if (exception != null) {
+            exception.setContentSetting(browserContextHandle, value);
         }
     }
 

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/WebsitePermissionsFetcher.java

@@ -285,20 +285,6 @@ public class WebsitePermissionsFetcher {
     }
 
     private void setException(int contentSettingsType) {
-        @ContentSettingException.Type
-        int exceptionType;
-        for (exceptionType = 0; exceptionType < ContentSettingException.Type.NUM_ENTRIES;
-                exceptionType++) {
-            if (contentSettingsType
-                    == ContentSettingException.getContentSettingsType(exceptionType)) {
-                break;
-            }
-        }
-        assert contentSettingsType
-                == ContentSettingException.getContentSettingsType(exceptionType)
-            : "Unexpected content setting type received: "
-                        + contentSettingsType;
-
         for (ContentSettingException exception :
                 mWebsitePreferenceBridge.getContentSettingsExceptions(
                         mBrowserContextHandle, contentSettingsType)) {
@@ -309,7 +295,7 @@ public class WebsitePermissionsFetcher {
                 continue;
             }
             Website site = findOrCreateSite(address, embedder);
-            site.setContentSettingException(exceptionType, exception);
+            site.setContentSettingException(contentSettingsType, exception);
         }
     }