Pie controls: Catching activation corner cases

As stated before there are still corner cases that may
deadlock pie controls.

This commit makes PieInputFilter respect ACTION_CANCEL,
simplifies monitoring mechanics in PieService and
fixes some corner cases in PieController that allowed that
activations got lost and thus pie controls got stuck.

Test case for PieController:
1) Enable pie controls
2) Enable top/bottom snap position for pie
2) Enable expanded desktop and check if it works
3) Disable expanded desktop
4) Lock your phone
5) Swipe from the top down/bottom up, as if you want to
   activate the pie from that position
6) Dismiss the notification tray and unlock your phone
   again
7) Enable extpanded desktop
8) Pie controls will no longer show up and
   "dumpsys pieservice" will show a active record.

Change-Id: Id9d2df6b2735a743eec2fd1ba809fd72be9474cc

2 files changed, 105 insertions, 114 deletions

diff --git a/services/java/com/android/server/pie/PieInputFilter.java b/services/java/com/android/server/pie/PieInputFilter.java
index 06b630b..999cf72 100644
--- a/services/java/com/android/server/pie/PieInputFilter.java
+++ b/services/java/com/android/server/pie/PieInputFilter.java
@@ -66,7 +66,7 @@ import java.io.PrintWriter;
  * 5) POSTSYNTHESIZE:
  *    mSyntheticDownTime != -1
  *    All following events will have the down time set to the synthesized ACTION_DOWN event time
- *    until an ACTION_UP is encountered and the state is reset to LISTEN.
+ *    until an ACTION_UP or ACTION_CANCEL is encountered and the state is reset to LISTEN.
  * <p>
  * If you are reading this within Java Doc, you are doing something wrong ;)
  */
