From 69d13b1522cbeae5316cb38f6224cf0197f56b00 Mon Sep 17 00:00:00 2001
From: Rhed Jao <rhedjao@google.com>
Date: Tue, 2 Apr 2019 16:16:36 +0800
Subject: [PATCH] Fix unbind a11y service fail

Bug: 128422953
Test: all a11y cts & framework tests
Change-Id: I391c81b97f73805f375fd3ab36759dfc826847ec
---
 .../accessibility/AccessibilityManagerService.java   |  7 +++++++
 .../AccessibilityServiceConnection.java              | 12 ++++++++++--
 .../AccessibilityServiceConnectionTest.java          | 20 ++++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
index 47cd917bf84e..d7e56c3b0e0d 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
@@ -4085,6 +4085,13 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
             return mBindingServices;
         }
 
+        /**
+         * Returns enabled service list.
+         */
+        public Set<ComponentName> getEnabledServicesLocked() {
+            return mEnabledServices;
+        }
+
         public int getSoftKeyboardShowMode() {
             return mSoftKeyboardShowMode;
         }
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
index 3bd6220b4767..b66caa5e324a 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
@@ -123,12 +123,12 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect
         synchronized (mLock) {
             UserState userState = mUserStateWeakReference.get();
             if (userState == null) return;
-            if (userState.mEnabledServices.remove(mComponentName)) {
+            if (userState.getEnabledServicesLocked().remove(mComponentName)) {
                 final long identity = Binder.clearCallingIdentity();
                 try {
                     mSystemSupport.persistComponentNamesToSettingLocked(
                             Settings.Secure.ENABLED_ACCESSIBILITY_SERVICES,
-                            userState.mEnabledServices, userState.mUserId);
+                            userState.getEnabledServicesLocked(), userState.mUserId);
                 } finally {
                     Binder.restoreCallingIdentity(identity);
                 }
@@ -183,6 +183,14 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect
                 mWasConnectedAndDied = false;
                 serviceInterface = mServiceInterface;
             }
+            // There's a chance that service is removed from enabled_accessibility_services setting
+            // key, but skip unbinding because of it's in binding state. Unbinds it if it's
+            // not in enabled service list.
+            if (serviceInterface != null
+                    && !userState.getEnabledServicesLocked().contains(mComponentName)) {
+                mSystemSupport.onClientChangeLocked(false);
+                return;
+            }
         }
         if (serviceInterface == null) {
             binderDied();
diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityServiceConnectionTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityServiceConnectionTest.java
index 2ed25eab50c0..f3364974231b 100644
--- a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityServiceConnectionTest.java
+++ b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityServiceConnectionTest.java
@@ -23,10 +23,12 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import android.accessibilityservice.AccessibilityServiceInfo;
+import android.accessibilityservice.IAccessibilityServiceClient;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -46,6 +48,7 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 
 
@@ -149,4 +152,21 @@ public class AccessibilityServiceConnectionTest {
         assertTrue(mConnection.getServiceInfo().crashed);
         verify(mMockKeyEventDispatcher).flush(mConnection);
     }
+
+    @Test
+    public void connectedService_notInEnabledServiceList_doNotInitClient()
+            throws RemoteException {
+        IBinder mockBinder = mock(IBinder.class);
+        IAccessibilityServiceClient mockClient = mock(IAccessibilityServiceClient.class);
+        when(mockBinder.queryLocalInterface(any())).thenReturn(mockClient);
+        when(mMockUserState.getEnabledServicesLocked())
+                .thenReturn(Collections.emptySet());
+        setServiceBinding(COMPONENT_NAME);
+
+        mConnection.bindLocked();
+        mConnection.onServiceConnected(COMPONENT_NAME, mockBinder);
+        mHandler.sendAllMessages();
+        verify(mMockSystemSupport, times(2)).onClientChangeLocked(false);
+        verify(mockClient, times(0)).init(any(), anyInt(), any());
+    }
 }
-- 
2.11.0