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

3 files changed, 179 insertions, 66 deletions

diff --git a/src/com/android/settings/ConfirmLockPassword.java b/src/com/android/settings/ConfirmLockPassword.java
index 1c42045..cce29dd 100644
--- a/src/com/android/settings/ConfirmLockPassword.java
+++ b/src/com/android/settings/ConfirmLockPassword.java
@@ -16,10 +16,8 @@
 
 package com.android.settings;
 
-import android.os.UserHandle;
 import android.text.TextUtils;
 import com.android.internal.logging.MetricsLogger;
-import com.android.internal.util.ArrayUtils;
 import com.android.internal.widget.LockPatternChecker;
 import com.android.internal.widget.LockPatternUtils;
 import com.android.internal.widget.TextViewInputDisabler;
@@ -78,19 +76,20 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
     }
 
     public static class ConfirmLockPasswordFragment extends ConfirmDeviceCredentialBaseFragment
-            implements OnClickListener, OnEditorActionListener {
-        private static final String KEY_NUM_WRONG_CONFIRM_ATTEMPTS
-                = "confirm_lock_password_fragment.key_num_wrong_confirm_attempts";
+            implements OnClickListener, OnEditorActionListener,
+            CredentialCheckResultTracker.Listener {
         private static final long ERROR_MESSAGE_TIMEOUT = 3000;
+        private static final String FRAGMENT_TAG_CHECK_LOCK_RESULT = "check_lock_result";
         private TextView mPasswordEntry;
         private TextViewInputDisabler mPasswordEntryInputDisabler;
         private LockPatternUtils mLockPatternUtils;
         private AsyncTask<?, ?, ?> mPendingLockCheck;
+        private CredentialCheckResultTracker mCredentialCheckResultTracker;
+        private boolean mDisappearing = false;
         private TextView mHeaderTextView;
         private TextView mDetailsTextView;
         private TextView mErrorTextView;
         private Handler mHandler = new Handler();
-        private int mNumWrongConfirmAttempts;
         private CountDownTimer mCountdownTimer;
         private boolean mIsAlpha;
         private InputMethodManager mImm;
@@ -110,11 +109,6 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
             super.onCreate(savedInstanceState);
             mLockPatternUtils = new LockPatternUtils(getActivity());
             mEffectiveUserId = Utils.getEffectiveUserId(getActivity());
-
-            if (savedInstanceState != null) {
-                mNumWrongConfirmAttempts = savedInstanceState.getInt(
-                        KEY_NUM_WRONG_CONFIRM_ATTEMPTS, 0);
-            }
         }
 
         @Override
@@ -165,6 +159,15 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                     0.5f /* delayScale */, AnimationUtils.loadInterpolator(
                             getContext(), android.R.interpolator.fast_out_linear_in));
             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;
         }
 
@@ -227,10 +230,7 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                 mCountdownTimer.cancel();
                 mCountdownTimer = null;
             }
-            if (mPendingLockCheck != null) {
-                mPendingLockCheck.cancel(false);
-                mPendingLockCheck = null;
-            }
+            mCredentialCheckResultTracker.setListener(null);
         }
 
         @Override
@@ -243,21 +243,17 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
             super.onResume();
             long deadline = mLockPatternUtils.getLockoutAttemptDeadline(mEffectiveUserId);
             if (deadline != 0) {
+                mCredentialCheckResultTracker.clearResult();
                 handleAttemptLockout(deadline);
             } else {
                 resetState();
             }
-        }
-
-        @Override
-        public void onSaveInstanceState(Bundle outState) {
-            super.onSaveInstanceState(outState);
-            outState.putInt(KEY_NUM_WRONG_CONFIRM_ATTEMPTS, mNumWrongConfirmAttempts);
+            mCredentialCheckResultTracker.setListener(this);
         }
 
         @Override
         protected void authenticationSucceeded() {
-            startDisappearAnimation(new Intent());
+            mCredentialCheckResultTracker.setResult(true, new Intent(), 0, mEffectiveUserId);
         }
 
         @Override
