From 008aa6cf07134ddde4cead0831eeca296269d360 Mon Sep 17 00:00:00 2001 From: sallyyuen Date: Mon, 12 Dec 2022 08:36:59 -0800 Subject: [PATCH] Filter a11y events by display id for AccessibilityDisplayProxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before dispatching the accessibility event, check if the event’s display id belongs to a proxy display. If so, only send the event to that proxy. Also, improve locking. Test: manual test with sample app. Added cts test in AccessibilityDisplayProxyTest Bug: 241429275 Change-Id: I12033ce6b394482d0d7f2b9eac3fd759f3f88851 --- .../AccessibilityDisplayProxy.java | 6 +--- .../AccessibilityManagerService.java | 15 ++++---- .../server/accessibility/ProxyManager.java | 35 ++++++++++++------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/core/java/android/view/accessibility/AccessibilityDisplayProxy.java b/core/java/android/view/accessibility/AccessibilityDisplayProxy.java index 44b6deb3015fc..a757236f92fda 100644 --- a/core/java/android/view/accessibility/AccessibilityDisplayProxy.java +++ b/core/java/android/view/accessibility/AccessibilityDisplayProxy.java @@ -242,11 +242,7 @@ public abstract class AccessibilityDisplayProxy { super(context, executor, new AccessibilityService.Callbacks() { @Override public void onAccessibilityEvent(AccessibilityEvent event) { - // TODO(254545943): Remove check when event processing is done more upstream in - // AccessibilityManagerService. - if (event.getDisplayId() == mDisplayId) { - AccessibilityDisplayProxy.this.onAccessibilityEvent(event); - } + AccessibilityDisplayProxy.this.onAccessibilityEvent(event); } @Override diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 3145139cc6bb8..4fbbd799a3db9 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -1018,10 +1018,13 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub } private void dispatchAccessibilityEventLocked(AccessibilityEvent event) { - notifyAccessibilityServicesDelayedLocked(event, false); - notifyAccessibilityServicesDelayedLocked(event, true); + if (mProxyManager.isProxyed(event.getDisplayId())) { + mProxyManager.sendAccessibilityEventLocked(event); + } else { + notifyAccessibilityServicesDelayedLocked(event, false); + notifyAccessibilityServicesDelayedLocked(event, true); + } mUiAutomationManager.sendAccessibilityEventLocked(event); - mProxyManager.sendAccessibilityEvent(event); } private void sendAccessibilityEventToInputFilter(AccessibilityEvent event) { @@ -1162,7 +1165,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub } List services = getUserStateLocked(resolvedUserId).mBoundServices; - int numServices = services.size() + mProxyManager.getNumProxys(); + int numServices = services.size() + mProxyManager.getNumProxysLocked(); interfacesToInterrupt = new ArrayList<>(numServices); for (int i = 0; i < services.size(); i++) { AccessibilityServiceConnection service = services.get(i); @@ -1172,7 +1175,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub interfacesToInterrupt.add(a11yServiceInterface); } } - mProxyManager.addServiceInterfaces(interfacesToInterrupt); + mProxyManager.addServiceInterfacesLocked(interfacesToInterrupt); } for (int i = 0, count = interfacesToInterrupt.size(); i < count; i++) { try { @@ -1963,7 +1966,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub mUiAutomationManager.getServiceInfo(), client) ? mUiAutomationManager.getRelevantEventTypes() : 0; - relevantEventTypes |= mProxyManager.getRelevantEventTypes(); + relevantEventTypes |= mProxyManager.getRelevantEventTypesLocked(); return relevantEventTypes; } diff --git a/services/accessibility/java/com/android/server/accessibility/ProxyManager.java b/services/accessibility/java/com/android/server/accessibility/ProxyManager.java index 2184878b4c362..f28191ff1bce1 100644 --- a/services/accessibility/java/com/android/server/accessibility/ProxyManager.java +++ b/services/accessibility/java/com/android/server/accessibility/ProxyManager.java @@ -37,6 +37,7 @@ import java.util.List; * proxy connection will belong to a separate user state. * * TODO(241117292): Remove or cut down during simultaneous user refactoring. + * TODO(262244375): Add unit tests. */ public class ProxyManager { // Names used to populate ComponentName and ResolveInfo in connection.mA11yServiceInfo and in @@ -84,7 +85,9 @@ public class ProxyManager { windowManagerInternal, awm, displayId); - mProxyA11yServiceConnections.put(displayId, connection); + synchronized (mLock) { + mProxyA11yServiceConnections.put(displayId, connection); + } // If the client dies, make sure to remove the connection. IBinder.DeathRecipient deathRecipient = @@ -98,7 +101,9 @@ public class ProxyManager { client.asBinder().linkToDeath(deathRecipient, 0); // Notify apps that the service state has changed. // A11yManager#A11yServicesStateChangeListener - connection.mSystemSupport.onClientChangeLocked(true); + synchronized (mLock) { + connection.mSystemSupport.onClientChangeLocked(true); + } connection.initializeServiceInterface(client); } @@ -111,9 +116,11 @@ public class ProxyManager { } private boolean clearConnection(int displayId) { - if (mProxyA11yServiceConnections.contains(displayId)) { - mProxyA11yServiceConnections.remove(displayId); - return true; + synchronized (mLock) { + if (mProxyA11yServiceConnections.contains(displayId)) { + mProxyA11yServiceConnections.remove(displayId); + return true; + } } return false; } @@ -122,7 +129,9 @@ public class ProxyManager { * Checks if a display id is being proxy-ed. */ public boolean isProxyed(int displayId) { - return mProxyA11yServiceConnections.contains(displayId); + synchronized (mLock) { + return mProxyA11yServiceConnections.contains(displayId); + } } /** @@ -130,10 +139,10 @@ public class ProxyManager { * {@link android.view.accessibility.AccessibilityDisplayProxy} will filter based on display. * TODO(b/250929565): Filtering should happen in the system, not in the proxy. */ - public void sendAccessibilityEvent(AccessibilityEvent event) { - for (int i = 0; i < mProxyA11yServiceConnections.size(); i++) { - ProxyAccessibilityServiceConnection proxy = - mProxyA11yServiceConnections.valueAt(i); + public void sendAccessibilityEventLocked(AccessibilityEvent event) { + final ProxyAccessibilityServiceConnection proxy = + mProxyA11yServiceConnections.get(event.getDisplayId()); + if (proxy != null) { proxy.notifyAccessibilityEvent(event); } } @@ -187,7 +196,7 @@ public class ProxyManager { * Returns the relevant event types of every proxy. * TODO(254545943): When A11yManager is separated, return based on the A11yManager display. */ - public int getRelevantEventTypes() { + public int getRelevantEventTypesLocked() { int relevantEventTypes = 0; for (int i = 0; i < mProxyA11yServiceConnections.size(); i++) { ProxyAccessibilityServiceConnection proxy = @@ -201,7 +210,7 @@ public class ProxyManager { * Gets the number of current proxy connections. * @return */ - public int getNumProxys() { + public int getNumProxysLocked() { return mProxyA11yServiceConnections.size(); } @@ -209,7 +218,7 @@ public class ProxyManager { * Adds the service interfaces to a list. * @param interfaces */ - public void addServiceInterfaces(List interfaces) { + public void addServiceInterfacesLocked(List interfaces) { for (int i = 0; i < mProxyA11yServiceConnections.size(); i++) { final ProxyAccessibilityServiceConnection proxy = mProxyA11yServiceConnections.valueAt(i);