From e0202b219b273c7be4661bd2995e4bb5f7251ebf Mon Sep 17 00:00:00 2001 From: Antony Sargent Date: Thu, 20 Sep 2018 16:35:06 -0700 Subject: [PATCH] Fix account deletion not updating account display The visible symptom of this problem is that when deleting an account, if a screen lock is set, after confirming the removal and entering the credentials, you end up back on the account details for the page and it looks like the deletion failed (even though it didn't). There were two problems here: -We were expecting the AccountDetailDashboardFragment to be in a resumed state at the end of the confirmation dialog, but it wasn't if we had launched the activity to have the user enter their screen lock credentials. In the past trying to finish an activity that wasn't in resumed state seemed to have generated a crash (b/6494527), but that isn't the case anymore from some tests I ran. -The AccountDetailDashboardFragment doesn't check in onResume that the account still exists. This CL fixes the bug in 2 ways - we'll always try to finish the AccountDetailDashboardFragment if the account removal succeeded, and when AccountDetailDashboardFragment's onResume is called we'll always check for the account existence. Either approach would be sufficient on its own. Change-Id: Iaa65e97fca5dfc8b1251968142e47315e3b590c2 Fixes: 112845988 Test: make RunSettingsRoboTests --- .../AccountDetailDashboardFragment.java | 23 +++++++++++++++++ .../RemoveAccountPreferenceController.java | 4 --- .../AccountDetailDashboardFragmentTest.java | 25 ++++++++++++++++++- ...RemoveAccountPreferenceControllerTest.java | 23 +++++++++++++++-- 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/com/android/settings/accounts/AccountDetailDashboardFragment.java b/src/com/android/settings/accounts/AccountDetailDashboardFragment.java index f98aee1a00a..18e990635d8 100644 --- a/src/com/android/settings/accounts/AccountDetailDashboardFragment.java +++ b/src/com/android/settings/accounts/AccountDetailDashboardFragment.java @@ -16,6 +16,7 @@ package com.android.settings.accounts; import android.accounts.Account; +import android.accounts.AccountManager; import android.app.Activity; import android.content.Context; import android.os.Bundle; @@ -89,6 +90,28 @@ public class AccountDetailDashboardFragment extends DashboardFragment { updateUi(); } + @VisibleForTesting + void finishIfAccountMissing() { + AccountManager accountManager = (AccountManager) getContext().getSystemService( + Context.ACCOUNT_SERVICE); + boolean accountExists = false; + for (Account account : accountManager.getAccountsByType(mAccount.type)) { + if (account.equals(mAccount)) { + accountExists = true; + break; + } + } + if (!accountExists) { + finish(); + } + } + + @Override + public void onResume() { + super.onResume(); + finishIfAccountMissing(); + } + @Override public int getMetricsCategory() { return MetricsEvent.ACCOUNT; diff --git a/src/com/android/settings/accounts/RemoveAccountPreferenceController.java b/src/com/android/settings/accounts/RemoveAccountPreferenceController.java index 24946641c01..9770332e83b 100644 --- a/src/com/android/settings/accounts/RemoveAccountPreferenceController.java +++ b/src/com/android/settings/accounts/RemoveAccountPreferenceController.java @@ -157,10 +157,6 @@ public class RemoveAccountPreferenceController extends AbstractPreferenceControl new AccountManagerCallback() { @Override public void run(AccountManagerFuture future) { - // If already out of this screen, don't proceed. - if (!getTargetFragment().isResumed()) { - return; - } boolean failed = true; try { if (future.getResult() diff --git a/tests/robotests/src/com/android/settings/accounts/AccountDetailDashboardFragmentTest.java b/tests/robotests/src/com/android/settings/accounts/AccountDetailDashboardFragmentTest.java index f2fb1219094..a4d15676327 100644 --- a/tests/robotests/src/com/android/settings/accounts/AccountDetailDashboardFragmentTest.java +++ b/tests/robotests/src/com/android/settings/accounts/AccountDetailDashboardFragmentTest.java @@ -23,9 +23,13 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.accounts.Account; +import android.accounts.AccountManager; import android.content.Context; import android.content.Intent; import android.content.pm.ActivityInfo; @@ -49,6 +53,8 @@ import org.junit.runner.RunWith; import org.robolectric.Robolectric; import org.robolectric.RuntimeEnvironment; import org.robolectric.Shadows; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowAccountManager; import org.robolectric.util.ReflectionHelpers; @RunWith(SettingsRobolectricTestRunner.class) @@ -73,10 +79,11 @@ public class AccountDetailDashboardFragmentTest { final Bundle args = new Bundle(); args.putParcelable(METADATA_USER_HANDLE, UserHandle.CURRENT); - mFragment = new AccountDetailDashboardFragment(); + mFragment = spy(new AccountDetailDashboardFragment()); mFragment.setArguments(args); mFragment.mAccountType = "com.abc"; mFragment.mAccount = new Account("name1@abc.com", "com.abc"); + when(mFragment.getContext()).thenReturn(mContext); } @Test @@ -143,4 +150,20 @@ public class AccountDetailDashboardFragmentTest { assertThat(intent.getStringExtra("extra.accountName")).isEqualTo("name1@abc.com"); } + + @Test + @Config(shadows = {ShadowAccountManager.class}) + public void onResume_accountMissing_shouldFinish() { + mFragment.finishIfAccountMissing(); + verify(mFragment).finish(); + } + + @Test + @Config(shadows = {ShadowAccountManager.class}) + public void onResume_accountPresent_shouldNotFinish() { + AccountManager mgr = mContext.getSystemService(AccountManager.class); + Shadows.shadowOf(mgr).addAccount(mFragment.mAccount); + mFragment.finishIfAccountMissing(); + verify(mFragment, never()).finish(); + } } diff --git a/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java index de67bd2b059..3bebd66e08c 100644 --- a/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java @@ -15,6 +15,8 @@ */ package com.android.settings.accounts; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.Answers.RETURNS_DEEP_STUBS; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Matchers.any; @@ -28,7 +30,10 @@ import static org.mockito.Mockito.when; import android.accounts.Account; import android.accounts.AccountManager; import android.accounts.AccountManagerCallback; +import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorDescription; +import android.accounts.AuthenticatorException; +import android.accounts.OperationCanceledException; import android.app.Activity; import android.content.ComponentName; import android.content.Context; @@ -56,12 +61,14 @@ import com.android.settings.testutils.shadow.ShadowUserManager; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowApplication; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -157,7 +164,8 @@ public class RemoveAccountPreferenceControllerTest { @Test @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) - public void confirmRemove_shouldRemoveAccount() { + public void confirmRemove_shouldRemoveAccount() + throws AuthenticatorException, OperationCanceledException, IOException { when(mFragment.isAdded()).thenReturn(true); FragmentActivity activity = mock(FragmentActivity.class); when(activity.getSystemService(Context.ACCOUNT_SERVICE)).thenReturn(mAccountManager); @@ -170,7 +178,18 @@ public class RemoveAccountPreferenceControllerTest { mFragment, account, userHandle); dialog.onCreate(new Bundle()); dialog.onClick(null, 0); + ArgumentCaptor> callbackCaptor = ArgumentCaptor.forClass( + AccountManagerCallback.class); verify(mAccountManager).removeAccountAsUser(eq(account), nullable(Activity.class), - nullable(AccountManagerCallback.class), nullable(Handler.class), eq(userHandle)); + callbackCaptor.capture(), nullable(Handler.class), eq(userHandle)); + + AccountManagerCallback callback = callbackCaptor.getValue(); + assertThat(callback).isNotNull(); + AccountManagerFuture future = mock(AccountManagerFuture.class); + Bundle resultBundle = new Bundle(); + resultBundle.putBoolean(AccountManager.KEY_BOOLEAN_RESULT, true); + when(future.getResult()).thenReturn(resultBundle); + callback.run(future); + verify(activity).finish(); } }