@@ -298,11 +294,12 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
         }
 
         private void handleNext() {
-            mPasswordEntryInputDisabler.setInputEnabled(false);
-            if (mPendingLockCheck != null) {
-                mPendingLockCheck.cancel(false);
+            if (mPendingLockCheck != null || mDisappearing) {
+                return;
             }
 
+            mPasswordEntryInputDisabler.setInputEnabled(false);
+
             final String pin = mPasswordEntry.getText().toString();
             final boolean verifyChallenge = getActivity().getIntent().getBooleanExtra(
                     ChooseLockSettingsHelper.EXTRA_KEY_HAS_CHALLENGE, false);
@@ -317,7 +314,7 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                 return;
             }
 
-            onPasswordChecked(false, intent, 0, mEffectiveUserId);
+            mCredentialCheckResultTracker.setResult(false, intent, 0, mEffectiveUserId);
         }
 
         private boolean isInternalActivity() {
@@ -344,7 +341,8 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                                         ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN,
                                         token);
                             }
-                            onPasswordChecked(matched, intent, timeoutMs, localEffectiveUserId);
+                            mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs,
+                                    localEffectiveUserId);
                         }
                     });
         }
@@ -366,16 +364,27 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                                 intent.putExtra(
                                         ChooseLockSettingsHelper.EXTRA_KEY_PASSWORD, pin);
                             }
-                            onPasswordChecked(matched, intent, timeoutMs, localEffectiveUserId);
+                            mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs,
+                                    localEffectiveUserId);
                         }
                     });
         }
 
         private void startDisappearAnimation(final Intent intent) {
+            if (mDisappearing) {
+                return;
+            }
+            mDisappearing = true;
+
             if (getActivity().getThemeResId() == R.style.Theme_ConfirmDeviceCredentialsDark) {
                 mDisappearAnimationUtils.startAnimation(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(
@@ -405,6 +414,12 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
             }
         }
 
+        @Override
+        public void onCredentialChecked(boolean matched, Intent intent, int timeoutMs,
+                int effectiveUserId) {
+            onPasswordChecked(matched, intent, timeoutMs, effectiveUserId);
+        }
+
         private void handleAttemptLockout(long elapsedRealtimeDeadline) {
             long elapsedRealtime = SystemClock.elapsedRealtime();
             mPasswordEntry.setEnabled(false);
@@ -424,7 +439,6 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity {
                 public void onFinish() {
                     resetState();
                     mErrorTextView.setText("");
-                    mNumWrongConfirmAttempts = 0;
                 }
             }.start();
         }
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();
diff --git a/src/com/android/settings/CredentialCheckResultTracker.java b/src/com/android/settings/CredentialCheckResultTracker.java
new file mode 100644
index 0000000..7f72caa
--- /dev/null
+++ b/src/com/android/settings/CredentialCheckResultTracker.java
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settings;
+
+import android.app.Fragment;
+import android.content.Intent;
+import android.os.Bundle;
+
+/**
+ * An invisible retained fragment to track lock check result.
+ */
+class CredentialCheckResultTracker extends Fragment {
+
+    private Listener mListener;
+    private boolean mHasResult = false;
+
+    private boolean mResultMatched;
+    private Intent mResultData;
+    private int mResultTimeoutMs;
+    private int mResultEffectiveUserId;
+
+    @Override
+    public void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        setRetainInstance(true);
+    }
+
+    public void setListener(Listener listener) {
+        if (mListener == listener) {
+            return;
+        }
+
+        mListener = listener;
+        if (mListener != null && mHasResult) {
+            mListener.onCredentialChecked(mResultMatched, mResultData, mResultTimeoutMs,
+                    mResultEffectiveUserId);
+        }
+    }
+
+    public void setResult(boolean matched, Intent intent, int timeoutMs, int effectiveUserId) {
+        mResultMatched = matched;
+        mResultData = intent;
+        mResultTimeoutMs = timeoutMs;
+        mResultEffectiveUserId = effectiveUserId;
+
+        mHasResult = true;
+        if (mListener != null) {
+            mListener.onCredentialChecked(mResultMatched, mResultData, mResultTimeoutMs,
+                    mResultEffectiveUserId);
+        }
+    }
+
+    public void clearResult() {
+        mHasResult = false;
+        mResultMatched = false;
+        mResultData = null;
+        mResultTimeoutMs = 0;
+        mResultEffectiveUserId = 0;
+    }
+
+    interface Listener {
+        public void onCredentialChecked(boolean matched, Intent intent, int timeoutMs,
+                int effectiveUserId);
+    }
+}