Merge "Move contents of FeatureFlags into FeatureFlagManager." into sc-v2-dev

packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt

@@ -19,6 +19,11 @@ package com.android.systemui.flags
  * Plugin for loading flag values
  */
 interface FlagReader {
+    /** Returns a boolean value for the given flag.  */
+    fun isEnabled(flag: BooleanFlag): Boolean {
+        return flag.default
+    }
+
     /** Returns a boolean value for the given flag.  */
     fun isEnabled(id: Int, def: Boolean): Boolean {
         return def

packages/SystemUI/src-debug/com/android/systemui/flags/FeatureFlagManager.java

@@ -26,13 +26,16 @@ import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
+import android.content.res.Resources;
 import android.os.Bundle;
 import android.util.Log;
 
+import androidx.annotation.BoolRes;
 import androidx.annotation.NonNull;
 
 import com.android.systemui.Dumpable;
 import com.android.systemui.dagger.SysUISingleton;
+import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.dump.DumpManager;
 import com.android.systemui.util.settings.SecureSettings;
 
@@ -62,14 +65,19 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable {
 
     private final FlagManager mFlagManager;
     private final SecureSettings mSecureSettings;
+    private final Resources mResources;
     private final Map<Integer, Boolean> mBooleanFlagCache = new HashMap<>();
 
     @Inject
-    public FeatureFlagManager(FlagManager flagManager,
+    public FeatureFlagManager(
-            SecureSettings secureSettings, Context context,
+            FlagManager flagManager,
+            Context context,
+            SecureSettings secureSettings,
+            @Main Resources resources,
             DumpManager dumpManager) {
         mFlagManager = flagManager;
         mSecureSettings = secureSettings;
+        mResources = resources;
         IntentFilter filter = new IntentFilter();
         filter.addAction(ACTION_SET_FLAG);
         filter.addAction(ACTION_GET_FLAGS);
@@ -77,17 +85,32 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable {
         dumpManager.registerDumpable(TAG, this);
     }
 
-    /** Return a {@link BooleanFlag}'s value. */
     @Override
-    public boolean isEnabled(int id, boolean defaultValue) {
+    public boolean isEnabled(BooleanFlag flag) {
+        int id = flag.getId();
         if (!mBooleanFlagCache.containsKey(id)) {
-            Boolean result = isEnabledInternal(id);
+            boolean def = flag.getDefault();
-            mBooleanFlagCache.put(id, result == null ? defaultValue : result);
+            if (flag.hasResourceOverride()) {
+                try {
+                    def = isEnabledInOverlay(flag.getResourceOverride());
+                } catch (Resources.NotFoundException e) {
+                    // no-op
+                }
+            }
+
+            mBooleanFlagCache.put(id, isEnabled(id, def));
         }
 
         return mBooleanFlagCache.get(id);
     }
 
+    /** Return a {@link BooleanFlag}'s value. */
+    @Override
+    public boolean isEnabled(int id, boolean defaultValue) {
+        Boolean result = isEnabledInternal(id);
+        return result == null ? defaultValue : result;
+    }
+
     /** Returns the stored value or null if not set. */
     private Boolean isEnabledInternal(int id) {
         try {
@@ -98,6 +121,10 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable {
         return null;
     }
 
+    private boolean isEnabledInOverlay(@BoolRes int resId) {
+        return mResources.getBoolean(resId);
+    }
+
     /** Set whether a given {@link BooleanFlag} is enabled or not. */
     @Override
     public void setEnabled(int id, boolean value) {

packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java

@@ -16,7 +16,6 @@
 
 package com.android.systemui.flags;
 
-import android.content.Context;
 import android.util.SparseBooleanArray;
 
 import androidx.annotation.NonNull;
@@ -24,7 +23,6 @@ import androidx.annotation.NonNull;
 import com.android.systemui.Dumpable;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dump.DumpManager;
-import com.android.systemui.util.settings.SecureSettings;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
@@ -41,8 +39,7 @@ import javax.inject.Inject;
 public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable {
     SparseBooleanArray mAccessedFlags = new SparseBooleanArray();
     @Inject
-    public FeatureFlagManager(
+    public FeatureFlagManager(DumpManager dumpManager) {
-            SecureSettings secureSettings, Context context, DumpManager dumpManager) {
         dumpManager.registerDumpable("SysUIFlags", this);
     }
 
@@ -52,6 +49,11 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable {
     @Override
     public void removeListener(Listener run) {}
 
+    @Override
+    public boolean isEnabled(BooleanFlag flag) {
+        return isEnabled(flag.getId(), flag.getDefault());
+    }
+
     @Override
     public boolean isEnabled(int key, boolean defaultValue) {
         mAccessedFlags.append(key, defaultValue);

packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java

@@ -17,22 +17,11 @@
 package com.android.systemui.flags;
 
 import android.content.Context;
-import android.content.res.Resources;
 import android.util.FeatureFlagUtils;
 import android.util.Log;
-import android.util.SparseArray;
 import android.widget.Toast;
 
-import androidx.annotation.BoolRes;
-
-import com.android.internal.annotations.VisibleForTesting;
 import com.android.systemui.dagger.SysUISingleton;
-import com.android.systemui.dagger.qualifiers.Main;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 
 import javax.inject.Inject;
 
@@ -43,31 +32,13 @@ import javax.inject.Inject;
  */
 @SysUISingleton
 public class FeatureFlags {
-    private final Resources mResources;
     private final FlagReader mFlagReader;
     private final Context mContext;
-    private final Map<Integer, Flag<?>> mFlagMap = new HashMap<>();
-    private final Map<Integer, List<Listener>> mListeners = new HashMap<>();
-    private final SparseArray<Boolean> mCachedFlags = new SparseArray<>();
 
     @Inject
-    public FeatureFlags(@Main Resources resources, FlagReader flagReader, Context context) {
+    public FeatureFlags(FlagReader flagReader, Context context) {
-        mResources = resources;
         mFlagReader = flagReader;
         mContext = context;
-
-        flagReader.addListener(mListener);
-    }
-
-    private final FlagReader.Listener mListener = id -> {
-        if (mListeners.containsKey(id) && mFlagMap.containsKey(id)) {
-            mListeners.get(id).forEach(listener -> listener.onFlagChanged(mFlagMap.get(id)));
-        }
-    };
-
-    @VisibleForTesting
-    void addFlag(Flag<?> flag) {
-        mFlagMap.put(flag.getId(), flag);
     }
 
     /**
@@ -75,32 +46,7 @@ public class FeatureFlags {
      * @return The value of the flag.
      */
     public boolean isEnabled(BooleanFlag flag) {
-        boolean def = flag.getDefault();
+        return mFlagReader.isEnabled(flag);
-        if (flag.hasResourceOverride()) {
-            try {
-                def = isEnabledInOverlay(flag.getResourceOverride());
-            } catch (Resources.NotFoundException e) {
-                // no-op
-            }
-        }
-        return mFlagReader.isEnabled(flag.getId(), def);
-    }
-
-    /**
-     * @param flag The {@link IntFlag} of interest.
-
-    /** Add a listener for a specific flag. */
-    public void addFlagListener(Flag<?> flag, Listener listener) {
-        mListeners.putIfAbsent(flag.getId(), new ArrayList<>());
-        mListeners.get(flag.getId()).add(listener);
-        mFlagMap.putIfAbsent(flag.getId(), flag);
-    }
-
-    /** Remove a listener for a specific flag. */
-    public void removeFlagListener(Flag<?> flag, Listener listener) {
-        if (mListeners.containsKey(flag.getId())) {
-            mListeners.get(flag.getId()).remove(listener);
-        }
     }
 
     public void assertLegacyPipelineEnabled() {
@@ -205,20 +151,4 @@ public class FeatureFlags {
     public static boolean isProviderModelSettingEnabled(Context context) {
         return FeatureFlagUtils.isEnabled(context, FeatureFlagUtils.SETTINGS_PROVIDER_MODEL);
     }
-
-    private boolean isEnabledInOverlay(@BoolRes int resId) {
-        synchronized (mCachedFlags) {
-            if (!mCachedFlags.contains(resId)) {
-                mCachedFlags.put(resId, mResources.getBoolean(resId));
-            }
-
-            return mCachedFlags.get(resId);
-        }
-    }
-
-    /** Simple interface for beinga alerted when a specific flag changes value. */
-    public interface Listener {
-        /** */
-        void onFlagChanged(Flag<?> flag);
-    }
 }

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java

@@ -53,8 +53,6 @@ import java.io.StringWriter;
 public class FeatureFlagManagerTest extends SysuiTestCase {
     FeatureFlagManager mFeatureFlagManager;
 
-    @Mock private FlagManager mFlagManager;
-    @Mock private SecureSettings mSecureSettings;
     @Mock private Context mContext;
     @Mock private DumpManager mDumpManager;
 
@@ -62,14 +60,11 @@ public class FeatureFlagManagerTest extends SysuiTestCase {
     public void setup() {
         MockitoAnnotations.initMocks(this);
 
-        mFeatureFlagManager = new FeatureFlagManager(mSecureSettings, mContext, mDumpManager);
+        mFeatureFlagManager = new FeatureFlagManager(mDumpManager);
     }
 
     @After
     public void onFinished() {
-        // SecureSettings and Context are provided for constructor consistency with the
-        // debug version of the FeatureFlagManager, but should never be used.
-        verifyZeroInteractions(mSecureSettings, mContext);
         // The dump manager should be registered with even for the release version, but that's it.
         verify(mDumpManager).registerDumpable(anyString(), any());
         verifyNoMoreInteractions(mDumpManager);

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsTest.java

@@ -1,138 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.systemui.flags;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import static org.mockito.ArgumentMatchers.anyBoolean;
-import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import android.content.res.Resources;
-
-import androidx.test.filters.SmallTest;
-
-import com.android.systemui.SysuiTestCase;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.mockito.stubbing.Answer;
-
-@SmallTest
-public class FeatureFlagsTest extends SysuiTestCase {
-
-    @Mock Resources mResources;
-    @Mock FlagReader mFeatureFlagReader;
-
-    private FeatureFlags mFeatureFlags;
-
-    @Before
-    public void setup() {
-        MockitoAnnotations.initMocks(this);
-
-        when(mFeatureFlagReader.isEnabled(anyInt(), anyBoolean())).thenAnswer(
-                (Answer<Boolean>) invocation -> invocation.getArgument(1));
-
-        mFeatureFlags = new FeatureFlags(mResources, mFeatureFlagReader, getContext());
-    }
-
-    @Test
-    public void testAddListener() {
-        Flag<?> flag = new BooleanFlag(1);
-        mFeatureFlags.addFlag(flag);
-
-        // Assert and capture that a plugin listener was added.
-        ArgumentCaptor<FlagReader.Listener> pluginListenerCaptor =
-                ArgumentCaptor.forClass(FlagReader.Listener.class);
-        verify(mFeatureFlagReader).addListener(pluginListenerCaptor.capture());
-        FlagReader.Listener pluginListener = pluginListenerCaptor.getValue();
-
-        // Signal a change. No listeners, so no real effect.
-        pluginListener.onFlagChanged(flag.getId());
-
-        // Add a listener for the flag
-        final Flag<?>[] changedFlag = {null};
-        FeatureFlags.Listener listener = f -> changedFlag[0] = f;
-        mFeatureFlags.addFlagListener(flag, listener);
-
-        // No changes seen yet.
-        assertThat(changedFlag[0]).isNull();
-
-        // Signal a change.
-        pluginListener.onFlagChanged(flag.getId());
-
-        // Assert that the change was for the correct flag.
-        assertThat(changedFlag[0]).isEqualTo(flag);
-    }
-
-    @Test
-    public void testRemoveListener() {
-        Flag<?> flag = new BooleanFlag(1);
-        mFeatureFlags.addFlag(flag);
-
-        // Assert and capture that a plugin listener was added.
-        ArgumentCaptor<FlagReader.Listener> pluginListenerCaptor =
-                ArgumentCaptor.forClass(FlagReader.Listener.class);
-        verify(mFeatureFlagReader).addListener(pluginListenerCaptor.capture());
-        FlagReader.Listener pluginListener = pluginListenerCaptor.getValue();
-
-        // Add a listener for the flag
-        final Flag<?>[] changedFlag = {null};
-        FeatureFlags.Listener listener = f -> changedFlag[0] = f;
-        mFeatureFlags.addFlagListener(flag, listener);
-
-        // Signal a change.
-        pluginListener.onFlagChanged(flag.getId());
-
-        // Assert that the change was for the correct flag.
-        assertThat(changedFlag[0]).isEqualTo(flag);
-
-        changedFlag[0] = null;
-
-        // Now remove the listener.
-        mFeatureFlags.removeFlagListener(flag, listener);
-        // Signal a change.
-        pluginListener.onFlagChanged(flag.getId());
-        // Assert that the change was not triggered
-        assertThat(changedFlag[0]).isNull();
-    }
-
-    @Test
-    public void testBooleanDefault() {
-        BooleanFlag flag = new BooleanFlag(1, true);
-
-        mFeatureFlags.addFlag(flag);
-
-        assertThat(mFeatureFlags.isEnabled(flag)).isTrue();
-    }
-
-    @Test
-    public void testBooleanResourceOverlay() {
-        int resourceId = 12;
-        BooleanFlag flag = new BooleanFlag(1, false, resourceId);
-        when(mResources.getBoolean(resourceId)).thenReturn(true);
-        when(mResources.getResourceEntryName(resourceId)).thenReturn("flag");
-
-        mFeatureFlags.addFlag(flag);
-
-        assertThat(mFeatureFlags.isEnabled(flag)).isTrue();
-    }
-}