From 9d8603568d9b5c329992aeaecd9efee1f7f88de3 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Fri, 25 Mar 2022 22:37:32 +0800 Subject: [PATCH] Fix "Automatically sync app data" button state not changed This issue is caused by mPreference is null. (Not recovered when the fragment is recreated after configuration change.) Mimic the PreferenceDialogFragmentCompat.getPreference() in AndroidX to solve this issue. https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:preference/preference/src/main/java/androidx/preference/PreferenceDialogFragmentCompat.java;l=176;drc=ca9feb3b73769089afbfd36b4d4a3d91239f9cd5 Ideally in the long term, we could use PreferenceDialogFragmentCompat instead. Fix: 218754949 Test: robotest & manual Change-Id: I7fc8dd3b771aa45c91f915e25c8cc6c6afdd8d63 --- .../AutoSyncDataPreferenceController.java | 72 ++++++++++--------- ...oSyncPersonalDataPreferenceController.java | 6 +- .../AutoSyncWorkDataPreferenceController.java | 5 +- .../shadow/ShadowContentResolver.java | 9 ++- .../AutoSyncDataPreferenceControllerTest.java | 60 +++++++++++----- ...cPersonalDataPreferenceControllerTest.java | 4 +- ...oSyncWorkDataPreferenceControllerTest.java | 6 +- 7 files changed, 95 insertions(+), 67 deletions(-) diff --git a/src/com/android/settings/users/AutoSyncDataPreferenceController.java b/src/com/android/settings/users/AutoSyncDataPreferenceController.java index e7c5cb699a1..e3240c4bdcb 100644 --- a/src/com/android/settings/users/AutoSyncDataPreferenceController.java +++ b/src/com/android/settings/users/AutoSyncDataPreferenceController.java @@ -30,6 +30,7 @@ import android.util.Log; import androidx.appcompat.app.AlertDialog; import androidx.fragment.app.Fragment; import androidx.preference.Preference; +import androidx.preference.PreferenceFragmentCompat; import androidx.preference.SwitchPreference; import com.android.settings.R; @@ -45,11 +46,11 @@ public class AutoSyncDataPreferenceController extends AbstractPreferenceControll private static final String KEY_AUTO_SYNC_ACCOUNT = "auto_sync_account_data"; protected final UserManager mUserManager; - private final Fragment mParentFragment; + private final PreferenceFragmentCompat mParentFragment; protected UserHandle mUserHandle; - public AutoSyncDataPreferenceController(Context context, Fragment parent) { + public AutoSyncDataPreferenceController(Context context, PreferenceFragmentCompat parent) { super(context); mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE); mParentFragment = parent; @@ -72,8 +73,9 @@ public class AutoSyncDataPreferenceController extends AbstractPreferenceControll if (ActivityManager.isUserAMonkey()) { Log.d(TAG, "ignoring monkey's attempt to flip sync state"); } else { - ConfirmAutoSyncChangeFragment.show(mParentFragment, checked, mUserHandle, - switchPreference); + ConfirmAutoSyncChangeFragment + .newInstance(checked, mUserHandle.getIdentifier(), getPreferenceKey()) + .show(mParentFragment); } return true; } @@ -97,34 +99,33 @@ public class AutoSyncDataPreferenceController extends AbstractPreferenceControll */ public static class ConfirmAutoSyncChangeFragment extends InstrumentedDialogFragment implements DialogInterface.OnClickListener { - private static final String SAVE_ENABLING = "enabling"; - private static final String SAVE_USER_HANDLE = "userHandle"; - boolean mEnabling; - UserHandle mUserHandle; - SwitchPreference mPreference; + private static final String ARG_ENABLING = "enabling"; + private static final String ARG_USER_ID = "userId"; + private static final String ARG_KEY = "key"; - public static void show(Fragment parent, boolean enabling, UserHandle userHandle, - SwitchPreference preference) { - if (!parent.isAdded()) return; + static ConfirmAutoSyncChangeFragment newInstance(boolean enabling, int userId, String key) { + ConfirmAutoSyncChangeFragment dialogFragment = new ConfirmAutoSyncChangeFragment(); + Bundle arguments = new Bundle(); + arguments.putBoolean(ARG_ENABLING, enabling); + arguments.putInt(ARG_USER_ID, userId); + arguments.putString(ARG_KEY, key); + dialogFragment.setArguments(arguments); + return dialogFragment; + } - final ConfirmAutoSyncChangeFragment dialog = new ConfirmAutoSyncChangeFragment(); - dialog.mEnabling = enabling; - dialog.mUserHandle = userHandle; - dialog.setTargetFragment(parent, 0); - dialog.mPreference = preference; - dialog.show(parent.getFragmentManager(), TAG_CONFIRM_AUTO_SYNC_CHANGE); + void show(PreferenceFragmentCompat parent) { + if (!parent.isAdded()) { + return; + } + setTargetFragment(parent, 0); + show(parent.getParentFragmentManager(), TAG_CONFIRM_AUTO_SYNC_CHANGE); } @Override public Dialog onCreateDialog(Bundle savedInstanceState) { final Context context = getActivity(); - if (savedInstanceState != null) { - mEnabling = savedInstanceState.getBoolean(SAVE_ENABLING); - mUserHandle = (UserHandle) savedInstanceState.getParcelable(SAVE_USER_HANDLE); - } - final AlertDialog.Builder builder = new AlertDialog.Builder(context); - if (!mEnabling) { + if (!requireArguments().getBoolean(ARG_ENABLING)) { builder.setTitle(R.string.data_usage_auto_sync_off_dialog_title); builder.setMessage(R.string.data_usage_auto_sync_off_dialog); } else { @@ -138,13 +139,6 @@ public class AutoSyncDataPreferenceController extends AbstractPreferenceControll return builder.create(); } - @Override - public void onSaveInstanceState(Bundle outState) { - super.onSaveInstanceState(outState); - outState.putBoolean(SAVE_ENABLING, mEnabling); - outState.putParcelable(SAVE_USER_HANDLE, mUserHandle); - } - @Override public int getMetricsCategory() { return SettingsEnums.DIALOG_CONFIRM_AUTO_SYNC_CHANGE; @@ -153,10 +147,18 @@ public class AutoSyncDataPreferenceController extends AbstractPreferenceControll @Override public void onClick(DialogInterface dialog, int which) { if (which == DialogInterface.BUTTON_POSITIVE) { - ContentResolver.setMasterSyncAutomaticallyAsUser(mEnabling, - mUserHandle.getIdentifier()); - if (mPreference != null) { - mPreference.setChecked(mEnabling); + Bundle arguments = requireArguments(); + boolean enabling = arguments.getBoolean(ARG_ENABLING); + ContentResolver.setMasterSyncAutomaticallyAsUser(enabling, + arguments.getInt(ARG_USER_ID)); + Fragment targetFragment = getTargetFragment(); + if (targetFragment instanceof PreferenceFragmentCompat) { + Preference preference = + ((PreferenceFragmentCompat) targetFragment).findPreference( + arguments.getString(ARG_KEY)); + if (preference instanceof SwitchPreference) { + ((SwitchPreference) preference).setChecked(enabling); + } } } } diff --git a/src/com/android/settings/users/AutoSyncPersonalDataPreferenceController.java b/src/com/android/settings/users/AutoSyncPersonalDataPreferenceController.java index 2530977a3d9..c10e38ec759 100644 --- a/src/com/android/settings/users/AutoSyncPersonalDataPreferenceController.java +++ b/src/com/android/settings/users/AutoSyncPersonalDataPreferenceController.java @@ -18,14 +18,14 @@ package com.android.settings.users; import android.content.Context; import android.os.UserHandle; -import androidx.fragment.app.Fragment; +import androidx.preference.PreferenceFragmentCompat; public class AutoSyncPersonalDataPreferenceController extends AutoSyncDataPreferenceController { - private static final String TAG = "AutoSyncPersonalData"; private static final String KEY_AUTO_SYNC_PERSONAL_ACCOUNT = "auto_sync_personal_account_data"; - public AutoSyncPersonalDataPreferenceController(Context context, Fragment parent) { + public AutoSyncPersonalDataPreferenceController(Context context, + PreferenceFragmentCompat parent) { super(context, parent); } diff --git a/src/com/android/settings/users/AutoSyncWorkDataPreferenceController.java b/src/com/android/settings/users/AutoSyncWorkDataPreferenceController.java index 1e6284509a2..fb571734331 100644 --- a/src/com/android/settings/users/AutoSyncWorkDataPreferenceController.java +++ b/src/com/android/settings/users/AutoSyncWorkDataPreferenceController.java @@ -18,16 +18,15 @@ package com.android.settings.users; import android.content.Context; import android.os.UserHandle; -import androidx.fragment.app.Fragment; +import androidx.preference.PreferenceFragmentCompat; import com.android.settings.Utils; public class AutoSyncWorkDataPreferenceController extends AutoSyncPersonalDataPreferenceController { - private static final String TAG = "AutoSyncWorkData"; private static final String KEY_AUTO_SYNC_WORK_ACCOUNT = "auto_sync_work_account_data"; - public AutoSyncWorkDataPreferenceController(Context context, Fragment parent) { + public AutoSyncWorkDataPreferenceController(Context context, PreferenceFragmentCompat parent) { super(context, parent); mUserHandle = Utils.getManagedProfileWithDisabled(mUserManager); } diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowContentResolver.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowContentResolver.java index 4d4cfc7701d..5d9202fb70e 100644 --- a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowContentResolver.java +++ b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowContentResolver.java @@ -19,6 +19,7 @@ package com.android.settings.testutils.shadow; import static android.provider.SearchIndexablesContract.INDEXABLES_RAW_COLUMNS; import android.accounts.Account; +import android.annotation.UserIdInt; import android.content.ContentResolver; import android.content.SyncAdapterType; import android.database.Cursor; @@ -76,8 +77,12 @@ public class ShadowContentResolver { @Implementation protected static boolean getMasterSyncAutomaticallyAsUser(int userId) { - return sMasterSyncAutomatically.containsKey(userId) - ? sMasterSyncAutomatically.get(userId) : true; + return sMasterSyncAutomatically.getOrDefault(userId, true); + } + + @Implementation + protected static void setMasterSyncAutomaticallyAsUser(boolean sync, @UserIdInt int userId) { + sMasterSyncAutomatically.put(userId, sync); } public static void setSyncAdapterTypes(SyncAdapterType[] syncAdapterTypes) { diff --git a/tests/robotests/src/com/android/settings/users/AutoSyncDataPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/users/AutoSyncDataPreferenceControllerTest.java index 95fc3f80637..b96e70c8fd0 100644 --- a/tests/robotests/src/com/android/settings/users/AutoSyncDataPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/users/AutoSyncDataPreferenceControllerTest.java @@ -21,16 +21,20 @@ import static org.mockito.Answers.RETURNS_DEEP_STUBS; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.when; +import android.content.ContentResolver; import android.content.Context; import android.content.DialogInterface; import android.content.pm.UserInfo; import android.os.UserManager; -import androidx.fragment.app.Fragment; -import androidx.preference.Preference; +import androidx.preference.PreferenceFragmentCompat; import androidx.preference.PreferenceScreen; import androidx.preference.SwitchPreference; +import com.android.settings.testutils.shadow.ShadowContentResolver; +import com.android.settings.users.AutoSyncDataPreferenceController.ConfirmAutoSyncChangeFragment; + +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -38,12 +42,14 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowApplication; import java.util.ArrayList; import java.util.List; @RunWith(RobolectricTestRunner.class) +@Config(shadows = {ShadowContentResolver.class}) public class AutoSyncDataPreferenceControllerTest { @Mock(answer = RETURNS_DEEP_STUBS) @@ -51,12 +57,11 @@ public class AutoSyncDataPreferenceControllerTest { @Mock(answer = RETURNS_DEEP_STUBS) private UserManager mUserManager; @Mock - private Fragment mFragment; + private PreferenceFragmentCompat mFragment; - private Preference mPreference; + private SwitchPreference mPreference; private Context mContext; private AutoSyncDataPreferenceController mController; - private AutoSyncDataPreferenceController.ConfirmAutoSyncChangeFragment mConfirmSyncFragment; @Before public void setUp() { @@ -65,11 +70,17 @@ public class AutoSyncDataPreferenceControllerTest { shadowContext.setSystemService(Context.USER_SERVICE, mUserManager); mContext = RuntimeEnvironment.application; mController = new AutoSyncDataPreferenceController(mContext, mFragment); - mConfirmSyncFragment = new AutoSyncDataPreferenceController.ConfirmAutoSyncChangeFragment(); - mConfirmSyncFragment.setTargetFragment(mFragment, 0); - mPreference = new Preference(mContext); - mPreference.setKey(mController.getPreferenceKey()); - when(mScreen.findPreference(mPreference.getKey())).thenReturn(mPreference); + String preferenceKey = mController.getPreferenceKey(); + mPreference = new SwitchPreference(mContext); + mPreference.setKey(preferenceKey); + mPreference.setChecked(true); + when(mScreen.findPreference(preferenceKey)).thenReturn(mPreference); + when(mFragment.findPreference(preferenceKey)).thenReturn(mPreference); + } + + @After + public void tearDown() { + ShadowContentResolver.reset(); } @Test @@ -119,15 +130,26 @@ public class AutoSyncDataPreferenceControllerTest { } @Test - public void autoSyncData_shouldNotBeSetOnCancel() { - final Context context = RuntimeEnvironment.application; - final SwitchPreference preference = new SwitchPreference(context); - preference.setChecked(false); - mController = new AutoSyncDataPreferenceController(context, mFragment); - mConfirmSyncFragment.mPreference = preference; - mConfirmSyncFragment.mEnabling = true; + public void confirmDialog_uncheckThenOk_shouldUncheck() { + ConfirmAutoSyncChangeFragment confirmSyncFragment = + ConfirmAutoSyncChangeFragment.newInstance(false, 0, mController.getPreferenceKey()); + confirmSyncFragment.setTargetFragment(mFragment, 0); - mConfirmSyncFragment.onClick(null, DialogInterface.BUTTON_NEGATIVE); - assertThat(preference.isChecked()).isFalse(); + confirmSyncFragment.onClick(null, DialogInterface.BUTTON_POSITIVE); + + assertThat(ContentResolver.getMasterSyncAutomaticallyAsUser(0)).isFalse(); + assertThat(mPreference.isChecked()).isFalse(); + } + + @Test + public void confirmDialog_uncheckThenCancel_shouldNotUncheck() { + ConfirmAutoSyncChangeFragment confirmSyncFragment = + ConfirmAutoSyncChangeFragment.newInstance(false, 0, mController.getPreferenceKey()); + confirmSyncFragment.setTargetFragment(mFragment, 0); + + confirmSyncFragment.onClick(null, DialogInterface.BUTTON_NEGATIVE); + + assertThat(ContentResolver.getMasterSyncAutomaticallyAsUser(0)).isTrue(); + assertThat(mPreference.isChecked()).isTrue(); } } diff --git a/tests/robotests/src/com/android/settings/users/AutoSyncPersonalDataPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/users/AutoSyncPersonalDataPreferenceControllerTest.java index 70f516552a5..00210c8b63c 100644 --- a/tests/robotests/src/com/android/settings/users/AutoSyncPersonalDataPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/users/AutoSyncPersonalDataPreferenceControllerTest.java @@ -25,8 +25,8 @@ import android.content.Context; import android.content.pm.UserInfo; import android.os.UserManager; -import androidx.fragment.app.Fragment; import androidx.preference.Preference; +import androidx.preference.PreferenceFragmentCompat; import androidx.preference.PreferenceScreen; import org.junit.Before; @@ -49,7 +49,7 @@ public class AutoSyncPersonalDataPreferenceControllerTest { @Mock(answer = RETURNS_DEEP_STUBS) private UserManager mUserManager; @Mock(answer = RETURNS_DEEP_STUBS) - private Fragment mFragment; + private PreferenceFragmentCompat mFragment; private Context mContext; private Preference mPreference; diff --git a/tests/robotests/src/com/android/settings/users/AutoSyncWorkDataPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/users/AutoSyncWorkDataPreferenceControllerTest.java index 21b47590dfb..b6b347bed19 100644 --- a/tests/robotests/src/com/android/settings/users/AutoSyncWorkDataPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/users/AutoSyncWorkDataPreferenceControllerTest.java @@ -27,17 +27,17 @@ import android.content.pm.UserInfo; import android.os.UserHandle; import android.os.UserManager; -import androidx.fragment.app.Fragment; +import androidx.preference.PreferenceFragmentCompat; 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 java.util.ArrayList; import java.util.List; -import org.robolectric.RobolectricTestRunner; @RunWith(RobolectricTestRunner.class) public class AutoSyncWorkDataPreferenceControllerTest { @@ -47,7 +47,7 @@ public class AutoSyncWorkDataPreferenceControllerTest { @Mock(answer = RETURNS_DEEP_STUBS) private UserManager mUserManager; @Mock(answer = RETURNS_DEEP_STUBS) - private Fragment mFragment; + private PreferenceFragmentCompat mFragment; @Mock private Context mContext;