From 1f49715466e3213fe27f6c74d992ca0c7ce0dfec Mon Sep 17 00:00:00 2001 From: Doris Ling Date: Thu, 22 Feb 2018 16:33:07 -0800 Subject: [PATCH] Fix reset button being disabled. - When doing factory reset, we will launch the account credential confirmation. If this fails, the settings page is re-initialized. This steps might trigger multiple layout changes for the scrollview before it is finalized. However, we are removing the global layout listener once we receive the first update, which could result in the reset button being disabled incorrectly, as the scroll state is calculated based on wrong view heights. Remove the call that remove the layout listener, so that we can still receive further updates. - also remove the scroll listener once we enable the reset button from scroll, as no more action is really needed on suceeding scroll. Change-Id: I6ec1f592991629c15e5ad2bcb29fdd679d598f70 Fixes: 73298075 Test: make RunSettingsRoboTests --- src/com/android/settings/MasterClear.java | 35 ++++++++++--------- .../com/android/settings/MasterClearTest.java | 25 +++++++++---- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/com/android/settings/MasterClear.java b/src/com/android/settings/MasterClear.java index f66b1f91a61..9ad32e267d2 100644 --- a/src/com/android/settings/MasterClear.java +++ b/src/com/android/settings/MasterClear.java @@ -73,7 +73,7 @@ import java.util.List; * * This is the initial screen. */ -public class MasterClear extends InstrumentedFragment { +public class MasterClear extends InstrumentedFragment implements OnGlobalLayoutListener { private static final String TAG = "MasterClear"; @VisibleForTesting static final int KEYGUARD_REQUEST = 55; @@ -86,20 +86,17 @@ public class MasterClear extends InstrumentedFragment { static final String ERASE_ESIMS_EXTRA = "erase_esim"; private View mContentView; - private Button mInitiateButton; + @VisibleForTesting Button mInitiateButton; private View mExternalStorageContainer; @VisibleForTesting CheckBox mExternalStorage; private View mEsimStorageContainer; @VisibleForTesting CheckBox mEsimStorage; - private ScrollView mScrollView; + @VisibleForTesting ScrollView mScrollView; - private final OnGlobalLayoutListener mOnGlobalLayoutListener = new OnGlobalLayoutListener() { - @Override - public void onGlobalLayout() { - mScrollView.getViewTreeObserver().removeOnGlobalLayoutListener(mOnGlobalLayoutListener); - mInitiateButton.setEnabled(hasReachedBottom(mScrollView)); - } - }; + @Override + public void onGlobalLayout() { + mInitiateButton.setEnabled(hasReachedBottom(mScrollView)); + } @Override public void onCreate(@Nullable Bundle savedInstanceState) { @@ -259,13 +256,16 @@ public class MasterClear extends InstrumentedFragment { */ @VisibleForTesting void establishInitialState() { - mInitiateButton = (Button) mContentView.findViewById(R.id.initiate_master_clear); + mInitiateButton = mContentView.findViewById(R.id.initiate_master_clear); mInitiateButton.setOnClickListener(mInitiateListener); mExternalStorageContainer = mContentView.findViewById(R.id.erase_external_container); - mExternalStorage = (CheckBox) mContentView.findViewById(R.id.erase_external); + mExternalStorage = mContentView.findViewById(R.id.erase_external); mEsimStorageContainer = mContentView.findViewById(R.id.erase_esim_container); - mEsimStorage = (CheckBox) mContentView.findViewById(R.id.erase_esim); - mScrollView = (ScrollView) mContentView.findViewById(R.id.master_clear_scrollview); + mEsimStorage = mContentView.findViewById(R.id.erase_esim); + if (mScrollView != null) { + mScrollView.getViewTreeObserver().removeOnGlobalLayoutListener(this); + } + mScrollView = mContentView.findViewById(R.id.master_clear_scrollview); /* * If the external storage is emulated, it will be erased with a factory @@ -322,8 +322,8 @@ public class MasterClear extends InstrumentedFragment { final UserManager um = (UserManager) getActivity().getSystemService(Context.USER_SERVICE); loadAccountList(um); - StringBuffer contentDescription = new StringBuffer(); - View masterClearContainer = mContentView.findViewById(R.id.master_clear_container); + final StringBuffer contentDescription = new StringBuffer(); + final View masterClearContainer = mContentView.findViewById(R.id.master_clear_container); getContentDescription(masterClearContainer, contentDescription); masterClearContainer.setContentDescription(contentDescription); @@ -334,12 +334,13 @@ public class MasterClear extends InstrumentedFragment { int oldScrollY) { if (v instanceof ScrollView && hasReachedBottom((ScrollView) v)) { mInitiateButton.setEnabled(true); + mScrollView.setOnScrollChangeListener(null); } } }); // Set the initial state of the initiateButton - mScrollView.getViewTreeObserver().addOnGlobalLayoutListener(mOnGlobalLayoutListener); + mScrollView.getViewTreeObserver().addOnGlobalLayoutListener(this); } /** diff --git a/tests/robotests/src/com/android/settings/MasterClearTest.java b/tests/robotests/src/com/android/settings/MasterClearTest.java index c9efe8e1545..7d76444e6c1 100644 --- a/tests/robotests/src/com/android/settings/MasterClearTest.java +++ b/tests/robotests/src/com/android/settings/MasterClearTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -33,7 +34,6 @@ import static org.robolectric.Shadows.shadowOf; import android.accounts.Account; import android.accounts.AccountManager; import android.app.Activity; -import android.app.Fragment; import android.content.ComponentName; import android.content.ContentResolver; import android.content.Context; @@ -41,11 +41,11 @@ import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; -import android.os.Bundle; import android.provider.Settings; import android.view.LayoutInflater; import android.view.View; -import android.widget.CheckBox; +import android.view.ViewTreeObserver; +import android.widget.Button; import android.widget.LinearLayout; import android.widget.ScrollView; @@ -61,7 +61,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.Robolectric; import org.robolectric.annotation.Config; -import org.robolectric.shadows.ShadowAccountManager; import org.robolectric.shadows.ShadowActivity; @RunWith(SettingsRobolectricTestRunner.class) @@ -76,8 +75,6 @@ public class MasterClearTest { private static final String TEST_CONFIRMATION_CLASS = "android.test.conf.pkg.ConfActivity"; private static final String TEST_ACCOUNT_NAME = "test@example.com"; - @Mock - private MasterClear mMasterClear; @Mock private ScrollView mScrollView; @Mock @@ -95,8 +92,8 @@ public class MasterClearTest { @Mock private Intent mMockIntent; + private MasterClear mMasterClear; private ShadowActivity mShadowActivity; - private ShadowAccountManager mShadowAccountManager; private Activity mActivity; private View mContentView; @@ -355,6 +352,7 @@ public class MasterClearTest { assertEquals(TEST_CONFIRMATION_CLASS, actualIntent.getComponent().getClassName()); } + @Test public void testShowAccountCredentialConfirmation() { // Finally mock out the startActivityForResultCall doNothing().when(mMasterClear) @@ -371,6 +369,19 @@ public class MasterClearTest { assertThat(mMasterClear.isValidRequestCode(0)).isFalse(); } + @Test + public void testOnGlobalLayout_shouldNotRemoveListener() { + final ViewTreeObserver viewTreeObserver = mock(ViewTreeObserver.class); + mMasterClear.mScrollView = mScrollView; + mMasterClear.mInitiateButton = mock(Button.class); + doReturn(true).when(mMasterClear).hasReachedBottom(any()); + when(mScrollView.getViewTreeObserver()).thenReturn(viewTreeObserver); + + mMasterClear.onGlobalLayout(); + + verify(viewTreeObserver, never()).removeOnGlobalLayoutListener(mMasterClear); + } + private void initScrollView(int height, int scrollY, int childBottom) { when(mScrollView.getHeight()).thenReturn(height); when(mScrollView.getScrollY()).thenReturn(scrollY);