[MainSwitchPreference] OnPreferenceChangeListener does not work

Bug: 400335057
Fix: 400334550
Flag: EXEMPT bugfix
Test: Unit&Manual test
Change-Id: I63064a3c02ae3b616be8a2876653016fd8d29977

2 files changed, 55 insertions, 2 deletions

diff --git a/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java b/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java
index d883fb0594e6..5170581aa382 100644
--- a/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java
+++ b/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java
@@ -80,7 +80,14 @@ public class MainSwitchPreference extends TwoStatePreference implements OnChecke
         mainSwitchBar.setIconSpaceReserved(isIconSpaceReserved());
         // To support onPreferenceChange callback, it needs to call callChangeListener() when
         // MainSwitchBar is clicked.
-        mainSwitchBar.setOnClickListener(view -> callChangeListener(isChecked()));
+        mainSwitchBar.setOnClickListener(view -> {
+            boolean isChecked = isChecked();
+            if (!callChangeListener(isChecked)) {
+                // Change checked state back if listener doesn't like it.
+                // Note that CompoundButton maintains internal state to avoid infinite recursion.
+                mainSwitchBar.setChecked(!isChecked);
+            }
+        });
 
         // Remove all listeners to 1. avoid triggering listener when update UI 2. prevent potential
         // listener leaking when the view holder is reused by RecyclerView
@@ -88,7 +95,11 @@ public class MainSwitchPreference extends TwoStatePreference implements OnChecke
         mainSwitchBar.setChecked(isChecked());
         mainSwitchBar.addOnSwitchChangeListener(this);
 
-        mainSwitchBar.show();
+        if (isVisible()) {
+            mainSwitchBar.show();
+        } else {
+            mainSwitchBar.hide();
+        }
     }
 
     @Override
@@ -101,7 +112,10 @@ public class MainSwitchPreference extends TwoStatePreference implements OnChecke
 
     /**
      * Adds a listener for switch changes
+     *
+     * @deprecated Use {@link #setOnPreferenceChangeListener(OnPreferenceChangeListener)}
      */
+    @Deprecated
     public void addOnSwitchChangeListener(OnCheckedChangeListener listener) {
         if (!mSwitchChangeListeners.contains(listener)) {
             mSwitchChangeListeners.add(listener);
@@ -110,7 +124,10 @@ public class MainSwitchPreference extends TwoStatePreference implements OnChecke
 
     /**
      * Remove a listener for switch changes
+     *
+     * @deprecated Use {@link #setOnPreferenceChangeListener(OnPreferenceChangeListener)}
      */
+    @Deprecated
     public void removeOnSwitchChangeListener(OnCheckedChangeListener listener) {
         mSwitchChangeListeners.remove(listener);
     }
diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/widget/MainSwitchPreferenceTest.java b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/widget/MainSwitchPreferenceTest.java
index a47b4d5c354f..093833ec41cf 100644
--- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/widget/MainSwitchPreferenceTest.java
+++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/widget/MainSwitchPreferenceTest.java
@@ -18,13 +18,22 @@ package com.android.settingslib.widget;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import android.content.Context;
 import android.view.View;
 import android.widget.TextView;
 
+import androidx.preference.PreferenceDataStore;
 import androidx.preference.PreferenceViewHolder;
 import androidx.test.core.app.ApplicationProvider;
 
+import com.android.settingslib.preference.PreferenceScreenFactory;
 import com.android.settingslib.widget.mainswitch.R;
 
 import org.junit.Before;
@@ -67,4 +76,31 @@ public class MainSwitchPreferenceTest {
         assertThat(mRootView.<MainSwitchBar>requireViewById(
                 R.id.settingslib_main_switch_bar).isChecked()).isTrue();
     }
+
+    @Test
+    public void setOnPreferenceChangeListener() {
+        // Attach preference to preference screen, otherwise `Preference.performClick` does not
+        // interact with underlying datastore
+        new PreferenceScreenFactory(mContext).getOrCreatePreferenceScreen().addPreference(
+                mPreference);
+
+        PreferenceDataStore preferenceDataStore = mock(PreferenceDataStore.class);
+        // always return the provided default value
+        when(preferenceDataStore.getBoolean(any(), anyBoolean())).thenAnswer(
+                invocation -> invocation.getArguments()[1]);
+        mPreference.setPreferenceDataStore(preferenceDataStore);
+
+        String key = "key";
+        mPreference.setKey(key);
+        mPreference.setOnPreferenceChangeListener((preference, newValue) -> false);
+        mPreference.onBindViewHolder(mHolder);
+
+        mPreference.performClick();
+        verify(preferenceDataStore, never()).putBoolean(any(), anyBoolean());
+
+        mPreference.setOnPreferenceChangeListener((preference, newValue) -> true);
+
+        mPreference.performClick();
+        verify(preferenceDataStore).putBoolean(any(), anyBoolean());
+    }
 }