Fix MainSwitchPreference

This change simplifies MainSwitchPreference widget significantly and fix
some issues:

- Override setTitle/setSummary/etc to update UI is incorrect.
  Under the hood, notifyChange will be invoked when any change is
  detected and onBindViewHolder is triggered. All the UI binding logic
  must be done within onBindViewHolder.
- The listener is added to MainSwitchBar but never be removed, which
  will cause weird issue (e.g. ag/3645453) when view holder is reused.

Bug: 391092187
Flag: EXEMPT library
Test: atest&manual
Change-Id: I4fc5ad382253c7dade1165487142078e56164941

4 files changed, 51 insertions, 117 deletions

diff --git a/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchBar.java b/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchBar.java
index 73728bcd1ff7..7bd4b3f771ab 100644
--- a/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchBar.java
+++ b/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchBar.java
@@ -240,6 +240,11 @@ public class MainSwitchBar extends LinearLayout implements OnCheckedChangeListen
         }
     }
 
+    /** Removes all [OnCheckedChangeListener]s. */
+    public void removeAllOnSwitchChangeListeners() {
+        mSwitchChangeListeners.clear();
+    }
+
     /**
      * Remove a listener for switch changes
      */
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 83858d9c9c54..d883fb0594e6 100644
--- a/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java
+++ b/packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java
@@ -17,12 +17,13 @@
 package com.android.settingslib.widget;
 
 import android.content.Context;
-import android.content.res.TypedArray;
 import android.os.Build;
 import android.util.AttributeSet;
 import android.widget.CompoundButton;
 import android.widget.CompoundButton.OnCheckedChangeListener;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.preference.PreferenceViewHolder;
 import androidx.preference.TwoStatePreference;
 
@@ -34,124 +35,67 @@ import java.util.List;
 /**
  * MainSwitchPreference is a Preference with a customized Switch.
  * This component is used as the main switch of the page
- * to enable or disable the prefereces on the page.
+ * to enable or disable the preferences on the page.
  */
