Fix races in ConfirmPassword/Pattern

- Add a CheckLockResultTracker to track result of async lock check so that
  it can finish on configuration change;
- Let the pending lock check finish and ignore subsequent check requests;
- Add a mDisappearing flag to prevent running disappear animation
  multiple times;
- Check whether activity is still active after disappear animation
  before setting result and finishing it;
- Remove no longer used mNumWrongConfirmAttempts;

Bug:23190499
Change-Id: If1784d3d1fcc152ac06137b12748b9def5726692

1 files changed, 57 insertions, 37 deletions

diff --git a/src/com/android/settings/ConfirmLockPattern.java b/src/com/android/settings/ConfirmLockPattern.java
index 51cde60..94ba01a 100644
--- a/src/com/android/settings/ConfirmLockPattern.java
+++ b/src/com/android/settings/ConfirmLockPattern.java
@@ -32,7 +32,6 @@ import android.os.CountDownTimer;
 import android.os.SystemClock;
 import android.os.AsyncTask;
 import android.os.Bundle;
-import android.os.UserHandle;
 import android.os.storage.StorageManager;
 import android.view.animation.AnimationUtils;
 import android.view.animation.Interpolator;
@@ -76,17 +75,18 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
     }
 
     public static class ConfirmLockPatternFragment extends ConfirmDeviceCredentialBaseFragment
-            implements AppearAnimationCreator<Object> {
+            implements AppearAnimationCreator<Object>, CredentialCheckResultTracker.Listener {
 
         // how long we wait to clear a wrong pattern
         private static final int WRONG_PATTERN_CLEAR_TIMEOUT_MS = 2000;
 
-        private static final String KEY_NUM_WRONG_ATTEMPTS = "num_wrong_attempts";
+        private static final String FRAGMENT_TAG_CHECK_LOCK_RESULT = "check_lock_result";
 
         private LockPatternView mLockPatternView;
         private LockPatternUtils mLockPatternUtils;
         private AsyncTask<?, ?, ?> mPendingLockCheck;
-        private int mNumWrongConfirmAttempts;
+        private CredentialCheckResultTracker mCredentialCheckResultTracker;
+        private boolean mDisappearing = false;
         private CountDownTimer mCountdownTimer;
 
         private TextView mHeaderTextView;
@@ -148,9 +148,7 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
             mLockPatternView.setOnPatternListener(mConfirmExistingLockPatternListener);
             updateStage(Stage.NeedToUnlock);
 
-            if (savedInstanceState != null) {
-                mNumWrongConfirmAttempts = savedInstanceState.getInt(KEY_NUM_WRONG_ATTEMPTS);
-            } else {
+            if (savedInstanceState == null) {
                 // on first launch, if no lock pattern is set, then finish with
                 // success (don't want user to get stuck confirming something that
                 // doesn't exist).
@@ -174,13 +172,21 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
                         }
                     });
             setAccessibilityTitle(mHeaderTextView.getText());
+
+            mCredentialCheckResultTracker = (CredentialCheckResultTracker) getFragmentManager()
+                    .findFragmentByTag(FRAGMENT_TAG_CHECK_LOCK_RESULT);
+            if (mCredentialCheckResultTracker == null) {
+                mCredentialCheckResultTracker = new CredentialCheckResultTracker();
+                getFragmentManager().beginTransaction().add(mCredentialCheckResultTracker,
+                        FRAGMENT_TAG_CHECK_LOCK_RESULT).commit();
+            }
+
             return view;
         }
 
         @Override
         public void onSaveInstanceState(Bundle outState) {
             // deliberately not calling super since we are managing this in full
-            outState.putInt(KEY_NUM_WRONG_ATTEMPTS, mNumWrongConfirmAttempts);
         }
 
         @Override
@@ -190,10 +196,7 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
             if (mCountdownTimer != null) {
                 mCountdownTimer.cancel();
             }
-            if (mPendingLockCheck != null) {
-                mPendingLockCheck.cancel(false);
-                mPendingLockCheck = null;
-            }
+            mCredentialCheckResultTracker.setListener(null);
         }
 
         @Override
@@ -208,13 +211,14 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
             // if the user is currently locked out, enforce it.
             long deadline = mLockPatternUtils.getLockoutAttemptDeadline(mEffectiveUserId);
             if (deadline != 0) {
+                mCredentialCheckResultTracker.clearResult();
                 handleAttemptLockout(deadline);
             } else if (!mLockPatternView.isEnabled()) {
                 // The deadline has passed, but the timer was cancelled. Or the pending lock
                 // check was cancelled. Need to clean up.
-                mNumWrongConfirmAttempts = 0;
                 updateStage(Stage.NeedToUnlock);
             }
+            mCredentialCheckResultTracker.setListener(this);
         }
 
         @Override
