From 501fb28601a289a7a57a0cdd09e80d879d7d4685 Mon Sep 17 00:00:00 2001 From: sallyyuen Date: Wed, 14 Dec 2022 07:55:59 -0800 Subject: [PATCH] Ensure that windows are separated between the A11yDisplayProxy user and the a11y service user. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AbstractA11yServiceConnection holds a flag to determine if a connection should get proxy windows or non-proxy windows. A11yServiceConnection (A11yServices) should get non-proxy windows, UIAutomationService should get both proxy and non-proxy windows, and ProxyA11yServiceConnection (proxies) should get proxy windows. We need to prevent the windows from being stored in the cache, so filter the windows before any A11yInteractionClient processing. Future additional work to for service and proxy window separation: - Clear all caches upon proxy registration. This should clear out any cached data relating to the proxy display in services. - When adding findFocus to A11yDisplayProxy, make sure the proxy and service cannot retrieve each other’s nodes (which gives access to windows). Test: atest AccessibilityDisplayProxyTest (added a test to check an AccessibilityService does not get proxy windows), manual check of windows in sample app Bug: 241429275 Change-Id: I73e3be1e3ad52ebacfc813bcfd6dff0bb7b541e8 --- ...bstractAccessibilityServiceConnection.java | 25 +++++++++++- .../AccessibilityManagerService.java | 2 +- .../AccessibilityWindowManager.java | 40 ++++++++++++++++++- .../ProxyAccessibilityServiceConnection.java | 14 +++++++ .../server/accessibility/ProxyManager.java | 8 +++- .../accessibility/UiAutomationManager.java | 1 + ...actAccessibilityServiceConnectionTest.java | 2 +- .../AccessibilityWindowManagerTest.java | 4 +- 8 files changed, 89 insertions(+), 7 deletions(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java index e92150b895b03..77855f4efee99 100644 --- a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java +++ b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java @@ -42,6 +42,7 @@ import android.accessibilityservice.AccessibilityTrace; import android.accessibilityservice.IAccessibilityServiceClient; import android.accessibilityservice.IAccessibilityServiceConnection; import android.accessibilityservice.MagnificationConfig; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.PendingIntent; @@ -107,6 +108,8 @@ import com.android.server.wm.WindowManagerInternal; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -129,6 +132,11 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ private static final String TRACE_WM = "WindowManagerInternal"; private static final int WAIT_WINDOWS_TIMEOUT_MILLIS = 5000; + /** Display type for displays associated with the default user of th device. */ + public static final int DISPLAY_TYPE_DEFAULT = 1 << 0; + /** Display type for displays associated with an AccessibilityDisplayProxy user. */ + public static final int DISPLAY_TYPE_PROXY = 1 << 1; + protected static final String TAKE_SCREENSHOT = "takeScreenshot"; protected final Context mContext; protected final SystemSupport mSystemSupport; @@ -157,6 +165,8 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ // The attribution tag set by the service that is bound to this instance protected String mAttributionTag; + protected int mDisplayTypes = DISPLAY_TYPE_DEFAULT; + // The service that's bound to this instance. Whenever this value is non-null, this // object is registered as a death recipient IBinder mService; @@ -224,6 +234,14 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ */ private SparseArray mRequestTakeScreenshotOfWindowTimestampMs = new SparseArray<>(); + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(flag = true, prefix = { "DISPLAY_TYPE_" }, value = { + DISPLAY_TYPE_DEFAULT, + DISPLAY_TYPE_PROXY + }) + public @interface DisplayTypes {} + public interface SystemSupport { /** * @return The current dispatcher for key events @@ -520,7 +538,8 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ } final AccessibilityWindowInfo.WindowListSparseArray allWindows = new AccessibilityWindowInfo.WindowListSparseArray(); - final ArrayList displayList = mA11yWindowManager.getDisplayListLocked(); + final ArrayList displayList = mA11yWindowManager.getDisplayListLocked( + mDisplayTypes); final int displayListCounts = displayList.size(); if (displayListCounts > 0) { for (int i = 0; i < displayListCounts; i++) { @@ -538,6 +557,10 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ } } + protected void setDisplayTypes(@DisplayTypes int displayTypes) { + mDisplayTypes = displayTypes; + } + @Override public AccessibilityWindowInfo getWindow(int windowId) { if (svcConnTracingEnabled()) { diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 4fbbd799a3db9..b496ba1233a4a 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -454,7 +454,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub new MagnificationScaleProvider(mContext)); mMagnificationProcessor = new MagnificationProcessor(mMagnificationController); mCaptioningManagerImpl = new CaptioningManagerImpl(mContext); - mProxyManager = new ProxyManager(mLock); + mProxyManager = new ProxyManager(mLock, mA11yWindowManager); init(); } diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java index 7c68c8a5bdb1b..8af5e111d8bf2 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java @@ -21,6 +21,8 @@ import static android.accessibilityservice.AccessibilityTrace.FLAGS_WINDOW_MANAG import static android.view.WindowManager.LayoutParams.TYPE_ACCESSIBILITY_OVERLAY; import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage; +import static com.android.server.accessibility.AbstractAccessibilityServiceConnection.DISPLAY_TYPE_DEFAULT; +import static com.android.server.accessibility.AbstractAccessibilityServiceConnection.DISPLAY_TYPE_PROXY; import android.annotation.NonNull; import android.annotation.Nullable; @@ -169,6 +171,7 @@ public class AccessibilityWindowManager { private List mWindows; private boolean mTrackingWindows = false; private boolean mHasWatchOutsideTouchWindow; + private boolean mIsProxy; /** * Constructor for DisplayWindowsObserver. @@ -1015,6 +1018,33 @@ public class AccessibilityWindowManager { } } + /** + * Starts tracking a display as belonging to a proxy. Creates the window observer if necessary. + * @param displayId + */ + public void startTrackingDisplayProxy(int displayId) { + startTrackingWindows(displayId); + synchronized (mLock) { + DisplayWindowsObserver observer = mDisplayWindowsObservers.get(displayId); + if (observer != null) { + observer.mIsProxy = true; + } + } + } + + /** + * Stops tracking a display as belonging to a proxy. + * @param displayId + */ + public void stopTrackingDisplayProxy(int displayId) { + synchronized (mLock) { + DisplayWindowsObserver observer = mDisplayWindowsObservers.get(displayId); + if (observer != null) { + observer.mIsProxy = false; + } + } + } + /** * Checks if we are tracking windows on any display. * @@ -1728,15 +1758,21 @@ public class AccessibilityWindowManager { /** * Returns the display list including all displays which are tracking windows. * + * @param displayTypes the types of displays to retrieve * @return The display list. */ - public ArrayList getDisplayListLocked() { + public ArrayList getDisplayListLocked( + @AbstractAccessibilityServiceConnection.DisplayTypes int displayTypes) { final ArrayList displayList = new ArrayList<>(); final int count = mDisplayWindowsObservers.size(); for (int i = 0; i < count; i++) { final DisplayWindowsObserver observer = mDisplayWindowsObservers.valueAt(i); if (observer != null) { - displayList.add(observer.mDisplayId); + if (!observer.mIsProxy && (displayTypes & DISPLAY_TYPE_DEFAULT) != 0) { + displayList.add(observer.mDisplayId); + } else if (observer.mIsProxy && (displayTypes & DISPLAY_TYPE_PROXY) != 0) { + displayList.add(observer.mDisplayId); + } } } return displayList; diff --git a/services/accessibility/java/com/android/server/accessibility/ProxyAccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/ProxyAccessibilityServiceConnection.java index d7f9c12b78850..d53a080fd5859 100644 --- a/services/accessibility/java/com/android/server/accessibility/ProxyAccessibilityServiceConnection.java +++ b/services/accessibility/java/com/android/server/accessibility/ProxyAccessibilityServiceConnection.java @@ -39,12 +39,14 @@ import android.os.RemoteException; import android.view.KeyEvent; import android.view.accessibility.AccessibilityDisplayProxy; import android.view.accessibility.AccessibilityNodeInfo; +import android.view.accessibility.AccessibilityWindowInfo; import androidx.annotation.Nullable; import com.android.server.wm.WindowManagerInternal; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -74,6 +76,7 @@ public class ProxyAccessibilityServiceConnection extends AccessibilityServiceCon mainHandler, lock, securityPolicy, systemSupport, trace, windowManagerInternal, /* systemActionPerformer= */ null, awm, /* activityTaskManagerService= */ null); mDisplayId = displayId; + setDisplayTypes(DISPLAY_TYPE_PROXY); } /** @@ -188,6 +191,17 @@ public class ProxyAccessibilityServiceConnection extends AccessibilityServiceCon } } + @Override + public AccessibilityWindowInfo.WindowListSparseArray getWindows() { + final AccessibilityWindowInfo.WindowListSparseArray allWindows = super.getWindows(); + AccessibilityWindowInfo.WindowListSparseArray displayWindows = new + AccessibilityWindowInfo.WindowListSparseArray(); + // Filter here so A11yInteractionClient will not cache all the windows belonging to other + // proxy connections. + displayWindows.put(mDisplayId, allWindows.get(mDisplayId, Collections.emptyList())); + return displayWindows; + } + @Override public void binderDied() { } diff --git a/services/accessibility/java/com/android/server/accessibility/ProxyManager.java b/services/accessibility/java/com/android/server/accessibility/ProxyManager.java index f28191ff1bce1..85273586e5dcb 100644 --- a/services/accessibility/java/com/android/server/accessibility/ProxyManager.java +++ b/services/accessibility/java/com/android/server/accessibility/ProxyManager.java @@ -55,8 +55,11 @@ public class ProxyManager { private SparseArray mProxyA11yServiceConnections = new SparseArray<>(); - ProxyManager(Object lock) { + private AccessibilityWindowManager mA11yWindowManager; + + ProxyManager(Object lock, AccessibilityWindowManager awm) { mLock = lock; + mA11yWindowManager = awm; } /** @@ -99,6 +102,8 @@ public class ProxyManager { } }; client.asBinder().linkToDeath(deathRecipient, 0); + + mA11yWindowManager.startTrackingDisplayProxy(displayId); // Notify apps that the service state has changed. // A11yManager#A11yServicesStateChangeListener synchronized (mLock) { @@ -122,6 +127,7 @@ public class ProxyManager { return true; } } + mA11yWindowManager.stopTrackingDisplayProxy(displayId); return false; } diff --git a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java index 6cc22143d6d6f..2188b99a3d839 100644 --- a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java +++ b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java @@ -253,6 +253,7 @@ class UiAutomationManager { securityPolicy, systemSupport, trace, windowManagerInternal, systemActionPerformer, awm); mMainHandler = mainHandler; + setDisplayTypes(DISPLAY_TYPE_DEFAULT | DISPLAY_TYPE_PROXY); } void connectServiceUnknownThread() { diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java index 57f577708ed5d..e168596b8eb2c 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java @@ -207,7 +207,7 @@ public class AbstractAccessibilityServiceConnectionTest { addA11yWindowInfo(mA11yWindowInfos, PIP_WINDOWID, true, Display.DEFAULT_DISPLAY); addA11yWindowInfo(mA11yWindowInfosOnSecondDisplay, WINDOWID_ONSECONDDISPLAY, false, SECONDARY_DISPLAY_ID); - when(mMockA11yWindowManager.getDisplayListLocked()).thenReturn(mDisplayList); + when(mMockA11yWindowManager.getDisplayListLocked(anyInt())).thenReturn(mDisplayList); when(mMockA11yWindowManager.getWindowListLocked(Display.DEFAULT_DISPLAY)) .thenReturn(mA11yWindowInfos); when(mMockA11yWindowManager.findA11yWindowInfoByIdLocked(WINDOWID)) diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityWindowManagerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityWindowManagerTest.java index acbcad54626db..e66a1d4ea679f 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityWindowManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityWindowManagerTest.java @@ -16,6 +16,7 @@ package com.android.server.accessibility; +import static com.android.server.accessibility.AbstractAccessibilityServiceConnection.DISPLAY_TYPE_DEFAULT; import static com.android.server.accessibility.AccessibilityWindowManagerTest.DisplayIdMatcher.displayId; import static com.android.server.accessibility.AccessibilityWindowManagerTest.WindowChangesMatcher.a11yWindowChanges; import static com.android.server.accessibility.AccessibilityWindowManagerTest.WindowIdMatcher.a11yWindowId; @@ -823,7 +824,8 @@ public class AccessibilityWindowManagerTest { // Starts tracking window of second display. startTrackingPerDisplay(SECONDARY_DISPLAY_ID); - final ArrayList displayList = mA11yWindowManager.getDisplayListLocked(); + final ArrayList displayList = mA11yWindowManager.getDisplayListLocked( + DISPLAY_TYPE_DEFAULT); assertTrue(displayList.equals(mExpectedDisplayList)); }