Merge "DeviceStateRotationLockSettingsManager: only persist setting when value changed" into tm-dev

5 files changed, 269 insertions, 20 deletions

diff --git a/packages/SettingsLib/src/com/android/settingslib/devicestate/AndroidSecureSettings.java b/packages/SettingsLib/src/com/android/settingslib/devicestate/AndroidSecureSettings.java
new file mode 100644
index 000000000000..8aee576c3d04
--- /dev/null
+++ b/packages/SettingsLib/src/com/android/settingslib/devicestate/AndroidSecureSettings.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2022 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.settingslib.devicestate;
+
+import android.content.ContentResolver;
+import android.database.ContentObserver;
+import android.provider.Settings;
+
+/**
+ * Implementation of {@link SecureSettings} that uses Android's {@link Settings.Secure}
+ * implementation.
+ */
+class AndroidSecureSettings implements SecureSettings {
+
+    private final ContentResolver mContentResolver;
+
+    AndroidSecureSettings(ContentResolver contentResolver) {
+        mContentResolver = contentResolver;
+    }
+
+    @Override
+    public void putStringForUser(String name, String value, int userHandle) {
+        Settings.Secure.putStringForUser(mContentResolver, name, value, userHandle);
+    }
+
+    @Override
+    public String getStringForUser(String name, int userHandle) {
+        return Settings.Secure.getStringForUser(mContentResolver, name, userHandle);
+    }
+
+    @Override
+    public void registerContentObserver(String name, boolean notifyForDescendants,
+            ContentObserver observer, int userHandle) {
+        mContentResolver.registerContentObserver(
+                Settings.Secure.getUriFor(name),
+                notifyForDescendants,
+                observer,
+                userHandle);
+    }
+}
diff --git a/packages/SettingsLib/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManager.java b/packages/SettingsLib/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManager.java
index afd3626ab889..961fab32fc2c 100644
--- a/packages/SettingsLib/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManager.java
+++ b/packages/SettingsLib/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManager.java
@@ -24,6 +24,7 @@ import android.content.ContentResolver;
 import android.content.Context;
 import android.database.ContentObserver;
 import android.os.Handler;
+import android.os.Looper;
 import android.os.UserHandle;
 import android.provider.Settings;
 import android.text.TextUtils;