@@ -317,16 +321,26 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
 
         @Override
         protected void authenticationSucceeded() {
-            startDisappearAnimation(new Intent());
+            mCredentialCheckResultTracker.setResult(true, new Intent(), 0, mEffectiveUserId);
         }
 
         private void startDisappearAnimation(final Intent intent) {
+            if (mDisappearing) {
+                return;
+            }
+            mDisappearing = true;
+
             if (getActivity().getThemeResId() == R.style.Theme_ConfirmDeviceCredentialsDark) {
                 mLockPatternView.clearPattern();
                 mDisappearAnimationUtils.startAnimation2d(getActiveViews(),
                         new Runnable() {
                             @Override
                             public void run() {
+                                // Bail if there is no active activity.
+                                if (getActivity() == null || getActivity().isFinishing()) {
+                                    return;
+                                }
+
                                 getActivity().setResult(RESULT_OK, intent);
                                 getActivity().finish();
                                 getActivity().overridePendingTransition(
@@ -370,11 +384,12 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
             }
 
             public void onPatternDetected(List<LockPatternView.Cell> pattern) {
-                mLockPatternView.setEnabled(false);
-                if (mPendingLockCheck != null) {
-                    mPendingLockCheck.cancel(false);
+                if (mPendingLockCheck != null || mDisappearing) {
+                    return;
                 }
 
+                mLockPatternView.setEnabled(false);
+
                 final boolean verifyChallenge = getActivity().getIntent().getBooleanExtra(
                         ChooseLockSettingsHelper.EXTRA_KEY_HAS_CHALLENGE, false);
                 Intent intent = new Intent();
@@ -388,7 +403,7 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
                     return;
                 }
 
-                onPatternChecked(pattern, false, intent, 0, mEffectiveUserId);
+                mCredentialCheckResultTracker.setResult(false, intent, 0, mEffectiveUserId);
             }
 
             private boolean isInternalActivity() {
@@ -416,8 +431,8 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
                                             ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN,
                                             token);
                                 }
-                                onPatternChecked(pattern,
-                                        matched, intent, timeoutMs, localEffectiveUserId);
+                                mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs,
+                                        localEffectiveUserId);
                             }
                         });
             }
@@ -425,7 +440,7 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
             private void startCheckPattern(final List<LockPatternView.Cell> pattern,
                     final Intent intent) {
                 if (pattern.size() < LockPatternUtils.MIN_PATTERN_REGISTER_FAIL) {
-                    onPatternChecked(pattern, false, intent, 0, mEffectiveUserId);
+                    mCredentialCheckResultTracker.setResult(false, intent, 0, mEffectiveUserId);
                     return;
                 }
 
@@ -444,29 +459,35 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
                                     intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_PASSWORD,
                                                     LockPatternUtils.patternToString(pattern));
                                 }
-                                onPatternChecked(pattern, matched, intent, timeoutMs,
+                                mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs,
                                         localEffectiveUserId);
                             }
                         });
             }
+        };
 
-            private void onPatternChecked(List<LockPatternView.Cell> pattern,
-                    boolean matched, Intent intent, int timeoutMs, int effectiveUserId) {
-                mLockPatternView.setEnabled(true);
-                if (matched) {
-                    startDisappearAnimation(intent);
+        private void onPatternChecked(boolean matched, Intent intent, int timeoutMs,
+                int effectiveUserId) {
+            mLockPatternView.setEnabled(true);
+            if (matched) {
+                startDisappearAnimation(intent);
+            } else {
+                if (timeoutMs > 0) {
+                    long deadline = mLockPatternUtils.setLockoutAttemptDeadline(
+                            effectiveUserId, timeoutMs);
+                    handleAttemptLockout(deadline);
                 } else {
-                    if (timeoutMs > 0) {
-                        long deadline = mLockPatternUtils.setLockoutAttemptDeadline(
-                                effectiveUserId, timeoutMs);
-                        handleAttemptLockout(deadline);
-                    } else {
-                        updateStage(Stage.NeedToUnlockWrong);
-                        postClearPatternRunnable();
-                    }
+                    updateStage(Stage.NeedToUnlockWrong);
+                    postClearPatternRunnable();
                 }
             }
-        };
+        }
+
+        @Override
+        public void onCredentialChecked(boolean matched, Intent intent, int timeoutMs,
+                int effectiveUserId) {
+            onPatternChecked(matched, intent, timeoutMs, effectiveUserId);
+        }
 
         private void handleAttemptLockout(long elapsedRealtimeDeadline) {
             updateStage(Stage.LockedOut);
@@ -485,7 +506,6 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity {
 
                 @Override
                 public void onFinish() {
-                    mNumWrongConfirmAttempts = 0;
                     updateStage(Stage.NeedToUnlock);
                 }
             }.start();