From a8cfb3733d7022d3a4be87f15701dbac7a17045c Mon Sep 17 00:00:00 2001
From: Phil Weaver <pweaver@google.com>
Date: Fri, 29 Jul 2016 17:26:41 -0700
Subject: Improve handling of crashing a11y services.

We were confusing handling of services that were unbound with
those that had crashed. We would lose track of services that
has crashed, start new ones, and then when the system restarted
a killed services we would have multiple instances running. It
was possible for this to get very out of hand.

Bug: 30306689
Change-Id: I4e63d25b6d2fec3ec68f450a4602898c43a2b2ad
---
 .../accessibility/AccessibilityManagerService.java | 35 ++++++++++++++--------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
index 7ebc150bb30b..4dab0b1374f1 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
@@ -1111,9 +1111,11 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
 
     private void addServiceLocked(Service service, UserState userState) {
         try {
-            service.onAdded();
-            userState.mBoundServices.add(service);
-            userState.mComponentNameToServiceMap.put(service.mComponentName, service);
+            if (!userState.mBoundServices.contains(service)) {
+                service.onAdded();
+                userState.mBoundServices.add(service);
+                userState.mComponentNameToServiceMap.put(service.mComponentName, service);
+            }
         } catch (RemoteException re) {
             /* do nothing */
         }
@@ -1126,8 +1128,14 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
      */
     private void removeServiceLocked(Service service, UserState userState) {
         userState.mBoundServices.remove(service);
-        userState.mComponentNameToServiceMap.remove(service.mComponentName);
         service.onRemoved();
+        // It may be possible to bind a service twice, which confuses the map. Rebuild the map
+        // to make sure we can still reach a service
+        userState.mComponentNameToServiceMap.clear();
+        for (int i = 0; i < userState.mBoundServices.size(); i++) {
+            Service boundService = userState.mBoundServices.get(i);
+            userState.mComponentNameToServiceMap.put(boundService.mComponentName, boundService);
+        }
     }
 
     /**
@@ -2324,15 +2332,12 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
         }
 
         /**
-         * Unbinds form the accessibility service and removes it from the data
+         * Unbinds from the accessibility service and removes it from the data
          * structures for service management.
          *
          * @return True if unbinding is successful.
          */
         public boolean unbindLocked() {
-            if (mService == null) {
-                return false;
-            }
             UserState userState = getUserStateLocked(mUserId);
             getKeyEventDispatcher().flush(this);
             if (!mIsAutomation) {
@@ -3033,7 +3038,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
 
         @Override
         public void onServiceDisconnected(ComponentName componentName) {
-            /* do nothing - #binderDied takes care */
+            binderDied();
         }
 
         public void onAdded() throws RemoteException {
@@ -3062,14 +3067,18 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
         }
 
         public void unlinkToOwnDeathLocked() {
-            mService.unlinkToDeath(this, 0);
+            if (mService != null) {
+                mService.unlinkToDeath(this, 0);
+            }
         }
 
         public void resetLocked() {
             try {
                 // Clear the proxy in the other process so this
                 // IAccessibilityServiceConnection can be garbage collected.
-                mServiceInterface.init(null, mId, null);
+                if (mServiceInterface != null) {
+                    mServiceInterface.init(null, mId, null);
+                }
             } catch (RemoteException re) {
                 /* ignore */
             }
@@ -3093,10 +3102,10 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
                 mWasConnectedAndDied = true;
                 getKeyEventDispatcher().flush(this);
                 UserState userState = getUserStateLocked(mUserId);
-                // The death recipient is unregistered in removeServiceLocked
-                removeServiceLocked(this, userState);
                 resetLocked();
                 if (mIsAutomation) {
+                    // This is typically done when unbinding, but UiAutomation isn't bound.
+                    removeServiceLocked(this, userState);
                     // We no longer have an automation service, so restore
                     // the state based on values in the settings database.
                     userState.mInstalledServices.remove(mAccessibilityServiceInfo);
-- 
cgit v1.2.3-59-g8ed1b