@@ -47,28 +48,33 @@ public final class DeviceStateRotationLockSettingsManager {
 
     private static DeviceStateRotationLockSettingsManager sSingleton;
 
-    private final ContentResolver mContentResolver;
     private final String[] mDeviceStateRotationLockDefaults;
-    private final Handler mMainHandler = Handler.getMain();
+    private final Handler mMainHandler = new Handler(Looper.getMainLooper());
     private final Set<DeviceStateRotationLockSettingsListener> mListeners = new HashSet<>();
+    private final SecureSettings mSecureSettings;
     private SparseIntArray mDeviceStateRotationLockSettings;
     private SparseIntArray mDeviceStateRotationLockFallbackSettings;
+    private String mLastSettingValue;
 
-    private DeviceStateRotationLockSettingsManager(Context context) {
-        mContentResolver = context.getContentResolver();
+    @VisibleForTesting
+    DeviceStateRotationLockSettingsManager(Context context, SecureSettings secureSettings) {
+        this.mSecureSettings = secureSettings;
         mDeviceStateRotationLockDefaults =
                 context.getResources()
                         .getStringArray(R.array.config_perDeviceStateRotationLockDefaults);
         loadDefaults();
         initializeInMemoryMap();
-        listenForSettingsChange(context);
+        listenForSettingsChange();
     }
 
     /** Returns a singleton instance of this class */
     public static synchronized DeviceStateRotationLockSettingsManager getInstance(Context context) {
         if (sSingleton == null) {
+            Context applicationContext = context.getApplicationContext();
+            ContentResolver contentResolver = applicationContext.getContentResolver();
+            SecureSettings secureSettings = new AndroidSecureSettings(contentResolver);
             sSingleton =
-                    new DeviceStateRotationLockSettingsManager(context.getApplicationContext());
+                    new DeviceStateRotationLockSettingsManager(applicationContext, secureSettings);
         }
         return sSingleton;
     }
@@ -81,11 +87,11 @@ public final class DeviceStateRotationLockSettingsManager {
                 > 0;
     }
 
-    private void listenForSettingsChange(Context context) {
-        context.getContentResolver()
+    private void listenForSettingsChange() {
+        mSecureSettings
                 .registerContentObserver(
-                        Settings.Secure.getUriFor(Settings.Secure.DEVICE_STATE_ROTATION_LOCK),
-                        /* notifyForDescendents= */ false, //NOTYPO
+                        Settings.Secure.DEVICE_STATE_ROTATION_LOCK,
+                        /* notifyForDescendants= */ false,
                         new ContentObserver(mMainHandler) {
                             @Override
                             public void onChange(boolean selfChange) {
@@ -182,8 +188,7 @@ public final class DeviceStateRotationLockSettingsManager {
 
     private void initializeInMemoryMap() {
         String serializedSetting =
-                Settings.Secure.getStringForUser(
-                        mContentResolver,
+                mSecureSettings.getStringForUser(
                         Settings.Secure.DEVICE_STATE_ROTATION_LOCK,
                         UserHandle.USER_CURRENT);
         if (TextUtils.isEmpty(serializedSetting)) {
@@ -222,11 +227,7 @@ public final class DeviceStateRotationLockSettingsManager {
 
     private void persistSettings() {
         if (mDeviceStateRotationLockSettings.size() == 0) {
-            Settings.Secure.putStringForUser(
-                    mContentResolver,
-                    Settings.Secure.DEVICE_STATE_ROTATION_LOCK,
-                    /* value= */ "",
-                    UserHandle.USER_CURRENT);
+            persistSettingIfChanged(/* newSettingValue= */ "");
             return;
         }
 
@@ -243,10 +244,17 @@ public final class DeviceStateRotationLockSettingsManager {
                     .append(SEPARATOR_REGEX)
                     .append(mDeviceStateRotationLockSettings.valueAt(i));
         }
-        Settings.Secure.putStringForUser(
-                mContentResolver,
+        persistSettingIfChanged(stringBuilder.toString());
+    }
+
+    private void persistSettingIfChanged(String newSettingValue) {
+        if (TextUtils.equals(mLastSettingValue, newSettingValue)) {
+            return;
+        }
+        mLastSettingValue = newSettingValue;
+        mSecureSettings.putStringForUser(
                 Settings.Secure.DEVICE_STATE_ROTATION_LOCK,
-                stringBuilder.toString(),
+                /* value= */ newSettingValue,
                 UserHandle.USER_CURRENT);
     }
 
diff --git a/packages/SettingsLib/src/com/android/settingslib/devicestate/SecureSettings.java b/packages/SettingsLib/src/com/android/settingslib/devicestate/SecureSettings.java
new file mode 100644
index 000000000000..10528739b2b0
--- /dev/null
+++ b/packages/SettingsLib/src/com/android/settingslib/devicestate/SecureSettings.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2022 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.settingslib.devicestate;
+
+import android.database.ContentObserver;
+
+/** Minimal wrapper interface around {@link android.provider.Settings.Secure} for easier testing. */
+interface SecureSettings {
+
+    void putStringForUser(String name, String value, int userHandle);
+
+    String getStringForUser(String name, int userHandle);
+
+    void registerContentObserver(String name, boolean notifyForDescendants,
+            ContentObserver settingsObserver, int userHandle);
+}
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManagerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManagerTest.java
new file mode 100644
index 000000000000..1a45384bc768
--- /dev/null
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/DeviceStateRotationLockSettingsManagerTest.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2022 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.settingslib.devicestate;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.Mockito.when;
+
+import android.content.Context;
+import android.content.res.Resources;
+import android.database.ContentObserver;
+import android.os.UserHandle;
+import android.provider.Settings;
+
+import androidx.test.InstrumentationRegistry;
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class DeviceStateRotationLockSettingsManagerTest {
+
+    @Mock private Context mMockContext;
+    @Mock private Resources mMockResources;
+
+    private DeviceStateRotationLockSettingsManager mManager;
+    private int mNumSettingsChanges = 0;
+    private final ContentObserver mContentObserver = new ContentObserver(null) {
+        @Override
+        public void onChange(boolean selfChange) {
+            mNumSettingsChanges++;
+        }
+    };
+    private final FakeSecureSettings mFakeSecureSettings = new FakeSecureSettings();
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        Context context = InstrumentationRegistry.getTargetContext();
+        when(mMockContext.getApplicationContext()).thenReturn(mMockContext);
+        when(mMockContext.getResources()).thenReturn(mMockResources);
+        when(mMockContext.getContentResolver()).thenReturn(context.getContentResolver());
+        mFakeSecureSettings.registerContentObserver(
+                Settings.Secure.DEVICE_STATE_ROTATION_LOCK,
+                /* notifyForDescendents= */ false, //NOTYPO
+                mContentObserver,
+                UserHandle.USER_CURRENT);
+        mManager = new DeviceStateRotationLockSettingsManager(context, mFakeSecureSettings);
+    }
+
+    @Test
+    public void initialization_settingsAreChangedOnce() {
+        assertThat(mNumSettingsChanges).isEqualTo(1);
+    }
+
+    @Test
+    public void updateSetting_multipleTimes_sameValue_settingsAreChangedOnlyOnce() {
+        mNumSettingsChanges = 0;
+
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ true);
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ true);
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ true);
+
+        assertThat(mNumSettingsChanges).isEqualTo(1);
+    }
+
+    @Test
+    public void updateSetting_multipleTimes_differentValues_settingsAreChangedMultipleTimes() {
+        mNumSettingsChanges = 0;
+
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ true);
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ false);
+        mManager.updateSetting(/* deviceState= */ 1, /* rotationLocked= */ true);
+
+        assertThat(mNumSettingsChanges).isEqualTo(3);
+    }
+}
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/FakeSecureSettings.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/FakeSecureSettings.java
new file mode 100644
index 000000000000..91baa68a1c49
--- /dev/null
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/devicestate/FakeSecureSettings.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2022 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.settingslib.devicestate;
+
+import android.database.ContentObserver;
+import android.util.Pair;
+
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/** Fake implementation of {@link SecureSettings} that stores everything in memory. */
+class FakeSecureSettings implements SecureSettings {
+
+    private final Map<SettingsKey, String> mValues = new HashMap<>();
+    private final Multimap<SettingsKey, ContentObserver> mContentObservers = HashMultimap.create();
+
+    @Override
+    public void putStringForUser(String name, String value, int userHandle) {
+        SettingsKey settingsKey = new SettingsKey(userHandle, name);
+        mValues.put(settingsKey, value);
+        for (ContentObserver observer : mContentObservers.get(settingsKey)) {
+            observer.onChange(/* selfChange= */ false);
+        }
+    }
+
+    @Override
+    public String getStringForUser(String name, int userHandle) {
+        return mValues.getOrDefault(new SettingsKey(userHandle, name), "");
+    }
+
+    @Override
+    public void registerContentObserver(String name, boolean notifyForDescendants,
+            ContentObserver settingsObserver, int userHandle) {
+        mContentObservers.put(new SettingsKey(userHandle, name), settingsObserver);
+    }
+
+    private static class SettingsKey extends Pair<Integer, String> {
+
+        SettingsKey(Integer userHandle, String settingName) {
+            super(userHandle, settingName);
+        }
+    }
+}