From e3ecf5bb9e0381bded56592cd7346169d19b8882 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Tue, 23 Apr 2019 15:09:39 -0700 Subject: Fix a NPE when removing accounts. Guard against null activity. If activity is already null, there is no need to call finish(). Fixes: 131180213 Test: robotests Change-Id: I19232ed67ddd0c3539b1827de23fdc584850b519 --- .../RemoveAccountPreferenceController.java | 44 +++++++++++----------- .../RemoveAccountPreferenceControllerTest.java | 36 +++++++++++++++++- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/com/android/settings/accounts/RemoveAccountPreferenceController.java b/src/com/android/settings/accounts/RemoveAccountPreferenceController.java index 938606b19d..1bc30d0313 100644 --- a/src/com/android/settings/accounts/RemoveAccountPreferenceController.java +++ b/src/com/android/settings/accounts/RemoveAccountPreferenceController.java @@ -17,8 +17,6 @@ package com.android.settings.accounts; import android.accounts.Account; import android.accounts.AccountManager; -import android.accounts.AccountManagerCallback; -import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.app.Activity; @@ -30,6 +28,7 @@ import android.content.Intent; import android.os.Bundle; import android.os.UserHandle; import android.os.UserManager; +import android.util.Log; import android.view.View; import android.view.View.OnClickListener; import android.widget.Button; @@ -153,28 +152,27 @@ public class RemoveAccountPreferenceController extends AbstractPreferenceControl public void onClick(DialogInterface dialog, int which) { Activity activity = getTargetFragment().getActivity(); AccountManager.get(activity).removeAccountAsUser(mAccount, activity, - new AccountManagerCallback() { - @Override - public void run(AccountManagerFuture future) { - boolean failed = true; - try { - if (future.getResult() - .getBoolean(AccountManager.KEY_BOOLEAN_RESULT)) { - failed = false; - } - } catch (OperationCanceledException e) { - // handled below - } catch (IOException e) { - // handled below - } catch (AuthenticatorException e) { - // handled below - } - final Activity activity = getTargetFragment().getActivity(); - if (failed && activity != null && !activity.isFinishing()) { - RemoveAccountFailureDialog.show(getTargetFragment()); - } else { - activity.finish(); + future -> { + final Activity targetActivity = getTargetFragment().getActivity(); + if (targetActivity == null || targetActivity.isFinishing()) { + Log.w(TAG, "Activity is no longer alive, skipping results"); + return; + } + boolean failed = true; + try { + if (future.getResult() + .getBoolean(AccountManager.KEY_BOOLEAN_RESULT)) { + failed = false; } + } catch (OperationCanceledException + | IOException + | AuthenticatorException e) { + // handled below + } + if (failed) { + RemoveAccountFailureDialog.show(getTargetFragment()); + } else { + targetActivity.finish(); } }, null, mUserHandle); } diff --git a/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java index cf4cb7cf34..d98d30aff6 100644 --- a/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/accounts/RemoveAccountPreferenceControllerTest.java @@ -144,7 +144,7 @@ public class RemoveAccountPreferenceControllerTest { } @Test - public void onClick_shouldNotStartConfirmDialogWhenModifyAccountsIsDisallowed() { + public void onClick_modifyAccountsIsDisallowed_shouldNotStartConfirmDialog() { when(mFragment.isAdded()).thenReturn(true); final int userId = UserHandle.myUserId(); @@ -195,7 +195,41 @@ public class RemoveAccountPreferenceControllerTest { Bundle resultBundle = new Bundle(); resultBundle.putBoolean(AccountManager.KEY_BOOLEAN_RESULT, true); when(future.getResult()).thenReturn(resultBundle); + callback.run(future); verify(activity).finish(); } + + @Test + @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) + public void confirmRemove_activityGone_shouldSilentlyRemoveAccount() + throws AuthenticatorException, OperationCanceledException, IOException { + final Account account = new Account("Account11", "com.acct1"); + final UserHandle userHandle = new UserHandle(10); + final FragmentActivity activity = mock(FragmentActivity.class); + when(mFragment.isAdded()).thenReturn(true); + when(activity.getSystemService(Context.ACCOUNT_SERVICE)).thenReturn(mAccountManager); + when(mFragment.getActivity()).thenReturn(activity).thenReturn(null); + + final RemoveAccountPreferenceController.ConfirmRemoveAccountDialog dialog = + RemoveAccountPreferenceController.ConfirmRemoveAccountDialog.show( + 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), + 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, never()).finish(); + } } -- cgit v1.2.3