Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec61f8be authored by Chun-Ku Lin's avatar Chun-Ku Lin
Browse files

Move PreferenceControllers to member variables to prevent memory leak

- Move PreferenceControllers to xml
- Clean up the PreferenceController so that it's less tied to a fragment
- Update and clean up the related robolectric test, so there are less
  mocks needed

Bug: 352606511
Test: manually check the Color Correction screen is shown correctly, and
choosing color correction options are reflected correctly
Test: atest DaltonizerRadioButtonPreferenceControllerTest
Test: atest ToggleDaltonizerPreferenceFragmentTest
Flag: EXEMPT (moving controller to xml can't be flagged)

Change-Id: I89b9366cfd7a398bb0572d34226d31d49373fd94
parent dc5469cf
Loading
Loading
Loading
Loading
+8 −4
Original line number Diff line number Diff line
@@ -32,24 +32,28 @@
        android:key="daltonizer_mode_deuteranomaly"
        android:persistent="false"
        android:summary="@string/daltonizer_mode_deuteranomaly_summary"
        android:title="@string/daltonizer_mode_deuteranomaly_title" />
        android:title="@string/daltonizer_mode_deuteranomaly_title"
        settings:controller="com.android.settings.accessibility.DaltonizerRadioButtonPreferenceController" />

    <com.android.settingslib.widget.SelectorWithWidgetPreference
        android:key="daltonizer_mode_protanomaly"
        android:persistent="false"
        android:summary="@string/daltonizer_mode_protanomaly_summary"
        android:title="@string/daltonizer_mode_protanomaly_title" />
        android:title="@string/daltonizer_mode_protanomaly_title"
        settings:controller="com.android.settings.accessibility.DaltonizerRadioButtonPreferenceController" />

    <com.android.settingslib.widget.SelectorWithWidgetPreference
        android:key="daltonizer_mode_tritanomaly"
        android:persistent="false"
        android:summary="@string/daltonizer_mode_tritanomaly_summary"
        android:title="@string/daltonizer_mode_tritanomaly_title" />
        android:title="@string/daltonizer_mode_tritanomaly_title"
        settings:controller="com.android.settings.accessibility.DaltonizerRadioButtonPreferenceController" />

    <com.android.settingslib.widget.SelectorWithWidgetPreference
        android:key="daltonizer_mode_grayscale"
        android:persistent="false"
        android:title="@string/daltonizer_mode_grayscale_title" />
        android:title="@string/daltonizer_mode_grayscale_title"
        settings:controller="com.android.settings.accessibility.DaltonizerRadioButtonPreferenceController" />

    <com.android.settings.widget.SeekBarPreference
        android:key="daltonizer_saturation"
+38 −54
Original line number Diff line number Diff line
@@ -19,17 +19,21 @@ package com.android.settings.accessibility;
import android.content.ContentResolver;
import android.content.Context;
import android.content.res.Resources;
import android.database.ContentObserver;
import android.os.Handler;
import android.os.Looper;
import android.provider.Settings;
import android.view.View;
import android.view.accessibility.AccessibilityManager;

import androidx.lifecycle.LifecycleObserver;
import androidx.annotation.NonNull;
import androidx.lifecycle.DefaultLifecycleObserver;
import androidx.lifecycle.LifecycleOwner;
import androidx.preference.Preference;
import androidx.preference.PreferenceScreen;

import com.android.settings.R;
import com.android.settings.core.BasePreferenceController;
import com.android.settingslib.core.lifecycle.Lifecycle;
import com.android.settingslib.widget.SelectorWithWidgetPreference;

import com.google.common.primitives.Ints;
@@ -39,41 +43,36 @@ import java.util.Map;

/** Controller class that control radio button of accessibility daltonizer settings. */
public class DaltonizerRadioButtonPreferenceController extends BasePreferenceController implements
        LifecycleObserver, SelectorWithWidgetPreference.OnClickListener {
    private static final String TYPE = Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER;
        DefaultLifecycleObserver, SelectorWithWidgetPreference.OnClickListener {
    private static final String DALTONIZER_TYPE_SETTINGS_KEY =
            Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER;

    // pair the preference key and daltonizer value.
    private final Map<String, Integer> mAccessibilityDaltonizerKeyToValueMap = new HashMap<>();

    // RadioButtonPreference key, each preference represent a daltonizer value.
    private final ContentResolver mContentResolver;
    private final ContentObserver mSettingsContentObserver;
    private final Resources mResources;
    private DaltonizerRadioButtonPreferenceController.OnChangeListener mOnChangeListener;
    private SelectorWithWidgetPreference mPreference;
    private int mAccessibilityDaltonizerValue;

    public DaltonizerRadioButtonPreferenceController(Context context, String preferenceKey) {
        super(context, preferenceKey);

        mContentResolver = context.getContentResolver();
        mResources = context.getResources();
        mSettingsContentObserver = new ContentObserver(new Handler(Looper.getMainLooper())) {
            @Override
            public void onChange(boolean selfChange) {
                if (mPreference != null) {
                    updateState(mPreference);
                }

    public DaltonizerRadioButtonPreferenceController(Context context, Lifecycle lifecycle,
            String preferenceKey) {
        super(context, preferenceKey);

        mContentResolver = context.getContentResolver();
        mResources = context.getResources();

        if (lifecycle != null) {
            lifecycle.addObserver(this);
            }
        };
    }

    protected static int getSecureAccessibilityDaltonizerValue(ContentResolver resolver,
            String name) {
        final String daltonizerStringValue = Settings.Secure.getString(resolver, name);
    protected static int getSecureAccessibilityDaltonizerValue(ContentResolver resolver) {
        final String daltonizerStringValue = Settings.Secure.getString(
                resolver, DALTONIZER_TYPE_SETTINGS_KEY);
        if (daltonizerStringValue == null) {
            return AccessibilityManager.DALTONIZER_CORRECT_DEUTERANOMALY;
        }
@@ -82,13 +81,8 @@ public class DaltonizerRadioButtonPreferenceController extends BasePreferenceCon
                : daltonizerIntValue;
    }

    public void setOnChangeListener(
            DaltonizerRadioButtonPreferenceController.OnChangeListener listener) {
        mOnChangeListener = listener;
    }

    private Map<String, Integer> getDaltonizerValueToKeyMap() {
        if (mAccessibilityDaltonizerKeyToValueMap.size() == 0) {
        if (mAccessibilityDaltonizerKeyToValueMap.isEmpty()) {

            final String[] daltonizerKeys = mResources.getStringArray(
                    R.array.daltonizer_mode_keys);
@@ -104,12 +98,8 @@ public class DaltonizerRadioButtonPreferenceController extends BasePreferenceCon
        return mAccessibilityDaltonizerKeyToValueMap;
    }

    private void putSecureString(String name, String value) {
        Settings.Secure.putString(mContentResolver, name, value);
    }

    private void handlePreferenceChange(String value) {
        putSecureString(TYPE, value);
        Settings.Secure.putString(mContentResolver, DALTONIZER_TYPE_SETTINGS_KEY, value);
    }

    @Override
@@ -124,44 +114,38 @@ public class DaltonizerRadioButtonPreferenceController extends BasePreferenceCon
                screen.findPreference(getPreferenceKey());
        mPreference.setOnClickListener(this);
        mPreference.setAppendixVisibility(View.GONE);
        updateState(mPreference);
    }

    @Override
    public void onRadioButtonClicked(SelectorWithWidgetPreference preference) {
        final int value = getDaltonizerValueToKeyMap().get(mPreferenceKey);
        handlePreferenceChange(String.valueOf(value));
        if (mOnChangeListener != null) {
            mOnChangeListener.onCheckedChanged(mPreference);
        }
    }

    private int getAccessibilityDaltonizerValue() {
        final int daltonizerValue = getSecureAccessibilityDaltonizerValue(mContentResolver,
                TYPE);
        final int daltonizerValue = getSecureAccessibilityDaltonizerValue(mContentResolver);
        return daltonizerValue;
    }

    protected void updatePreferenceCheckedState(int value) {
        if (mAccessibilityDaltonizerValue == value) {
            mPreference.setChecked(true);
        }
    }

    @Override
    public void updateState(Preference preference) {
        super.updateState(preference);
        mAccessibilityDaltonizerValue = getAccessibilityDaltonizerValue();

        // reset RadioButton
        mPreference.setChecked(false);
        final int daltonizerValueInSetting = getAccessibilityDaltonizerValue();
        final int preferenceValue = getDaltonizerValueToKeyMap().get(mPreference.getKey());
        updatePreferenceCheckedState(preferenceValue);
        mPreference.setChecked(preferenceValue == daltonizerValueInSetting);
    }

    /** Listener interface handles checked event. */
    public interface OnChangeListener {
        /** A hook that is called when preference checked. */
        void onCheckedChanged(Preference preference);
    @Override
    public void onResume(@NonNull LifecycleOwner owner) {
        mContentResolver.registerContentObserver(
                Settings.Secure.getUriFor(DALTONIZER_TYPE_SETTINGS_KEY),
                /* notifyForDescendants= */ false,
                mSettingsContentObserver
        );
    }

    @Override
    public void onPause(@NonNull LifecycleOwner owner) {
        mContentResolver.unregisterContentObserver(mSettingsContentObserver);
    }
}
+5 −54
Original line number Diff line number Diff line
@@ -24,8 +24,6 @@ import static com.android.settings.accessibility.AccessibilityUtil.State.ON;

import android.app.settings.SettingsEnums;
import android.content.ComponentName;
import android.content.Context;
import android.content.res.Resources;
import android.os.Bundle;
import android.provider.Settings;
import android.view.LayoutInflater;
@@ -33,14 +31,11 @@ import android.view.View;
import android.view.ViewGroup;

import androidx.annotation.VisibleForTesting;
import androidx.preference.Preference;

import com.android.settings.R;
import com.android.settings.accessibility.AccessibilityUtil.QuickSettingsTooltipType;
import com.android.settings.search.BaseSearchIndexProvider;
import com.android.settings.widget.SettingsMainSwitchPreference;
import com.android.settingslib.core.AbstractPreferenceController;
import com.android.settingslib.core.lifecycle.Lifecycle;
import com.android.settingslib.search.SearchIndexable;

import java.util.ArrayList;
@@ -48,40 +43,18 @@ import java.util.List;

/** Settings for daltonizer. */
@SearchIndexable(forTarget = SearchIndexable.ALL & ~SearchIndexable.ARC)
public class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePreferenceFragment
        implements DaltonizerRadioButtonPreferenceController.OnChangeListener {
public class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePreferenceFragment {

    private static final String TAG = "ToggleDaltonizerPreferenceFragment";
    private static final String ENABLED = Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER_ENABLED;
    private static final String KEY_PREVIEW = "daltonizer_preview";
    @VisibleForTesting
    static final String KEY_DEUTERANOMALY = "daltonizer_mode_deuteranomaly";
    @VisibleForTesting
    static final String KEY_PROTANOMALY = "daltonizer_mode_protanomaly";
    @VisibleForTesting
    static final String KEY_TRITANOMEALY = "daltonizer_mode_tritanomaly";
    @VisibleForTesting
    static final String KEY_GRAYSCALE = "daltonizer_mode_grayscale";
    private static final String KEY_DEUTERANOMALY = "daltonizer_mode_deuteranomaly";
    private static final String KEY_PROTANOMALY = "daltonizer_mode_protanomaly";
    private static final String KEY_TRITANOMEALY = "daltonizer_mode_tritanomaly";
    private static final String KEY_GRAYSCALE = "daltonizer_mode_grayscale";
    @VisibleForTesting
    static final String KEY_SATURATION = "daltonizer_saturation";

    private static final List<AbstractPreferenceController> sControllers = new ArrayList<>();

    private static List<AbstractPreferenceController> buildPreferenceControllers(Context context,
            Lifecycle lifecycle) {
        if (sControllers.size() == 0) {
            final Resources resources = context.getResources();
            final String[] daltonizerKeys = resources.getStringArray(
                    R.array.daltonizer_mode_keys);

            for (String daltonizerKey : daltonizerKeys) {
                sControllers.add(new DaltonizerRadioButtonPreferenceController(
                        context, lifecycle, daltonizerKey));
            }
        }
        return sControllers;
    }

    @Override
    protected void registerKeysToObserverCallback(
            AccessibilitySettingsContentObserver contentObserver) {
@@ -117,13 +90,6 @@ public class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePreferenceF
        }
    }

    @Override
    public void onCheckedChanged(Preference preference) {
        for (AbstractPreferenceController controller : sControllers) {
            controller.updateState(preference);
        }
    }

    private void updateFooterPreference() {
        final String title = getPrefContext()
                .getString(R.string.accessibility_daltonizer_about_title);
@@ -155,21 +121,6 @@ public class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePreferenceF
    public void onResume() {
        super.onResume();
        updateSwitchBarToggleSwitch();
        for (AbstractPreferenceController controller :
                buildPreferenceControllers(getPrefContext(), getSettingsLifecycle())) {
            ((DaltonizerRadioButtonPreferenceController) controller).setOnChangeListener(this);
            ((DaltonizerRadioButtonPreferenceController) controller).displayPreference(
                    getPreferenceScreen());
        }
    }

    @Override
    public void onPause() {
        for (AbstractPreferenceController controller :
                buildPreferenceControllers(getPrefContext(), getSettingsLifecycle())) {
            ((DaltonizerRadioButtonPreferenceController) controller).setOnChangeListener(null);
        }
        super.onPause();
    }

    @Override
+77 −47
Original line number Diff line number Diff line
@@ -18,61 +18,65 @@ package com.android.settings.accessibility;

import static com.google.common.truth.Truth.assertThat;

import static org.mockito.Mockito.atLeastOnce;
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.provider.Settings;

import androidx.preference.Preference;
import androidx.lifecycle.LifecycleOwner;
import androidx.preference.PreferenceManager;
import androidx.preference.PreferenceScreen;
import androidx.test.core.app.ApplicationProvider;

import com.android.settings.R;
import com.android.settingslib.core.lifecycle.Lifecycle;
import com.android.settingslib.widget.SelectorWithWidgetPreference;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.shadows.ShadowLooper;

/**
 * Tests for {@link DaltonizerRadioButtonPreferenceController}
 */
@RunWith(RobolectricTestRunner.class)
public class DaltonizerRadioButtonPreferenceControllerTest implements
        DaltonizerRadioButtonPreferenceController.OnChangeListener {
    private static final String PREF_KEY = "daltonizer_mode_protanomaly";
    private static final String PREF_VALUE = "11";
    private static final String PREF_FAKE_VALUE = "-1";

public class DaltonizerRadioButtonPreferenceControllerTest {
    private static final int DALTONIZER_MODE_INDEX = 0;
    private static final String PREF_INVALID_VALUE = "-1";

    private final Context mContext = ApplicationProvider.getApplicationContext();
    private final String mPrefKey =
            mContext.getResources()
                    .getStringArray(R.array.daltonizer_mode_keys)[DALTONIZER_MODE_INDEX];
    private final String mPrefValue =
            String.valueOf(mContext.getResources()
                    .getIntArray(R.array.daltonizer_type_values)[DALTONIZER_MODE_INDEX]);
    private DaltonizerRadioButtonPreferenceController mController;

    @Mock
    private SelectorWithWidgetPreference mMockPref;
    private Context mContext;

    @Mock
    private SelectorWithWidgetPreference mPreference;
    private PreferenceScreen mScreen;

    private LifecycleOwner mLifecycleOwner;
    private Lifecycle mLifecycle;

    @Before
    public void setUp() {
        MockitoAnnotations.initMocks(this);
        mContext = RuntimeEnvironment.application;
        mController = new DaltonizerRadioButtonPreferenceController(mContext, mock(Lifecycle.class),
                PREF_KEY);
        mController.setOnChangeListener(this);

        when(mScreen.findPreference(mController.getPreferenceKey())).thenReturn(mMockPref);
        when(mMockPref.getKey()).thenReturn(PREF_KEY);
        // initialize the value as unchecked
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, PREF_INVALID_VALUE);
        mController = new DaltonizerRadioButtonPreferenceController(mContext, mPrefKey);
        mPreference = new SelectorWithWidgetPreference(mContext);
        mPreference.setKey(mPrefKey);
        mScreen = new PreferenceManager(mContext).createPreferenceScreen(mContext);
        mScreen.addPreference(mPreference);
        mController.displayPreference(mScreen);
        mLifecycleOwner = () -> mLifecycle;
        mLifecycle = new Lifecycle(mLifecycleOwner);
    }

    @Override
    public void onCheckedChanged(Preference preference) {
        mController.updateState(preference);
    @After
    public void cleanUp() {
        mLifecycle.removeObserver(mController);
    }

    @Test
@@ -81,36 +85,62 @@ public class DaltonizerRadioButtonPreferenceControllerTest implements
    }

    @Test
    public void updateState_notChecked() {
    public void updateState_valueNotMatched_notChecked() {
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, PREF_FAKE_VALUE);
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, PREF_INVALID_VALUE);

        mController.updateState(mMockPref);
        mController.updateState(mPreference);

        // the first checked state is set to false by control
        verify(mMockPref, atLeastOnce()).setChecked(false);
        verify(mMockPref, never()).setChecked(true);
        assertThat(mPreference.isChecked()).isFalse();
    }

    @Test
    public void updateState_checked() {
    public void updateState_valueMatched_checked() {
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, PREF_VALUE);
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, mPrefValue);

        mController.updateState(mMockPref);
        mController.updateState(mPreference);

        // the first checked state is set to false by control
        verify(mMockPref, atLeastOnce()).setChecked(false);
        verify(mMockPref, atLeastOnce()).setChecked(true);
        assertThat(mPreference.isChecked()).isTrue();
    }

    @Test
    public void onRadioButtonClick_shouldReturnDaltonizerValue() {
        mController.onRadioButtonClicked(mMockPref);
        mController.onRadioButtonClicked(mPreference);

        final String accessibilityDaltonizerValue = Settings.Secure.getString(
                mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER);
        assertThat(accessibilityDaltonizerValue).isEqualTo(mPrefValue);
    }

    @Test
    public void onResume_settingsValueChangedToUnmatch_preferenceBecomesUnchecked() {
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, mPrefValue);
        mController.updateState(mPreference);
        assertThat(mPreference.isChecked()).isTrue();
        mLifecycle.addObserver(mController);

        mLifecycle.handleLifecycleEvent(androidx.lifecycle.Lifecycle.Event.ON_RESUME);
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, PREF_INVALID_VALUE);
        ShadowLooper.idleMainLooper();

        assertThat(mPreference.isChecked()).isFalse();
    }

    @Test
    public void onPause_settingsValueChangedAndMatch_preferenceStateNotUpdated() {
        assertThat(mPreference.isChecked()).isFalse();
        mLifecycle.addObserver(mController);
        mLifecycle.handleLifecycleEvent(androidx.lifecycle.Lifecycle.Event.ON_RESUME);

        mLifecycle.handleLifecycleEvent(androidx.lifecycle.Lifecycle.Event.ON_PAUSE);
        Settings.Secure.putString(mContext.getContentResolver(),
                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, mPrefValue);
        ShadowLooper.idleMainLooper();

        assertThat(accessibilityDaltonizerValue).isEqualTo(PREF_VALUE);
        assertThat(mPreference.isChecked()).isFalse();
    }
}
+51 −139

File changed.

Preview size limit exceeded, changes collapsed.