-public class MainSwitchPreference extends TwoStatePreference
-        implements OnCheckedChangeListener, GroupSectionDividerMixin {
+public class MainSwitchPreference extends TwoStatePreference implements OnCheckedChangeListener,
+        GroupSectionDividerMixin {
 
     private final List<OnCheckedChangeListener> mSwitchChangeListeners = new ArrayList<>();
 
-    private MainSwitchBar mMainSwitchBar;
-
-    public MainSwitchPreference(Context context) {
-        super(context);
-        init(context, null);
+    public MainSwitchPreference(@NonNull Context context) {
+        this(context, null);
     }
 
-    public MainSwitchPreference(Context context, AttributeSet attrs) {
-        super(context, attrs);
-        init(context, attrs);
+    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs) {
+        this(context, attrs, 0);
     }
 
-    public MainSwitchPreference(Context context, AttributeSet attrs, int defStyleAttr) {
-        super(context, attrs, defStyleAttr);
-        init(context, attrs);
+    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs,
+            int defStyleAttr) {
+        this(context, attrs, defStyleAttr, 0);
     }
 
-    public MainSwitchPreference(Context context, AttributeSet attrs, int defStyleAttr,
-            int defStyleRes) {
+    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs,
+            int defStyleAttr, int defStyleRes) {
         super(context, attrs, defStyleAttr, defStyleRes);
-        init(context, attrs);
+        boolean isExpressive = SettingsThemeHelper.isExpressiveTheme(context);
+        int resId = isExpressive ? R.layout.settingslib_expressive_main_switch_layout
+                : R.layout.settingslib_main_switch_layout;
+        setLayoutResource(resId);
     }
 
     @Override
-    public void onBindViewHolder(PreferenceViewHolder holder) {
+    public void onBindViewHolder(@NonNull PreferenceViewHolder holder) {
         super.onBindViewHolder(holder);
 
         holder.setDividerAllowedAbove(false);
         holder.setDividerAllowedBelow(false);
 
-        mMainSwitchBar = (MainSwitchBar) holder.findViewById(R.id.settingslib_main_switch_bar);
+        MainSwitchBar mainSwitchBar = holder.itemView.requireViewById(
+                R.id.settingslib_main_switch_bar);
+        mainSwitchBar.setTitle(getTitle());
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) {
+            mainSwitchBar.setSummary(getSummary());
+        }
+        mainSwitchBar.setIconSpaceReserved(isIconSpaceReserved());
         // To support onPreferenceChange callback, it needs to call callChangeListener() when
         // MainSwitchBar is clicked.
-        mMainSwitchBar.setOnClickListener((view) -> callChangeListener(isChecked()));
-        setIconSpaceReserved(isIconSpaceReserved());
-        updateStatus(isChecked());
-        registerListenerToSwitchBar();
-    }
+        mainSwitchBar.setOnClickListener(view -> callChangeListener(isChecked()));
 
-    private void init(Context context, AttributeSet attrs) {
-        boolean isExpressive = SettingsThemeHelper.isExpressiveTheme(context);
-        int resId = isExpressive
-                ? R.layout.settingslib_expressive_main_switch_layout
-                : R.layout.settingslib_main_switch_layout;
-        setLayoutResource(resId);
-        mSwitchChangeListeners.add(this);
-        if (attrs != null) {
-            final TypedArray a = context.obtainStyledAttributes(attrs,
-                    androidx.preference.R.styleable.Preference, 0 /*defStyleAttr*/,
-                    0 /*defStyleRes*/);
-            final CharSequence title = a.getText(
-                    androidx.preference.R.styleable.Preference_android_title);
-            setTitle(title);
-
-            CharSequence summary = a.getText(
-                    androidx.preference.R.styleable.Preference_android_summary);
-            setSummary(summary);
-
-            final boolean bIconSpaceReserved = a.getBoolean(
-                    androidx.preference.R.styleable.Preference_android_iconSpaceReserved, true);
-            setIconSpaceReserved(bIconSpaceReserved);
-            a.recycle();
-        }
-    }
+        // Remove all listeners to 1. avoid triggering listener when update UI 2. prevent potential
+        // listener leaking when the view holder is reused by RecyclerView
+        mainSwitchBar.removeAllOnSwitchChangeListeners();
+        mainSwitchBar.setChecked(isChecked());
+        mainSwitchBar.addOnSwitchChangeListener(this);
 
-    @Override
-    public void setChecked(boolean checked) {
-        super.setChecked(checked);
-        if (mMainSwitchBar != null && mMainSwitchBar.isChecked() != checked) {
-            mMainSwitchBar.setChecked(checked);
-        }
-    }
-
-    @Override
-    public void setTitle(CharSequence title) {
-        super.setTitle(title);
-        if (mMainSwitchBar != null) {
-            mMainSwitchBar.setTitle(title);
-        }
-    }
-
-    @Override
-    public void setSummary(CharSequence summary) {
-        super.setSummary(summary);
-        if (mMainSwitchBar != null
-                && Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) {
-            mMainSwitchBar.setSummary(summary);
-        }
-    }
-
-    @Override
-    public void setIconSpaceReserved(boolean iconSpaceReserved) {
-        super.setIconSpaceReserved(iconSpaceReserved);
-        if (mMainSwitchBar != null) {
-            mMainSwitchBar.setIconSpaceReserved(iconSpaceReserved);
-        }
+        mainSwitchBar.show();
     }
 
     @Override
     public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
         super.setChecked(isChecked);
-    }
-
-    /**
-     * Update the switch status of preference
-     */
-    public void updateStatus(boolean checked) {
-        setChecked(checked);
-        if (mMainSwitchBar != null) {
-            mMainSwitchBar.setTitle(getTitle());
-            mMainSwitchBar.show();
+        for (OnCheckedChangeListener listener : mSwitchChangeListeners) {
+            listener.onCheckedChanged(buttonView, isChecked);
         }
     }
 
@@ -162,10 +106,6 @@ public class MainSwitchPreference extends TwoStatePreference
         if (!mSwitchChangeListeners.contains(listener)) {
             mSwitchChangeListeners.add(listener);
         }
-
-        if (mMainSwitchBar != null) {
-            mMainSwitchBar.addOnSwitchChangeListener(listener);
-        }
     }
 
     /**
@@ -173,14 +113,5 @@ public class MainSwitchPreference extends TwoStatePreference
      */
     public void removeOnSwitchChangeListener(OnCheckedChangeListener listener) {
         mSwitchChangeListeners.remove(listener);
-        if (mMainSwitchBar != null) {
-            mMainSwitchBar.removeOnSwitchChangeListener(listener);
-        }
-    }
-
-    private void registerListenerToSwitchBar() {
-        for (OnCheckedChangeListener listener : mSwitchChangeListeners) {
-            mMainSwitchBar.addOnSwitchChangeListener(listener);
-        }
     }
 }
diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/satellite/SatelliteDialogUtilsTest.kt b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/satellite/SatelliteDialogUtilsTest.kt
index 2078b363f434..1deb62510daf 100644
--- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/satellite/SatelliteDialogUtilsTest.kt
+++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/satellite/SatelliteDialogUtilsTest.kt
@@ -67,7 +67,7 @@ class SatelliteDialogUtilsTest {
 
     @Test
     @RequiresFlagsEnabled(Flags.FLAG_OEM_ENABLED_SATELLITE_FLAG)
-    fun mayStartSatelliteWarningDialog_satelliteIsOn_showWarningDialog() = runBlocking {
+    fun mayStartSatelliteWarningDialog_satelliteIsOn_showWarningDialog(): Unit = runBlocking {
         `when`(satelliteManager.registerForModemStateChanged(any(), any()))
                 .thenAnswer { invocation ->
                     val callback = invocation
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 c2e81bd7d54c..a47b4d5c354f 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
@@ -23,6 +23,7 @@ import android.view.View;
 import android.widget.TextView;
 
 import androidx.preference.PreferenceViewHolder;
+import androidx.test.core.app.ApplicationProvider;
 
 import com.android.settingslib.widget.mainswitch.R;
 
@@ -30,19 +31,17 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
-import org.robolectric.RuntimeEnvironment;
 
 @RunWith(RobolectricTestRunner.class)
 public class MainSwitchPreferenceTest {
 
-    private Context mContext;
+    private final Context mContext = ApplicationProvider.getApplicationContext();
     private View mRootView;
     private PreferenceViewHolder mHolder;
     private MainSwitchPreference mPreference;
 
     @Before
     public void setUp() {
-        mContext = RuntimeEnvironment.application;
         mRootView = View.inflate(mContext, R.layout.settingslib_main_switch_layout,
                 null /* parent */);
         mHolder = PreferenceViewHolder.createInstanceForTests(mRootView);
@@ -50,23 +49,22 @@ public class MainSwitchPreferenceTest {
     }
 
     @Test
-    public void setTitle_shouldUpdateTitle() {
+    public void onBindViewHolder_title() {
         final String defaultOnText = "Test title";
 
-        mPreference.onBindViewHolder(mHolder);
         mPreference.setTitle(defaultOnText);
-        mPreference.updateStatus(true /* checked */);
+        mPreference.onBindViewHolder(mHolder);
 
-        assertThat(((TextView) mRootView.findViewById(R.id.switch_text)).getText())
-                .isEqualTo(defaultOnText);
+        assertThat(mRootView.<TextView>requireViewById(
+                R.id.switch_text).getText().toString()).isEqualTo(defaultOnText);
     }
 
     @Test
-    public void updateStatus_shouldMatchTheStatus() {
+    public void onBindViewHolder_checked() {
+        mPreference.setChecked(true);
         mPreference.onBindViewHolder(mHolder);
-        mPreference.updateStatus(true);
 
-        assertThat(mPreference.isChecked()).isTrue();
+        assertThat(mRootView.<MainSwitchBar>requireViewById(
+                R.id.settingslib_main_switch_bar).isChecked()).isTrue();
     }
-
 }