diff --git a/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java b/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java index 5b778d2a6acd4..74c6651003e50 100644 --- a/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java +++ b/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java @@ -23,6 +23,8 @@ package com.android.internal.config.sysui; */ public final class SystemUiDeviceConfigFlags { + // Flags related to NotificationAssistant + /** * Whether the Notification Assistant should generate replies for notifications. */ @@ -45,5 +47,39 @@ public final class SystemUiDeviceConfigFlags { */ public static final String NAS_MAX_SUGGESTIONS = "nas_max_suggestions"; + // Flags related to Smart Suggestions - these are read in SmartReplyConstants. + + /** (boolean) Whether to enable smart suggestions in notifications. */ + public static final String SSIN_ENABLED = "ssin_enabled"; + + /** + * (boolean) Whether apps need to target at least P to provide their own smart replies (this + * doesn't apply to actions!). + */ + public static final String SSIN_REQUIRES_TARGETING_P = "ssin_requires_targeting_p"; + + /** + * (int) The number of times we'll try to find a better line-break for double-line smart + * suggestion buttons. + */ + public static final String SSIN_MAX_SQUEEZE_REMEASURE_ATTEMPTS = + "ssin_max_squeeze_remeasure_attempts"; + + /** (boolean) Whether to let the user edit smart replies before sending. */ + public static final String SSIN_EDIT_CHOICES_BEFORE_SENDING = + "ssin_edit_choices_before_sending"; + + /** (boolean) Whether smart suggestions should be enabled in heads-up notifications. */ + public static final String SSIN_SHOW_IN_HEADS_UP = "ssin_show_in_heads_up"; + + /** (int) Minimum number of system generated replies to show in a notification. */ + public static final String SSIN_MIN_NUM_SYSTEM_GENERATED_REPLIES = + "ssin_min_num_system_generated_replies"; + + /** + * (int) Maximum number of actions to show in a notification, -1 if there shouldn't be a limit + */ + public static final String SSIN_MAX_NUM_ACTIONS = "ssin_max_num_actions"; + private SystemUiDeviceConfigFlags() { } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyConstants.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyConstants.java index 8b81585836dc9..72f4fc7820a53 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyConstants.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyConstants.java @@ -21,13 +21,14 @@ import static com.android.systemui.Dependency.MAIN_HANDLER_NAME; import android.app.RemoteInput; import android.content.Context; import android.content.res.Resources; -import android.database.ContentObserver; -import android.net.Uri; import android.os.Handler; -import android.provider.Settings; +import android.provider.DeviceConfig; +import android.text.TextUtils; import android.util.KeyValueListParser; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.systemui.R; import javax.inject.Inject; @@ -35,20 +36,10 @@ import javax.inject.Named; import javax.inject.Singleton; @Singleton -public final class SmartReplyConstants extends ContentObserver { +public final class SmartReplyConstants { private static final String TAG = "SmartReplyConstants"; - private static final String KEY_ENABLED = "enabled"; - private static final String KEY_REQUIRES_TARGETING_P = "requires_targeting_p"; - private static final String KEY_MAX_SQUEEZE_REMEASURE_ATTEMPTS = - "max_squeeze_remeasure_attempts"; - private static final String KEY_EDIT_CHOICES_BEFORE_SENDING = - "edit_choices_before_sending"; - private static final String KEY_SHOW_IN_HEADS_UP = "show_in_heads_up"; - private static final String KEY_MIN_NUM_REPLIES = "min_num_system_generated_replies"; - private static final String KEY_MAX_NUM_ACTIONS = "max_num_actions"; - private final boolean mDefaultEnabled; private final boolean mDefaultRequiresP; private final int mDefaultMaxSqueezeRemeasureAttempts; @@ -69,13 +60,13 @@ public final class SmartReplyConstants extends ContentObserver { private volatile int mMinNumSystemGeneratedReplies; private volatile int mMaxNumActions; + private final Handler mHandler; private final Context mContext; private final KeyValueListParser mParser = new KeyValueListParser(','); @Inject public SmartReplyConstants(@Named(MAIN_HANDLER_NAME) Handler handler, Context context) { - super(handler); - + mHandler = handler; mContext = context; final Resources resources = mContext.getResources(); mDefaultEnabled = resources.getBoolean( @@ -93,35 +84,86 @@ public final class SmartReplyConstants extends ContentObserver { mDefaultMaxNumActions = resources.getInteger( R.integer.config_smart_replies_in_notifications_max_num_actions); - mContext.getContentResolver().registerContentObserver( - Settings.Global.getUriFor(Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS), - false, this); + registerDeviceConfigListener(); updateConstants(); } - @Override - public void onChange(boolean selfChange, Uri uri) { + private void registerDeviceConfigListener() { + DeviceConfig.addOnPropertyChangedListener( + DeviceConfig.NAMESPACE_SYSTEMUI, + this::postToHandler, + this::onDeviceConfigPropertyChanged); + } + + private void postToHandler(Runnable r) { + this.mHandler.post(r); + } + + @VisibleForTesting + void onDeviceConfigPropertyChanged(String namespace, String name, String value) { + if (!DeviceConfig.NAMESPACE_SYSTEMUI.equals(namespace)) { + Log.e(TAG, "Received update from DeviceConfig for unrelated namespace: " + + namespace + " " + name + "=" + value); + return; + } + updateConstants(); } private void updateConstants() { synchronized (SmartReplyConstants.this) { - try { - mParser.setString(Settings.Global.getString(mContext.getContentResolver(), - Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS)); - } catch (IllegalArgumentException e) { - Log.e(TAG, "Bad smart reply constants", e); - } - mEnabled = mParser.getBoolean(KEY_ENABLED, mDefaultEnabled); - mRequiresTargetingP = mParser.getBoolean(KEY_REQUIRES_TARGETING_P, mDefaultRequiresP); - mMaxSqueezeRemeasureAttempts = mParser.getInt( - KEY_MAX_SQUEEZE_REMEASURE_ATTEMPTS, mDefaultMaxSqueezeRemeasureAttempts); - mEditChoicesBeforeSending = mParser.getBoolean( - KEY_EDIT_CHOICES_BEFORE_SENDING, mDefaultEditChoicesBeforeSending); - mShowInHeadsUp = mParser.getBoolean(KEY_SHOW_IN_HEADS_UP, mDefaultShowInHeadsUp); - mMinNumSystemGeneratedReplies = - mParser.getInt(KEY_MIN_NUM_REPLIES, mDefaultMinNumSystemGeneratedReplies); - mMaxNumActions = mParser.getInt(KEY_MAX_NUM_ACTIONS, mDefaultMaxNumActions); + mEnabled = readDeviceConfigBooleanOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_ENABLED, + mDefaultEnabled); + mRequiresTargetingP = readDeviceConfigBooleanOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_REQUIRES_TARGETING_P, + mDefaultRequiresP); + mMaxSqueezeRemeasureAttempts = readDeviceConfigIntegerOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_MAX_SQUEEZE_REMEASURE_ATTEMPTS, + mDefaultMaxSqueezeRemeasureAttempts); + mEditChoicesBeforeSending = readDeviceConfigBooleanOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_EDIT_CHOICES_BEFORE_SENDING, + mDefaultEditChoicesBeforeSending); + mShowInHeadsUp = readDeviceConfigBooleanOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_SHOW_IN_HEADS_UP, + mDefaultShowInHeadsUp); + mMinNumSystemGeneratedReplies = readDeviceConfigIntegerOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_MIN_NUM_SYSTEM_GENERATED_REPLIES, + mDefaultMinNumSystemGeneratedReplies); + mMaxNumActions = readDeviceConfigIntegerOrDefaultIfEmpty( + SystemUiDeviceConfigFlags.SSIN_MAX_NUM_ACTIONS, + mDefaultMaxNumActions); + } + } + + private static boolean readDeviceConfigBooleanOrDefaultIfEmpty(String propertyName, + boolean defaultValue) { + String value = DeviceConfig.getProperty(DeviceConfig.NAMESPACE_SYSTEMUI, propertyName); + if (TextUtils.isEmpty(value)) { + return defaultValue; + } + if ("true".equals(value)) { + return true; + } + if ("false".equals(value)) { + return false; + } + // For invalid configs we return the default value. + return defaultValue; + } + + private static int readDeviceConfigIntegerOrDefaultIfEmpty(String propertyName, + int defaultValue) { + String value = DeviceConfig.getProperty(DeviceConfig.NAMESPACE_SYSTEMUI, propertyName); + if (TextUtils.isEmpty(value)) { + return defaultValue; + } + try { + return Integer.parseInt(value); + } catch (NumberFormatException e) { + Log.e(TAG, "Tried to read an integer flag, property name=" + + propertyName + ", value=" + value); + return defaultValue; } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyConstantsTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyConstantsTest.java index bc0f7423eb9f1..cd9069aa12331 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyConstantsTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyConstantsTest.java @@ -23,15 +23,17 @@ import static junit.framework.Assert.assertTrue; import android.app.RemoteInput; import android.os.Handler; import android.os.Looper; -import android.provider.Settings; +import android.provider.DeviceConfig; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.testing.TestableResources; +import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,7 +49,7 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Before public void setUp() { - overrideSetting(null); // No config. + resetAllDeviceConfigFlags(); TestableResources resources = mContext.getOrCreateTestableResources(); resources.addOverride(R.bool.config_smart_replies_in_notifications_enabled, true); resources.addOverride( @@ -58,9 +60,16 @@ public class SmartReplyConstantsTest extends SysuiTestCase { resources.addOverride( R.integer.config_smart_replies_in_notifications_min_num_system_generated_replies, 2); + resources.addOverride( + R.integer.config_smart_replies_in_notifications_max_num_actions, -1); mConstants = new SmartReplyConstants(Handler.createAsync(Looper.myLooper()), mContext); } + @After + public void tearDown() { + resetAllDeviceConfigFlags(); + } + @Test public void testIsEnabledWithNoConfig() { assertTrue(mConstants.isEnabled()); @@ -68,25 +77,25 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testIsEnabledWithInvalidConfig() { - overrideSetting("invalid config"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_ENABLED, "invalid config"); triggerConstantsOnChange(); assertTrue(mConstants.isEnabled()); } @Test public void testIsEnabledWithValidConfig() { - overrideSetting("enabled=false,max_squeeze_remeasure_attempts=5"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_ENABLED, "false"); triggerConstantsOnChange(); assertFalse(mConstants.isEnabled()); } @Test public void testRequiresTargetingPConfig() { - overrideSetting("enabled=true,requires_targeting_p=false"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_REQUIRES_TARGETING_P, "false"); triggerConstantsOnChange(); assertEquals(false, mConstants.requiresTargetingP()); - overrideSetting("enabled=true"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_REQUIRES_TARGETING_P, ""); triggerConstantsOnChange(); assertEquals(true, mConstants.requiresTargetingP()); } @@ -99,21 +108,21 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testGetMaxSqueezeRemeasureAttemptsWithInvalidConfig() { - overrideSetting("invalid config"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_MAX_SQUEEZE_REMEASURE_ATTEMPTS, + "invalid config"); triggerConstantsOnChange(); assertEquals(7, mConstants.getMaxSqueezeRemeasureAttempts()); } @Test public void testGetMaxSqueezeRemeasureAttemptsWithValidConfig() { - overrideSetting("enabled=false,max_squeeze_remeasure_attempts=5"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_MAX_SQUEEZE_REMEASURE_ATTEMPTS, "5"); triggerConstantsOnChange(); assertEquals(5, mConstants.getMaxSqueezeRemeasureAttempts()); } @Test public void testGetEffectiveEditChoicesBeforeSendingWithNoConfig() { - overrideSetting("enabled=true"); triggerConstantsOnChange(); assertFalse( mConstants.getEffectiveEditChoicesBeforeSending( @@ -128,7 +137,7 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testGetEffectiveEditChoicesBeforeSendingWithEnabledConfig() { - overrideSetting("enabled=true,edit_choices_before_sending=true"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_EDIT_CHOICES_BEFORE_SENDING, "true"); triggerConstantsOnChange(); assertTrue( mConstants.getEffectiveEditChoicesBeforeSending( @@ -143,7 +152,7 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testGetEffectiveEditChoicesBeforeSendingWithDisabledConfig() { - overrideSetting("enabled=true,edit_choices_before_sending=false"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_EDIT_CHOICES_BEFORE_SENDING, "false"); triggerConstantsOnChange(); assertFalse( mConstants.getEffectiveEditChoicesBeforeSending( @@ -164,36 +173,18 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testShowInHeadsUpEnabled() { - overrideSetting("enabled=true,show_in_heads_up=true"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_SHOW_IN_HEADS_UP, "true"); triggerConstantsOnChange(); assertTrue(mConstants.getShowInHeadsUp()); } @Test public void testShowInHeadsUpDisabled() { - overrideSetting("enabled=true,show_in_heads_up=false"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_SHOW_IN_HEADS_UP, "false"); triggerConstantsOnChange(); assertFalse(mConstants.getShowInHeadsUp()); } - @Test - public void testMaxNumActionsWithNoConfig() { - assertTrue(mConstants.isEnabled()); - assertEquals(-1, mConstants.getMaxNumActions()); - } - - @Test - public void testMaxNumActionsSet() { - overrideSetting("enabled=true,max_num_actions=10"); - triggerConstantsOnChange(); - assertEquals(10, mConstants.getMaxNumActions()); - } - - private void overrideSetting(String flags) { - Settings.Global.putString(mContext.getContentResolver(), - Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS, flags); - } - @Test public void testGetMinNumSystemGeneratedRepliesWithNoConfig() { assertTrue(mConstants.isEnabled()); @@ -202,15 +193,51 @@ public class SmartReplyConstantsTest extends SysuiTestCase { @Test public void testGetMinNumSystemGeneratedRepliesWithValidConfig() { - overrideSetting("enabled=true,min_num_system_generated_replies=5"); + overrideSetting(SystemUiDeviceConfigFlags.SSIN_MIN_NUM_SYSTEM_GENERATED_REPLIES, "5"); triggerConstantsOnChange(); assertEquals(5, mConstants.getMinNumSystemGeneratedReplies()); } + @Test + public void testMaxNumActionsWithNoConfig() { + assertTrue(mConstants.isEnabled()); + assertEquals(-1, mConstants.getMaxNumActions()); + } + + @Test + public void testMaxNumActionsSet() { + overrideSetting(SystemUiDeviceConfigFlags.SSIN_MAX_NUM_ACTIONS, "10"); + triggerConstantsOnChange(); + assertEquals(10, mConstants.getMaxNumActions()); + } + + private void overrideSetting(String propertyName, String value) { + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + propertyName, value, false /* makeDefault */); + } + private void triggerConstantsOnChange() { - // Since Settings.Global is mocked in TestableContext, we need to manually trigger the - // content observer. - mConstants.onChange(false, - Settings.Global.getUriFor(Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS)); + mConstants.onDeviceConfigPropertyChanged(DeviceConfig.NAMESPACE_SYSTEMUI, + "" /* name */, "" /* value */); + } + + private void resetAllDeviceConfigFlags() { + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_ENABLED, "", false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_REQUIRES_TARGETING_P, "", false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_MAX_SQUEEZE_REMEASURE_ATTEMPTS, "", + false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_EDIT_CHOICES_BEFORE_SENDING, "", + false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_SHOW_IN_HEADS_UP, "", false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_MIN_NUM_SYSTEM_GENERATED_REPLIES, "", + false /* makeDefault */); + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.SSIN_MAX_NUM_ACTIONS, "", false /* makeDefault */); } }