@@ -81,7 +81,7 @@ public class PieInputFilter implements IInputFilter {
     private static final boolean DEBUG = false;
     private static final boolean DEBUG_INPUT = false;
     // TODO: Should be turned off in final commit
-    private static final boolean SYSTRACE = true;
+    private static final boolean SYSTRACE = false;
 
     private final Handler mHandler;
 
@@ -174,6 +174,7 @@ public class PieInputFilter implements IInputFilter {
                 res.getDimensionPixelSize(R.dimen.pie_perpendicular_distance));
         mTracker.setOnActivationListener(new OnActivationListener() {
             public void onActivation(MotionEvent event, int touchX, int touchY, PiePosition position) {
+                // mLock is held by #processMotionEvent
                 mHandler.obtainMessage(PieService.MSG_PIE_ACTIVATION,
                         touchX, touchY, position).sendToTarget();
                 mState = State.LOCKED;
@@ -307,7 +308,7 @@ public class PieInputFilter implements IInputFilter {
                 case SYNTHESIZE:
                     if (action == MotionEvent.ACTION_MOVE) {
                         clearDelayedMotionEventsLocked();
-                        sendSynthesizedMotionEvent(motionEvent, policyFlags);
+                        sendSynthesizedMotionEventLocked(motionEvent, policyFlags);
                         mState = State.POSTSYNTHESIZE;
                     } else {
                         // This is the case where a race condition caught us: We already
@@ -319,7 +320,7 @@ public class PieInputFilter implements IInputFilter {
                     break;
                 case POSTSYNTHESIZE:
                     motionEvent.setDownTime(mSyntheticDownTime);
-                    if (action == MotionEvent.ACTION_UP) {
+                    if (action == MotionEvent.ACTION_UP || action == MotionEvent.ACTION_CANCEL) {
                         mState = State.LISTEN;
                         mSyntheticDownTime = -1;
                     }
@@ -396,7 +397,7 @@ public class PieInputFilter implements IInputFilter {
                 if (info.event.getActionMasked() == MotionEvent.ACTION_DOWN) {
                     mSyntheticDownTime = info.event.getDownTime() + offset;
                 }
-                sendMotionEventWithOffset(info.event, info.policyFlags, mSyntheticDownTime, offset);
+                sendMotionEventWithOffsetLocked(info.event, info.policyFlags, mSyntheticDownTime, offset);
                 if (info.event.getActionMasked() == MotionEvent.ACTION_UP) {
                     mSyntheticDownTime = -1;
                 }
@@ -421,11 +422,11 @@ public class PieInputFilter implements IInputFilter {
         }
     }
 
-    private void sendMotionEventWithOffset(MotionEvent event, int policyFlags,
+    private void sendMotionEventWithOffsetLocked(MotionEvent event, int policyFlags,
             long downTime, long offset) {
         final int pointerCount = event.getPointerCount();
-        PointerCoords[] coords = getTempPointerCoordsWithMinSize(pointerCount);
-        PointerProperties[] properties = getTempPointerPropertiesWithMinSize(pointerCount);
+        PointerCoords[] coords = getTempPointerCoordsWithMinSizeLocked(pointerCount);
+        PointerProperties[] properties = getTempPointerPropertiesWithMinSizeLocked(pointerCount);
         for (int i = 0; i < pointerCount; i++) {
             event.getPointerCoords(i, coords[i]);
             event.getPointerProperties(i, properties[i]);
@@ -437,7 +438,7 @@ public class PieInputFilter implements IInputFilter {
                 policyFlags);
     }
 
-    private PointerCoords[] getTempPointerCoordsWithMinSize(int size) {
+    private PointerCoords[] getTempPointerCoordsWithMinSizeLocked(int size) {
         final int oldSize = mTempPointerCoords.length;
         if (oldSize < size) {
             PointerCoords[] oldTempPointerCoords = mTempPointerCoords;
@@ -450,7 +451,7 @@ public class PieInputFilter implements IInputFilter {
         return mTempPointerCoords;
     }
 
-    private PointerProperties[] getTempPointerPropertiesWithMinSize(int size) {
+    private PointerProperties[] getTempPointerPropertiesWithMinSizeLocked(int size) {
         final int oldSize = mTempPointerProperties.length;
         if (oldSize < size) {
             PointerProperties[] oldTempPointerProperties = mTempPointerProperties;
@@ -463,7 +464,7 @@ public class PieInputFilter implements IInputFilter {
         return mTempPointerProperties;
     }
 
-    private void sendSynthesizedMotionEvent(MotionEvent event, int policyFlags) {
+    private void sendSynthesizedMotionEventLocked(MotionEvent event, int policyFlags) {
         if (event.getPointerCount() == 1) {
             event.getPointerCoords(0, mTempPointerCoords[0]);
             event.getPointerProperties(0, mTempPointerProperties[0]);
diff --git a/services/java/com/android/server/pie/PieService.java b/services/java/com/android/server/pie/PieService.java
index 937c13b..cc945c6 100644
--- a/services/java/com/android/server/pie/PieService.java
+++ b/services/java/com/android/server/pie/PieService.java
@@ -40,7 +40,6 @@ import android.service.pie.IPieService;
 import android.util.Slog;
 import android.view.Display;
 import android.view.DisplayInfo;
-import android.view.View;
 import android.view.WindowManager;
 
 import com.android.internal.util.pie.PiePosition;
@@ -69,18 +68,16 @@ public class PieService extends IPieService.Stub {
 
     private final Context mContext;
     private final InputManagerService mInputManager;
-    private final WindowManagerService mWindowManager;
 
-    private HandlerThread mHandlerThread = new HandlerThread("Pie");
+    private final HandlerThread mHandlerThread = new HandlerThread("Pie");
+    private Handler mHandler;
 
-    // Lock for thread, handler, mInputFilter, activations and listener related variables
+    // Lock for mInputFilter, activations and listener related variables
     private final Object mLock = new Object();
-    private Handler mHandler;
     private PieInputFilter mInputFilter;
 
     private int mGlobalPositions = 0;
     private int mGlobalSensitivity = 3;
-    private boolean mIsMonitoring = false;
 
     private final class PieActivationListenerRecord extends IPieHostCallback.Stub implements DeathRecipient {
         private boolean mActive;
@@ -94,16 +91,16 @@ public class PieService extends IPieService.Stub {
             removeListenerRecord(this);
         }
 
-        public void updateFlags(int flags) {
+        private void updateFlags(int flags) {
             this.positions = flags & POSITION_MASK;
             this.sensitivity = (flags & SENSITIVITY_MASK) >> SENSITIVITY_SHIFT;
         }
 
-        public boolean eligibleForActivation(int positionFlag) {
+        private boolean eligibleForActivation(int positionFlag) {
             return (positions & positionFlag) != 0;
         }
 
-        public boolean notifyPieActivation(int touchX, int touchY, PiePosition position) {
+        private boolean notifyPieActivation(int touchX, int touchY, PiePosition position) {
             if ((positions & position.FLAG) != 0) {
                 try {
                     listener.onPieActivation(touchX, touchY, position.INDEX, 0);
@@ -167,7 +164,6 @@ public class PieService extends IPieService.Stub {
     public PieService(Context context, WindowManagerService windowManager, InputManagerService inputManager) {
         mContext = context;
         mInputManager = inputManager;
-        mWindowManager = windowManager;
     }
 
     // called by system server
@@ -186,19 +182,41 @@ public class PieService extends IPieService.Stub {
         });
         mDisplayObserver = new DisplayObserver(mContext, mHandler);
         // check if anyone registered during startup
+        updateMonitoring();
         mHandler.obtainMessage(MSG_UPDATE_SERVICE,
                 mGlobalPositions, mGlobalSensitivity).sendToTarget();
-        updateMonitoring();
+    }
+
+
+    private void updateMonitoring() {
+        synchronized(mLock) {
+            mGlobalPositions = 0;
+            mGlobalSensitivity = SENSITIVITY_NONE;
+            for (PieActivationListenerRecord temp : mPieActivationListener) {
+                mGlobalPositions |= temp.positions;
+                if (temp.sensitivity != SENSITIVITY_NONE) {
+                    mGlobalSensitivity = Math.max(mGlobalSensitivity, temp.sensitivity);
+                }
+            }
+            // if no special sensitivity is requested, we settle on DEFAULT
+            if (mGlobalSensitivity == SENSITIVITY_NONE) {
+                mGlobalSensitivity = SENSITIVITY_DEFAULT;
+            }
+
+            if (mInputFilter == null && mGlobalPositions != 0) {
+                enforceMonitoringLocked();
+            } else if (mInputFilter != null && mGlobalPositions == 0) {
+                shutdownMonitoringLocked();
+            }
+        }
     }
 
     private void enforceMonitoringLocked() {
         if (DEBUG) {
             Slog.d(TAG, "Attempting to start monitoring input events ...");
         }
-        if (mInputFilter == null) {
-            mInputFilter = new PieInputFilter(mContext, mHandler);
-            mInputManager.registerSecondaryInputFilter(mInputFilter);
-        }
+        mInputFilter = new PieInputFilter(mContext, mHandler);
+        mInputManager.registerSecondaryInputFilter(mInputFilter);
         mDisplayObserver.observe();
     }
 
@@ -207,21 +225,8 @@ public class PieService extends IPieService.Stub {
             Slog.d(TAG, "Shutting down monitoring input events ...");
         }
         mDisplayObserver.unobserve();
-        if (mInputFilter != null) {
-            mInputManager.unregisterSecondaryInputFilter(mInputFilter);
-            mInputFilter = null;
-        }
-    }
-
-    private void updateMonitoring() {
-        synchronized(mLock) {
-            if (!mIsMonitoring && mGlobalPositions != 0) {
-                enforceMonitoringLocked();
-            } else if (mIsMonitoring && mGlobalPositions == 0) {
-                shutdownMonitoringLocked();
-            }
-            mIsMonitoring = mGlobalPositions != 0;
-        }
+        mInputManager.unregisterSecondaryInputFilter(mInputFilter);
+        mInputFilter = null;
     }
 
     // called through Binder
@@ -242,6 +247,12 @@ public class PieService extends IPieService.Stub {
             record = findListenerRecordLocked(listener.asBinder());
             if (record == null) {
                 record = new PieActivationListenerRecord(listener);
+                try {
+                    listener.asBinder().linkToDeath(record, 0);
+                } catch (RemoteException e) {
+                    Slog.w(TAG, "Recipient died during registration pid=" + Binder.getCallingPid());
+                    return null;
+                }
                 mPieActivationListener.add(record);
             }
         }
@@ -260,14 +271,13 @@ public class PieService extends IPieService.Stub {
                 throw new IllegalStateException("listener not registered");
             }
             record.updateFlags(positionFlags);
-            updatePositionsLocked();
-            updateSensitivityLocked();
+            updateMonitoring();
+            // update input filter only when not being active and #systemReady() was called
             if (mActiveRecord == null && mHandler != null) {
                 mHandler.obtainMessage(MSG_UPDATE_SERVICE,
                         mGlobalPositions, mGlobalSensitivity).sendToTarget();
             }
         }
-        updateMonitoring();
     }
 
     private PieActivationListenerRecord findListenerRecordLocked(IBinder listener) {
@@ -279,38 +289,18 @@ public class PieService extends IPieService.Stub {
         return null;
     }
 
-    private void updatePositionsLocked() {
-        mGlobalPositions = 0;
-        for (PieActivationListenerRecord temp : mPieActivationListener) {
-            mGlobalPositions |= temp.positions;
-        }
-    }
-
-    private void updateSensitivityLocked() {
-        mGlobalSensitivity = SENSITIVITY_NONE;
-        for (PieActivationListenerRecord temp : mPieActivationListener) {
-            if (temp.sensitivity != SENSITIVITY_NONE) {
-                mGlobalSensitivity = Math.max(mGlobalSensitivity, temp.sensitivity);
-            }
-        }
-        // if no special sensitivity is requested, we settle on DEFAULT
-        if (mGlobalSensitivity == SENSITIVITY_NONE) {
-            mGlobalSensitivity = SENSITIVITY_DEFAULT;
-        }
-    }
-
     private void removeListenerRecord(PieActivationListenerRecord record) {
         synchronized(mLock) {
             mPieActivationListener.remove(record);
-            updatePositionsLocked();
+            updateMonitoring();
             // check if the record was the active one
             if (record == mActiveRecord) {
                 // restore input filter state by updating
                 mHandler.obtainMessage(MSG_UPDATE_SERVICE,
                         mGlobalPositions, mGlobalSensitivity).sendToTarget();
+                mActiveRecord = null;
             }
         }
-        updateMonitoring();
     }
 
     // called by handler thread
@@ -333,54 +323,7 @@ public class PieService extends IPieService.Stub {
             if (target != null && target.notifyPieActivation(touchX, touchY, position)) {
                 mActiveRecord = target;
             }
-        }
-        return mActiveRecord != null;
-    }
-
-    @Override
-    public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
-            throws RemoteException {
-        try {
-            return super.onTransact(code, data, reply, flags);
-        } catch (RuntimeException e) {
-            // let's log all exceptions we do not know about.
-            if (!(e instanceof IllegalArgumentException || e instanceof IllegalStateException)) {
-                Slog.e(TAG, "PieService crashed: ", e);
-            }
-            throw e;
-        }
-    }
-
-    @Override
-    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
-        if (mContext.checkCallingOrSelfPermission(Manifest.permission.DUMP)
-                != PackageManager.PERMISSION_GRANTED) {
-            pw.println("Permission Denial: can't dump PieService from from pid="
-                    + Binder.getCallingPid()
-                    + ", uid=" + Binder.getCallingUid());
-            return;
-        }
-
-        pw.println("PIE SERVICE (dumpsys pieservice)\n");
-        synchronized(mLock) {
-            pw.println("  mIsMonitoring=" + mIsMonitoring);
-            pw.println("  mInputFilter=" + mInputFilter);
-            if (mInputFilter != null) {
-                mInputFilter.dump(pw, "    ");
-            }
-            pw.println("  mGlobalPositions=0x" + Integer.toHexString(mGlobalPositions));
-            int i = 0;
-            for (PieActivationListenerRecord record : mPieActivationListener) {
-                if (record == mActiveRecord) break;
-                i++;
-            }
-            pw.println("  mActiveRecord=" + (mActiveRecord != null ? ("#" + i) : "null"));
-            i = 0;
-            for (PieActivationListenerRecord record : mPieActivationListener) {
-                pw.println("  Listener #" + i + ":");
-                record.dump(pw, "    ");
-                i++;
-            }
+            return mActiveRecord != null;
         }
     }
 
@@ -481,4 +424,51 @@ public class PieService extends IPieService.Stub {
             updateDisplayInfo();
         }
     }
+
+    @Override
+    public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
+            throws RemoteException {
+        try {
+            return super.onTransact(code, data, reply, flags);
+        } catch (RuntimeException e) {
+            // let's log all exceptions we do not know about.
+            if (!(e instanceof IllegalArgumentException || e instanceof IllegalStateException)) {
+                Slog.e(TAG, "PieService crashed: ", e);
+            }
+            throw e;
+        }
+    }
+
+    @Override
+    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
+        if (mContext.checkCallingOrSelfPermission(Manifest.permission.DUMP)
+                != PackageManager.PERMISSION_GRANTED) {
+            pw.println("Permission Denial: can't dump PieService from from pid="
+                    + Binder.getCallingPid()
+                    + ", uid=" + Binder.getCallingUid());
+            return;
+        }
+
+        pw.println("PIE SERVICE (dumpsys pieservice)\n");
+        synchronized(mLock) {
+            pw.println("  mInputFilter=" + mInputFilter);
+            if (mInputFilter != null) {
+                mInputFilter.dump(pw, "    ");
+            }
+            pw.println("  mGlobalPositions=0x" + Integer.toHexString(mGlobalPositions));
+            pw.println("  mGlobalSensitivity=" + mGlobalSensitivity);
+            int i = 0;
+            for (PieActivationListenerRecord record : mPieActivationListener) {
+                if (record == mActiveRecord) break;
+                i++;
+            }
+            pw.println("  mActiveRecord=" + (mActiveRecord != null ? ("#" + i) : "null"));
+            i = 0;
+            for (PieActivationListenerRecord record : mPieActivationListener) {
+                pw.println("  Listener #" + i + ":");
+                record.dump(pw, "    ");
+                i++;
+            }
+        }
+    }
 }