From 09a2a49bd3e6d61c9732ec1dadfe7f7ab97eb463 Mon Sep 17 00:00:00 2001 From: Peter_Liang Date: Wed, 6 Apr 2022 17:45:59 +0800 Subject: [PATCH] =?UTF-8?q?Fix=20that=20device=20isn't=20responding=20for?= =?UTF-8?q?=20a=20while=20when=20resetting=20all=20settings=20on=20?= =?UTF-8?q?=E2=80=9CDisplay=20size=20and=20text=E2=80=9D=20page.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goal: Probably has the race condition issue between "Bold text" and the other features including "Display Size", “Font Size” if they would be enabled at the same time, so our workaround is that the “Bold text” would be reset first and then do the remaining to avoid flickering problem. Bug: 223747686 Bug: 220082104 Bug: 220070773 Test: make RunSettingsRoboTests ROBOTEST_FILTER=TextReadingPreferenceFragmentTest Change-Id: If1425fe2579bec8dded69680ac73fbfb03c37321 --- .../TextReadingPreferenceFragment.java | 54 ++++++++- .../TextReadingPreferenceFragmentTest.java | 107 ++++++++++++++++++ 2 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/accessibility/TextReadingPreferenceFragmentTest.java diff --git a/src/com/android/settings/accessibility/TextReadingPreferenceFragment.java b/src/com/android/settings/accessibility/TextReadingPreferenceFragment.java index da0287677f6..11485844127 100644 --- a/src/com/android/settings/accessibility/TextReadingPreferenceFragment.java +++ b/src/com/android/settings/accessibility/TextReadingPreferenceFragment.java @@ -22,6 +22,7 @@ import android.app.Dialog; import android.app.settings.SettingsEnums; import android.content.Context; import android.content.DialogInterface; +import android.os.Bundle; import androidx.appcompat.app.AlertDialog; @@ -32,6 +33,8 @@ import com.android.settings.search.BaseSearchIndexProvider; import com.android.settingslib.core.AbstractPreferenceController; import com.android.settingslib.search.SearchIndexable; +import com.google.common.annotations.VisibleForTesting; + import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -49,6 +52,26 @@ public class TextReadingPreferenceFragment extends DashboardFragment { private static final String RESET_KEY = "reset"; private static final String BOLD_TEXT_KEY = "toggle_force_bold_text"; private static final String HIGHT_TEXT_CONTRAST_KEY = "toggle_high_text_contrast_preference"; + private static final String NEED_RESET_SETTINGS = "need_reset_settings"; + private FontWeightAdjustmentPreferenceController mFontWeightAdjustmentController; + + @VisibleForTesting + List mResetStateListeners; + + @VisibleForTesting + boolean mNeedResetSettings; + + @Override + public void onCreate(Bundle icicle) { + super.onCreate(icicle); + + mNeedResetSettings = false; + mResetStateListeners = getResetStateListeners(); + + if (icicle != null && icicle.getBoolean(NEED_RESET_SETTINGS)) { + mResetStateListeners.forEach(ResetStateListener::resetState); + } + } @Override protected int getPreferenceScreenResId() { @@ -69,7 +92,7 @@ public class TextReadingPreferenceFragment extends DashboardFragment { protected List createPreferenceControllers(Context context) { final List controllers = new ArrayList<>(); final FontSizeData fontSizeData = new FontSizeData(context); - final DisplaySizeData displaySizeData = new DisplaySizeData(context); + final DisplaySizeData displaySizeData = createDisplaySizeData(context); final TextReadingPreviewController previewController = new TextReadingPreviewController( context, PREVIEW_KEY, fontSizeData, displaySizeData); @@ -85,9 +108,9 @@ public class TextReadingPreferenceFragment extends DashboardFragment { displaySizeController.setInteractionListener(previewController); controllers.add(displaySizeController); - final FontWeightAdjustmentPreferenceController fontWeightController = + mFontWeightAdjustmentController = new FontWeightAdjustmentPreferenceController(context, BOLD_TEXT_KEY); - controllers.add(fontWeightController); + controllers.add(mFontWeightAdjustmentController); final HighTextContrastPreferenceController highTextContrastController = new HighTextContrastPreferenceController(context, HIGHT_TEXT_CONTRAST_KEY); @@ -126,12 +149,35 @@ public class TextReadingPreferenceFragment extends DashboardFragment { return super.getDialogMetricsCategory(dialogId); } + @Override + public void onSaveInstanceState(Bundle outState) { + if (mNeedResetSettings) { + outState.putBoolean(NEED_RESET_SETTINGS, true); + } + } + + @VisibleForTesting + DisplaySizeData createDisplaySizeData(Context context) { + return new DisplaySizeData(context); + } + private void onPositiveButtonClicked(DialogInterface dialog, int which) { // To avoid showing the dialog again, probably the onDetach() of SettingsDialogFragment // was interrupted by unexpectedly recreating the activity. removeDialog(DialogEnums.DIALOG_RESET_SETTINGS); - getResetStateListeners().forEach(ResetStateListener::resetState); + if (mFontWeightAdjustmentController.isChecked()) { + // TODO(b/228956791): Consider replacing or removing it once the root cause is + // clarified and the better method is available. + // Probably has the race condition issue between "Bold text" and the other features + // including "Display Size", “Font Size” if they would be enabled at the same time, + // so our workaround is that the “Bold text” would be reset first and then do the + // remaining to avoid flickering problem. + mNeedResetSettings = true; + mFontWeightAdjustmentController.resetState(); + } else { + mResetStateListeners.forEach(ResetStateListener::resetState); + } } private List getResetStateListeners() { diff --git a/tests/robotests/src/com/android/settings/accessibility/TextReadingPreferenceFragmentTest.java b/tests/robotests/src/com/android/settings/accessibility/TextReadingPreferenceFragmentTest.java new file mode 100644 index 00000000000..1793cc28984 --- /dev/null +++ b/tests/robotests/src/com/android/settings/accessibility/TextReadingPreferenceFragmentTest.java @@ -0,0 +1,107 @@ +/* + * 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.settings.accessibility; + +import static com.android.settings.accessibility.FontWeightAdjustmentPreferenceController.BOLD_TEXT_ADJUSTMENT; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.content.DialogInterface; +import android.provider.Settings; + +import androidx.appcompat.app.AlertDialog; +import androidx.fragment.app.FragmentActivity; +import androidx.preference.PreferenceManager; +import androidx.test.core.app.ApplicationProvider; + +import com.android.settings.R; +import com.android.settings.accessibility.AccessibilityDialogUtils.DialogEnums; +import com.android.settings.accessibility.TextReadingResetController.ResetStateListener; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; + +import java.util.ArrayList; +import java.util.Arrays; + +/** + * Tests for {@link TextReadingPreferenceFragment}. + */ +@RunWith(RobolectricTestRunner.class) +public class TextReadingPreferenceFragmentTest { + private TextReadingPreferenceFragment mFragment; + private Context mContext = ApplicationProvider.getApplicationContext(); + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private PreferenceManager mPreferenceManager; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext.setTheme(R.style.Theme_AppCompat); + + mFragment = spy(new TextReadingPreferenceFragment()); + when(mFragment.getPreferenceManager()).thenReturn(mPreferenceManager); + when(mFragment.getPreferenceManager().getContext()).thenReturn(mContext); + when(mFragment.getContext()).thenReturn(mContext); + when(mFragment.getActivity()).thenReturn(Robolectric.setupActivity(FragmentActivity.class)); + + // Avoid a NPE is happened in ShadowWindowManagerGlobal + doReturn(mock(DisplaySizeData.class)).when(mFragment).createDisplaySizeData(mContext); + mFragment.createPreferenceControllers(mContext); + } + + @Test + public void onDialogPositiveButtonClicked_boldTextEnabled_needResetSettings() { + Settings.Secure.putInt(mContext.getContentResolver(), + Settings.Secure.FONT_WEIGHT_ADJUSTMENT, BOLD_TEXT_ADJUSTMENT); + final AlertDialog dialog = (AlertDialog) mFragment.onCreateDialog( + DialogEnums.DIALOG_RESET_SETTINGS); + dialog.show(); + + dialog.getButton(DialogInterface.BUTTON_POSITIVE).callOnClick(); + + assertThat(mFragment.mNeedResetSettings).isTrue(); + } + + @Test + public void onDialogPositiveButtonClicked_boldTextDisabled_resetAllListeners() { + final ResetStateListener listener1 = mock(ResetStateListener.class); + final ResetStateListener listener2 = mock(ResetStateListener.class); + mFragment.mResetStateListeners = new ArrayList<>(Arrays.asList(listener1, listener2)); + final AlertDialog dialog = (AlertDialog) mFragment.onCreateDialog( + DialogEnums.DIALOG_RESET_SETTINGS); + dialog.show(); + + dialog.getButton(DialogInterface.BUTTON_POSITIVE).callOnClick(); + + verify(listener1).resetState(); + verify(listener2).resetState(); + } +}