Merge "Fix 6395288: Added lock to avoid unbind race condition" into jb-dev

2 files changed, 242 insertions, 95 deletions

diff --git a/policy/src/com/android/internal/policy/impl/BiometricSensorUnlock.java b/policy/src/com/android/internal/policy/impl/BiometricSensorUnlock.java
index ad26845366f4..a13ccc242c96 100644
--- a/policy/src/com/android/internal/policy/impl/BiometricSensorUnlock.java
+++ b/policy/src/com/android/internal/policy/impl/BiometricSensorUnlock.java
@@ -62,7 +62,7 @@ interface BiometricSensorUnlock {
     public boolean start();
 
     /**
-     * Stops the biometric unlock procedure and unbinds from the service.
+     * Stops the biometric unlock procedure and unbinds from the service.  Called on the UI thread.
      * @return whether the biometric unlock was running when called.
      */
     public boolean stop();
diff --git a/policy/src/com/android/internal/policy/impl/FaceUnlock.java b/policy/src/com/android/internal/policy/impl/FaceUnlock.java
index 16d4003354d2..6e09b7f62375 100644
--- a/policy/src/com/android/internal/policy/impl/FaceUnlock.java
+++ b/policy/src/com/android/internal/policy/impl/FaceUnlock.java
@@ -45,32 +45,44 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
 
     // TODO: is mServiceRunning needed or can we just use mIsRunning or check if mService is null?
     private boolean mServiceRunning = false;
+    // TODO: now that the code has been restructure to do almost all operations from a handler, this
+    // lock may no longer be necessary.
     private final Object mServiceRunningLock = new Object();
     private IFaceLockInterface mService;
     private boolean mBoundToService = false;
     private View mFaceUnlockView;
 
     private Handler mHandler;
-    private final int MSG_SHOW_AREA_VIEW = 0;
-    private final int MSG_HIDE_AREA_VIEW = 1;
+    private final int MSG_SHOW_FACE_UNLOCK_VIEW = 0;
+    private final int MSG_HIDE_FACE_UNLOCK_VIEW = 1;
+    private final int MSG_SERVICE_CONNECTED = 2;
+    private final int MSG_SERVICE_DISCONNECTED = 3;
+    private final int MSG_UNLOCK = 4;
+    private final int MSG_CANCEL = 5;
+    private final int MSG_REPORT_FAILED_ATTEMPT = 6;
+    private final int MSG_EXPOSE_FALLBACK = 7;
+    private final int MSG_POKE_WAKELOCK = 8;
 
     // TODO: This was added for the purpose of adhering to what the biometric interface expects
     // the isRunning() function to return.  However, it is probably not necessary to have both
     // mRunning and mServiceRunning.  I'd just rather wait to change that logic.
-    private boolean mIsRunning = false;
+    private volatile boolean mIsRunning = false;
 
     // Long enough to stay visible while the service starts
     // Short enough to not have to wait long for backup if service fails to start or crashes
     // The service can take a couple of seconds to start on the first try after boot
-    private final int VIEW_AREA_SERVICE_TIMEOUT = 3000;
+    private final int SERVICE_STARTUP_VIEW_TIMEOUT = 3000;
 
     // So the user has a consistent amount of time when brought to the backup method from Face
     // Unlock
     private final int BACKUP_LOCK_TIMEOUT = 5000;
 
-
     KeyguardScreenCallback mKeyguardScreenCallback;
 
+    /**
+     * Stores some of the structures that Face Unlock will need to access and creates the handler
+     * will be used to execute messages on the UI thread.
+     */
     public FaceUnlock(Context context, KeyguardUpdateMonitor updateMonitor,
             LockPatternUtils lockPatternUtils, KeyguardScreenCallback keyguardScreenCallback) {
         mContext = context;
@@ -86,6 +98,7 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * methods, we will have to add our other views (background, cancel button) here.
      */
     public void initializeView(View biometricUnlockView) {
+        Log.d(TAG, "initializeView()");
         mFaceUnlockView = biometricUnlockView;
         show(0);
     }
@@ -102,12 +115,13 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * timeoutMillis is 0, no hide is performed.
      */
     public void show(long timeoutMillis) {
-        removeAreaDisplayMessages();
+        if (DEBUG) Log.d(TAG, "show()");
+        removeDisplayMessages();
         if (mFaceUnlockView != null) {
             mFaceUnlockView.setVisibility(View.VISIBLE);
         }
         if (timeoutMillis > 0) {
-            mHandler.sendEmptyMessageDelayed(MSG_HIDE_AREA_VIEW, timeoutMillis);
+            mHandler.sendEmptyMessageDelayed(MSG_HIDE_FACE_UNLOCK_VIEW, timeoutMillis);
         }
     }
 
@@ -115,16 +129,18 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * Hides the Face Unlock view.
      */
     public void hide() {
+        if (DEBUG) Log.d(TAG, "hide()");
         // Remove messages to prevent a delayed show message from undo-ing the hide
-        removeAreaDisplayMessages();
-        mHandler.sendEmptyMessage(MSG_HIDE_AREA_VIEW);
+        removeDisplayMessages();
+        mHandler.sendEmptyMessage(MSG_HIDE_FACE_UNLOCK_VIEW);
     }
 
     /**
      * Binds to the Face Unlock service.  Face Unlock will be started when the bind completes.  The
-     * Face Unlock area is displayed to hide the backup while the service is starting up.
+     * Face Unlock view is displayed to hide the backup lock while the service is starting up.
      */
     public boolean start() {
+        if (DEBUG) Log.d(TAG, "start()");
         if (mIsRunning) {
             Log.w(TAG, "start() called when already running");
         }
@@ -132,14 +148,13 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
         // Show Face Unlock view, but only for a little bit so lockpattern will become visible if
         // Face Unlock fails to start or crashes
         // This must show before bind to guarantee that Face Unlock has a place to display
-        show(VIEW_AREA_SERVICE_TIMEOUT);
+        show(SERVICE_STARTUP_VIEW_TIMEOUT);
         if (!mBoundToService) {
-            if (DEBUG) Log.d(TAG, "before bind to Face Unlock service");
+            Log.d(TAG, "Binding to Face Unlock service");
             mContext.bindService(new Intent(IFaceLockInterface.class.getName()),
                     mConnection,
                     Context.BIND_AUTO_CREATE,
                     mLockPatternUtils.getCurrentUser());
-            if (DEBUG) Log.d(TAG, "after bind to Face Unlock service");
             mBoundToService = true;
         } else {
             Log.w(TAG, "Attempt to bind to Face Unlock when already bound");
@@ -158,11 +173,11 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * Stops Face Unlock and unbinds from the service.
      */
     public boolean stop() {
+        if (DEBUG) Log.d(TAG, "stop()");
         boolean mWasRunning = mIsRunning;
         stopUi();
 
         if (mBoundToService) {
-            if (DEBUG) Log.d(TAG, "before unbind from Face Unlock service");
             if (mService != null) {
                 try {
                     mService.unregisterCallback(mFaceUnlockCallback);
@@ -170,8 +185,8 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
                     // Not much we can do
                 }
             }
+            Log.d(TAG, "Unbinding from Face Unlock service");
             mContext.unbindService(mConnection);
-            if (DEBUG) Log.d(TAG, "after unbind from Face Unlock service");
             mBoundToService = false;
         } else {
             // This is usually not an error when this happens.  Sometimes we will tell it to
@@ -187,6 +202,7 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * Frees up resources used by Face Unlock and stops it if it is still running.
      */
     public void cleanUp() {
+        if (DEBUG) Log.d(TAG, "cleanUp()");
         if (mService != null) {
             try {
                 mService.unregisterCallback(mFaceUnlockCallback);
@@ -206,86 +222,224 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
     }
 
     /**
-     * Handles showing the Face Unlock view (hiding the backup lock) and hiding the Face Unlock view
-     * (exposing the backup lock).  In cases where 'show' needs to happen immediately,
-     * setVisibility() is called directly (without using this handler).  This handler is used when
-     * 'show' needs to happen from a non-UI thread.  It also handles hide() messages since they
-     * often require a delay.
+     * Handles messages such that everything happens on the UI thread in a deterministic order.
+     * Calls from the Face Unlock service come from binder threads.  Calls from lockscreen typically
+     * come from the UI thread.  This makes sure there are no race conditions between those calls.
      */
     @Override
     public boolean handleMessage(Message msg) {
         switch (msg.what) {
-        case MSG_SHOW_AREA_VIEW:
-            if (mFaceUnlockView != null) {
-                mFaceUnlockView.setVisibility(View.VISIBLE);
-            }
-            break;
-        case MSG_HIDE_AREA_VIEW:
-            if (mFaceUnlockView != null) {
-                mFaceUnlockView.setVisibility(View.INVISIBLE);
-            }
-            break;
-        default:
-            Log.w(TAG, "Unhandled message");
-            return false;
+            case MSG_SHOW_FACE_UNLOCK_VIEW:
+                handleShowFaceUnlockView();
+                break;
+            case MSG_HIDE_FACE_UNLOCK_VIEW:
+                handleHideFaceUnlockView();
+                break;
+            case MSG_SERVICE_CONNECTED:
+                handleServiceConnected();
+                break;
+            case MSG_SERVICE_DISCONNECTED:
+                handleServiceDisconnected();
+                break;
+            case MSG_UNLOCK:
+                handleUnlock();
+                break;
+            case MSG_CANCEL:
+                handleCancel();
+                break;
+            case MSG_REPORT_FAILED_ATTEMPT:
+                handleReportFailedAttempt();
+                break;
+            case MSG_EXPOSE_FALLBACK:
+                handleExposeFallback();
+                break;
+            case MSG_POKE_WAKELOCK:
+                handlePokeWakelock();
+                break;
+            default:
+                Log.e(TAG, "Unhandled message");
+                return false;
         }
         return true;
     }
 
     /**
-     * Removes show and hide messages from the message queue
+     * Sets the Face Unlock view to visible, thus covering the backup lock.
+     */
+    void handleShowFaceUnlockView() {
+        if (DEBUG) Log.d(TAG, "handleShowFaceUnlockView()");
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.VISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleShowFaceUnlockView()");
+        }
+    }
+
+    /**
+     * Sets the Face Unlock view to invisible, thus exposing the backup lock.
+     */
+    void handleHideFaceUnlockView() {
+        if (DEBUG) Log.d(TAG, "handleHideFaceUnlockView()");
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.INVISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleHideFaceUnlockView()");
+        }
+    }
+
+    /**
+     * Tells the service to start its UI via an AIDL interface.  Called when the
+     * onServiceConnected() callback is received.
+     */
+    void handleServiceConnected() {
+        if (DEBUG) Log.d(TAG, "handleServiceConnected()");
+        try {
+            mService.registerCallback(mFaceUnlockCallback);
+        } catch (RemoteException e) {
+            Log.e(TAG, "Caught exception connecting to Face Unlock: " + e.toString());
+            mService = null;
+            mBoundToService = false;
+            mIsRunning = false;
+            return;
+        }
+
+        if (mFaceUnlockView != null) {
+            IBinder windowToken = mFaceUnlockView.getWindowToken();
+            if (windowToken != null) {
+                int[] position;
+                position = new int[2];
+                mFaceUnlockView.getLocationInWindow(position);
+                startUi(windowToken, position[0], position[1], mFaceUnlockView.getWidth(),
+                        mFaceUnlockView.getHeight());
+            } else {
+                Log.e(TAG, "windowToken is null in handleServiceConnected()");
+            }
+        }
+    }
+
+    /**
+     * Called when the onServiceDisconnected() callback is received.  This should not happen during
+     * normal operation.  It indicates an error has occurred.
+     */
+    void handleServiceDisconnected() {
+        Log.e(TAG, "handleServiceDisconnected()");
+        // TODO: this lock may no longer be needed now that everything is being called from a
+        // handler
+        synchronized (mServiceRunningLock) {
+            mService = null;
+            mServiceRunning = false;
+        }
+        mBoundToService = false;
+        mIsRunning = false;
+    }
+
+    /**
+     * Stops the Face Unlock service and tells the device to grant access to the user.  Shows the
+     * Face Unlock view to keep the backup lock covered while the device unlocks.
+     */
+    void handleUnlock() {
+        if (DEBUG) Log.d(TAG, "handleUnlock()");
+        removeDisplayMessages();
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.VISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleUnlock()");
+        }
+        stop();
+        mKeyguardScreenCallback.keyguardDone(true);
+        mKeyguardScreenCallback.reportSuccessfulUnlockAttempt();
+    }
+
+    /**
+     * Stops the Face Unlock service and exposes the backup lock.  Called when the user presses the
+     * cancel button to skip Face Unlock or no face is detected.
+     */
+    void handleCancel() {
+        if (DEBUG) Log.d(TAG, "handleCancel()");
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.INVISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleCancel()");
+        }
+        stop();
+        mKeyguardScreenCallback.pokeWakelock(BACKUP_LOCK_TIMEOUT);
+    }
+
+    /**
+     * Stops the Face Unlock service and exposes the backup lock, reporting a failed unlock attempt.
+     * Called when Face Unlock denies access to the user.
+     */
+    void handleReportFailedAttempt() {
+        if (DEBUG) Log.d(TAG, "handleReportFailedAttempt()");
+        mUpdateMonitor.reportFailedBiometricUnlockAttempt();
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.INVISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleReportFailedAttempt()");
+        }
+        stop();
+        mKeyguardScreenCallback.pokeWakelock(BACKUP_LOCK_TIMEOUT);
+    }
+
+    /**
+     * Hides the Face Unlock view to expose the backup lock.  Called when the Face Unlock service UI
+     * is started, indicating there is no need to continue displaying the underlying view because
+     * the service UI is now covering the backup lock.
+     */
+    void handleExposeFallback() {
+        if (DEBUG) Log.d(TAG, "handleExposeFallback()");
+        if (mFaceUnlockView != null) {
+            mFaceUnlockView.setVisibility(View.INVISIBLE);
+        } else {
+            Log.e(TAG, "mFaceUnlockView is null in handleExposeFallback()");
+        }
+    }
+
+    /**
+     * Pokes the wakelock to keep the screen alive and active.
+     */
+    void handlePokeWakelock() {
+        mKeyguardScreenCallback.pokeWakelock();
+    }
+
+    /**
+     * Removes show and hide messages from the message queue.  Called to prevent delayed show/hide
+     * messages from undoing a new message.
      */
-    private void removeAreaDisplayMessages() {
-        mHandler.removeMessages(MSG_SHOW_AREA_VIEW);
-        mHandler.removeMessages(MSG_HIDE_AREA_VIEW);
+    private void removeDisplayMessages() {
+        mHandler.removeMessages(MSG_SHOW_FACE_UNLOCK_VIEW);
+        mHandler.removeMessages(MSG_HIDE_FACE_UNLOCK_VIEW);
     }
 
+    /**
+     * Implements service connection methods.
+     */
     private ServiceConnection mConnection = new ServiceConnection() {
         /**
-         * Completes connection, registers callback, and starts Face Unlock when service is bound
+         * Called when the Face Unlock service connects after calling bind().
          */
         @Override
         public void onServiceConnected(ComponentName className, IBinder iservice) {
+            Log.d(TAG, "Connected to Face Unlock service");
             mService = IFaceLockInterface.Stub.asInterface(iservice);
-            if (DEBUG) Log.d(TAG, "Connected to Face Unlock service");
-            try {
-                mService.registerCallback(mFaceUnlockCallback);
-            } catch (RemoteException e) {
-                Log.e(TAG, "Caught exception connecting to Face Unlock: " + e.toString());
-                mService = null;
-                mBoundToService = false;
-                mIsRunning = false;
-                return;
-            }
-
-            if (mFaceUnlockView != null) {
-                int[] position;
-                position = new int[2];
-                mFaceUnlockView.getLocationInWindow(position);
-                startUi(mFaceUnlockView.getWindowToken(), position[0], position[1],
-                        mFaceUnlockView.getWidth(), mFaceUnlockView.getHeight());
-            }
+            mHandler.sendEmptyMessage(MSG_SERVICE_CONNECTED);
         }
 
         /**
-         * Cleans up if Face Unlock service unexpectedly disconnects
+         * Called if the Face Unlock service unexpectedly disconnects.  This indicates an error.
          */
         @Override
         public void onServiceDisconnected(ComponentName className) {
-            synchronized(mServiceRunningLock) {
-                mService = null;
-                mServiceRunning = false;
-            }
-            mBoundToService = false;
-            mIsRunning = false;
-            Log.w(TAG, "Unexpected disconnect from Face Unlock service");
+            Log.e(TAG, "Unexpected disconnect from Face Unlock service");
+            mHandler.sendEmptyMessage(MSG_SERVICE_DISCONNECTED);
         }
     };
 
     /**
-     * Tells the Face Unlock service to start displaying its UI and perform recognition
+     * Tells the Face Unlock service to start displaying its UI and start processing.
      */
     private void startUi(IBinder windowToken, int x, int y, int w, int h) {
+        if (DEBUG) Log.d(TAG, "startUi()");
         synchronized (mServiceRunningLock) {
             if (!mServiceRunning) {
                 if (DEBUG) Log.d(TAG, "Starting Face Unlock");
@@ -298,93 +452,86 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
                 }
                 mServiceRunning = true;
             } else {
-                if (DEBUG) Log.w(TAG, "startUi() attempted while running");
+                Log.w(TAG, "startUi() attempted while running");
             }
         }
     }
 
     /**
-     * Tells the Face Unlock service to stop displaying its UI and stop recognition
+     * Tells the Face Unlock service to stop displaying its UI and stop processing.
      */
     private void stopUi() {
+        if (DEBUG) Log.d(TAG, "stopUi()");
         // Note that attempting to stop Face Unlock when it's not running is not an issue.
         // Face Unlock can return, which stops it and then we try to stop it when the
         // screen is turned off.  That's why we check.
         synchronized (mServiceRunningLock) {
             if (mServiceRunning) {
+                if (DEBUG) Log.d(TAG, "Stopping Face Unlock");
                 try {
-                    if (DEBUG) Log.d(TAG, "Stopping Face Unlock");
                     mService.stopUi();
                 } catch (RemoteException e) {
                     Log.e(TAG, "Caught exception stopping Face Unlock: " + e.toString());
                 }
                 mServiceRunning = false;
+            } else {
+                // This is usually not an error when this happens.  Sometimes we will tell it to
+                // stop multiple times because it's called from both onWindowFocusChanged and
+                // onDetachedFromWindow.
+                if (DEBUG) Log.d(TAG, "stopUi() attempted while not running");
             }
         }
     }
 
     /**
-     * Implements the biometric unlock service callback interface defined in AIDL
+     * Implements the AIDL biometric unlock service callback interface.
      */
     private final IFaceLockCallback mFaceUnlockCallback = new IFaceLockCallback.Stub() {
         /**
-         * Stops the Face Unlock UI and indicates that the phone should be unlocked
+         * Called when Face Unlock wants to grant access to the user.
          */
         @Override
         public void unlock() {
             if (DEBUG) Log.d(TAG, "unlock()");
-
-            // Keep fallback covered
-            removeAreaDisplayMessages();
-            mHandler.sendEmptyMessage(MSG_SHOW_AREA_VIEW);
-
-            stop();
-
-            mKeyguardScreenCallback.keyguardDone(true);
-            mKeyguardScreenCallback.reportSuccessfulUnlockAttempt();
+            mHandler.sendEmptyMessage(MSG_UNLOCK);
         }
 
         /**
-         * Stops the Face Unlock UI and exposes the backup method without unlocking
-         * This means the user has cancelled out
+         * Called when the user presses cancel to skip Face Unlock or a face cannot be found.
          */
         @Override
         public void cancel() {
             if (DEBUG) Log.d(TAG, "cancel()");
-            hide(); // Expose fallback
-            stop();
-            mKeyguardScreenCallback.pokeWakelock(BACKUP_LOCK_TIMEOUT);
+            mHandler.sendEmptyMessage(MSG_CANCEL);
         }
 
         /**
-         * Stops the Face Unlock UI and exposes the backup method without unlocking
-         * This means Face Unlock failed to recognize them
+         * Called when Face Unlock denies access to the user.
          */
         @Override
         public void reportFailedAttempt() {
             if (DEBUG) Log.d(TAG, "reportFailedAttempt()");
-            mUpdateMonitor.reportFailedBiometricUnlockAttempt();
-            hide(); // Expose fallback
-            stop();
-            mKeyguardScreenCallback.pokeWakelock(BACKUP_LOCK_TIMEOUT);
+            mHandler.sendEmptyMessage(MSG_REPORT_FAILED_ATTEMPT);
         }
 
         /**
-         * Removes the black area that covers the backup unlock method
+         * Called when the Face Unlock service starts displaying the UI, indicating that the backup
+         * unlock can be exposed because the Face Unlock service is now covering the backup with its
+         * UI.
          **/
         @Override
         public void exposeFallback() {
             if (DEBUG) Log.d(TAG, "exposeFallback()");
-            hide(); // Expose fallback
+            mHandler.sendEmptyMessage(MSG_EXPOSE_FALLBACK);
         }
 
         /**
-         * Allows the Face Unlock service to poke the wake lock to keep the lockscreen alive
+         * Called when Face Unlock wants to keep the screen alive and active.
          */
         @Override
         public void pokeWakelock() {
             if (DEBUG) Log.d(TAG, "pokeWakelock()");
-            mKeyguardScreenCallback.pokeWakelock();
+            mHandler.sendEmptyMessage(MSG_POKE_WAKELOCK);
         }
     };
 }