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
diff --git a/src/com/android/settings/accounts/AccountDetailDashboardFragment.java b/src/com/android/settings/accounts/AccountDetailDashboardFragment.java
index f98aee1..18e9906 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 @@
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 2494664..9770332 100644
--- a/src/com/android/settings/accounts/RemoveAccountPreferenceController.java
+++ b/src/com/android/settings/accounts/RemoveAccountPreferenceController.java
@@ -157,10 +157,6 @@
new AccountManagerCallback<Bundle>() {
@Override
public void run(AccountManagerFuture<Bundle> 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 f2fb121..a4d1567 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 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.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 @@
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 @@
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 de67bd2..3bebd66 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 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 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 @@
@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 @@
mFragment, account, userHandle);
dialog.onCreate(new Bundle());
dialog.onClick(null, 0);
+ ArgumentCaptor<AccountManagerCallback<Bundle>> 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<Bundle> callback = callbackCaptor.getValue();
+ assertThat(callback).isNotNull();
+ AccountManagerFuture<Bundle> 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();
}
}