From c266070aec3d99c9a4d422325fbceae44c37a4d3 Mon Sep 17 00:00:00 2001
From: Brian Colonna <bcolonna@google.com>
Date: Wed, 16 May 2012 22:11:30 -0400
Subject: [PATCH] Not calling startUi() if no longer bound

After the bind to the FUL service is complete, an
onServiceConnected() callback is received.  This callback is
asynchronous - bindService() does not block while we are waiting for
the service to finish binding.  Therefore, when rapidly turning the
screen on and off, it is possible to call bindService() and then call
unbindService() before the onServiceConnected() callback is received.
When onServiceConnected() is received, startUi() is called.  If the
service is no longer bound, a runtime restart occurs when calling
startUi().

Note that onServiceConnected() actually has its work done via a
handler.  The delay of calling the handler increases the possibility
of unbindService() being called before trying to call startUi().  But
since this problem still happens without using the handler,
eliminating using the handler would not solve the problem and would
just create the problems that come with performing operations on
different threads since onServiceConnected() is not called on the main
thread.

Also note that a new instance of FaceUnlock is created in
LockPatternKeyguardView with each iteration.  So, if we bind/stop/bind
before getting onServiceConnected(), the second bind happens in a new
instance of FaceUnlock and therefore does not lead to a problem when
onServiceConnected() returns as a result of the first bind.

This fixes some occurrences of bug 6409767.  However, this fixes the
problem when turned the device on and off rapidly.  It seems there
are some reports of bug 6409767 where this is not the case, so I
can't be sure this has any affect on those cases.

This change also cleans up some debugging and modifies other
debugging to try to get just the information that is useful for
tracking down the bug.

Change-Id: Ifa59107b9974acaa8a18b74b5d47e4cf3a794b8e
---
 .../internal/policy/impl/FaceUnlock.java      | 32 +++++++++----------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/policy/src/com/android/internal/policy/impl/FaceUnlock.java b/policy/src/com/android/internal/policy/impl/FaceUnlock.java
index c46b94aab6e87..737ea47c4b4fd 100644
--- a/policy/src/com/android/internal/policy/impl/FaceUnlock.java
+++ b/policy/src/com/android/internal/policy/impl/FaceUnlock.java
@@ -300,7 +300,18 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * onServiceConnected() callback is received.
      */
     void handleServiceConnected() {
-        if (DEBUG) Log.d(TAG, "handleServiceConnected()");
+        Log.d(TAG, "handleServiceConnected()");
+
+        // It is possible that an unbind has occurred in the time between the bind and when this
+        // function is reached.  If an unbind has already occurred, proceeding on to call startUi()
+        // can result in a fatal error.  Note that the onServiceConnected() callback is
+        // asynchronous, so this possibility would still exist if we executed this directly in
+        // onServiceConnected() rather than using a handler.
+        if (!mBoundToService) {
+            Log.d(TAG, "Dropping startUi() in handleServiceConnected() because no longer bound");
+            return;
+        }
+
         try {
             mService.registerCallback(mFaceUnlockCallback);
         } catch (RemoteException e) {
@@ -452,25 +463,12 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
      * 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) {
-        Log.d(TAG, "startUi()");
+        if (DEBUG) Log.d(TAG, "startUi()");
         synchronized (mServiceRunningLock) {
             if (!mServiceRunning) {
-                if (DEBUG) Log.d(TAG, "Starting Face Unlock");
+                Log.d(TAG, "Starting Face Unlock");
                 try {
-                    // TODO: these checks and logs are for tracking down bug 6409767 and can be
-                    // removed when that bug is fixed.
-                    if (mService == null) {
-                        Log.d(TAG, "mService is null");
-                    }
-                    if (windowToken == null) {
-                        Log.d(TAG, "windowToken is null");
-                    }
-                    if (mLockPatternUtils == null) {
-                        Log.d(TAG, "mLockPatternUtils is null");
-                    }
-                    Log.d(TAG, "x,y,w,h,live: " + x + "," + y + "," + w + "," + h + ", no");
                     mService.startUi(windowToken, x, y, w, h, false);
-                    Log.d(TAG, "mService.startUi() called");
                 } catch (RemoteException e) {
                     Log.e(TAG, "Caught exception starting Face Unlock: " + e.toString());
                     return;
@@ -492,7 +490,7 @@ public class FaceUnlock implements BiometricSensorUnlock, Handler.Callback {
         // screen is turned off.  That's why we check.
         synchronized (mServiceRunningLock) {
             if (mServiceRunning) {
-                if (DEBUG) Log.d(TAG, "Stopping Face Unlock");
+                Log.d(TAG, "Stopping Face Unlock");
                 try {
                     mService.stopUi();
                 } catch (RemoteException e) {