From 40293ab2075c777b4a8ee7cc8104244030809a23 Mon Sep 17 00:00:00 2001 From: Chloris Kuo Date: Thu, 11 Feb 2021 17:12:02 -0800 Subject: [PATCH] NAS Feedback UI Remove developers option settings for feedback UI. Use deviceconfig instead. Test: atest AssistantFeedbackControllerTest, manual on device Bug: 180057146 Change-Id: I624f073253067f70085b7c39218c7cc47af3aac3 --- .../AssistantFeedbackController.java | 67 ++++++++++--------- .../AssistantFeedbackControllerTest.java | 45 ++++++++----- 2 files changed, 63 insertions(+), 49 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java index 0465ebff9aebb..edcf6d4eddb3e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java @@ -18,23 +18,19 @@ package com.android.systemui.statusbar.notification; import static android.service.notification.NotificationListenerService.Ranking; +import static com.android.internal.config.sysui.SystemUiDeviceConfigFlags.ENABLE_NAS_FEEDBACK; + import android.app.NotificationManager; -import android.content.ContentResolver; import android.content.Context; -import android.database.ContentObserver; -import android.net.Uri; import android.os.Handler; -import android.os.Looper; -import android.os.UserHandle; -import android.provider.Settings; +import android.provider.DeviceConfig; import android.util.Pair; -import androidx.annotation.Nullable; - import com.android.internal.R; -import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.util.DeviceConfigProxy; import javax.inject.Inject; @@ -45,10 +41,10 @@ import javax.inject.Inject; * should show an indicator. */ @SysUISingleton -public class AssistantFeedbackController extends ContentObserver { - private final Uri FEEDBACK_URI - = Settings.Global.getUriFor(Settings.Global.NOTIFICATION_FEEDBACK_ENABLED); - private ContentResolver mResolver; +public class AssistantFeedbackController { + private final Context mContext; + private final Handler mHandler; + private final DeviceConfigProxy mDeviceConfigProxy; public static final int STATUS_UNCHANGED = 0; public static final int STATUS_ALERTED = 1; @@ -56,34 +52,39 @@ public class AssistantFeedbackController extends ContentObserver { public static final int STATUS_PROMOTED = 3; public static final int STATUS_DEMOTED = 4; - private boolean mFeedbackEnabled; + private volatile boolean mFeedbackEnabled; + + private final DeviceConfig.OnPropertiesChangedListener mPropertiesChangedListener = + new DeviceConfig.OnPropertiesChangedListener() { + @Override + public void onPropertiesChanged(DeviceConfig.Properties properties) { + if (properties.getKeyset().contains(ENABLE_NAS_FEEDBACK)) { + mFeedbackEnabled = properties.getBoolean( + ENABLE_NAS_FEEDBACK, false); + } + } + }; /** Injected constructor */ @Inject - public AssistantFeedbackController(Context context) { - super(new Handler(Looper.getMainLooper())); - mResolver = context.getContentResolver(); - mResolver.registerContentObserver(FEEDBACK_URI, false, this, UserHandle.USER_ALL); - update(null); + public AssistantFeedbackController(@Main Handler handler, + Context context, DeviceConfigProxy proxy) { + mHandler = handler; + mContext = context; + mDeviceConfigProxy = proxy; + mFeedbackEnabled = mDeviceConfigProxy.getBoolean(DeviceConfig.NAMESPACE_SYSTEMUI, + ENABLE_NAS_FEEDBACK, false); + mDeviceConfigProxy.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_SYSTEMUI, + this::postToHandler, mPropertiesChangedListener); } - @Override - public void onChange(boolean selfChange, @Nullable Uri uri, int flags) { - update(uri); - } - - @VisibleForTesting - public void update(@Nullable Uri uri) { - if (uri == null || FEEDBACK_URI.equals(uri)) { - mFeedbackEnabled = Settings.Global.getInt(mResolver, - Settings.Global.NOTIFICATION_FEEDBACK_ENABLED, 0) - != 0; - } + private void postToHandler(Runnable r) { + this.mHandler.post(r); } /** - * Determines whether to show any user controls related to the assistant. This is based on the - * settings flag {@link Settings.Global.NOTIFICATION_FEEDBACK_ENABLED} + * Determines whether to show any user controls related to the assistant based on the + * DeviceConfig flag value */ public boolean isFeedbackEnabled() { return mFeedbackEnabled; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java index d131dceb70db0..e16d4d71efe7d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java @@ -36,16 +36,20 @@ import static org.junit.Assert.assertTrue; import android.app.Notification; import android.app.NotificationChannel; +import android.os.Handler; import android.os.UserHandle; -import android.provider.Settings; +import android.provider.DeviceConfig; import android.service.notification.StatusBarNotification; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; import android.util.Pair; +import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.systemui.SysuiTestCase; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; +import com.android.systemui.util.DeviceConfigProxyFake; import junit.framework.Assert; @@ -55,38 +59,44 @@ import org.junit.runner.RunWith; @SmallTest @RunWith(AndroidTestingRunner.class) +@TestableLooper.RunWithLooper public class AssistantFeedbackControllerTest extends SysuiTestCase { - private static final int ON = 1; - private static final int OFF = 0; private static final String TEST_PACKAGE_NAME = "test_package"; private static final int TEST_UID = 1; private AssistantFeedbackController mAssistantFeedbackController; + private DeviceConfigProxyFake mProxyFake; + private TestableLooper mTestableLooper; + private StatusBarNotification mSbn; @Before public void setUp() { - mAssistantFeedbackController = new AssistantFeedbackController(mContext); - switchSetting(ON); + mProxyFake = new DeviceConfigProxyFake(); + mTestableLooper = TestableLooper.get(this); + mAssistantFeedbackController = new AssistantFeedbackController( + new Handler(mTestableLooper.getLooper()), + mContext, mProxyFake); mSbn = new StatusBarNotification(TEST_PACKAGE_NAME, TEST_PACKAGE_NAME, 0, null, TEST_UID, 0, new Notification(), UserHandle.CURRENT, null, 0); } @Test - public void testUserControls_settingDisabled() { - switchSetting(OFF); + public void testFlagDisabled() { + switchFlag("false"); assertFalse(mAssistantFeedbackController.isFeedbackEnabled()); } @Test - public void testUserControls_settingEnabled() { + public void testFlagEnabled() { + switchFlag("true"); assertTrue(mAssistantFeedbackController.isFeedbackEnabled()); } @Test - public void testFeedback_settingDisabled() { - switchSetting(OFF); + public void testFeedback_flagDisabled() { + switchFlag("false"); assertEquals(STATUS_UNCHANGED, mAssistantFeedbackController.getFeedbackStatus( getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); assertFalse(mAssistantFeedbackController.showFeedbackIndicator( @@ -95,6 +105,7 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { @Test public void testFeedback_changedImportance() { + switchFlag("true"); NotificationEntry entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_HIGH, RANKING_UNCHANGED); assertEquals(STATUS_PROMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); @@ -110,6 +121,7 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { @Test public void testFeedback_changedRanking() { + switchFlag("true"); NotificationEntry entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_PROMOTED); assertEquals(STATUS_PROMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); @@ -121,8 +133,8 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { } @Test - public void testGetFeedbackResources_settingDisabled() { - switchSetting(OFF); + public void testGetFeedbackResources_flagDisabled() { + switchFlag("false"); Assert.assertEquals(new Pair(0, 0), mAssistantFeedbackController.getFeedbackResources( getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); } @@ -138,9 +150,10 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { .build(); } - private void switchSetting(int setting) { - Settings.Global.putInt(mContext.getContentResolver(), - Settings.Global.NOTIFICATION_FEEDBACK_ENABLED, setting); - mAssistantFeedbackController.update(null); + private void switchFlag(String enabled) { + mProxyFake.setProperty( + DeviceConfig.NAMESPACE_SYSTEMUI, SystemUiDeviceConfigFlags.ENABLE_NAS_FEEDBACK, + enabled, false); + mTestableLooper.processAllMessages(); } }