Merge "Fix race condition between VDM and ProxyManager when registering a proxy" into main (a4290cd6) · Commits · e / os / android_frameworks_base

services/accessibility/Android.bp

+16 −0

Original line number	Diff line number	Diff line
		@@ -27,4 +27,20 @@ java_library_static {
		"services.core",
		"androidx.annotation_annotation",
		],
		static_libs: [
		"com_android_server_accessibility_flags_lib",
		],
		}

		aconfig_declarations {
		name: "com_android_server_accessibility_flags",
		package: "com.android.server.accessibility",
		srcs: [
		"accessibility.aconfig",
		],
		}

		java_aconfig_library {
		name: "com_android_server_accessibility_flags_lib",
		aconfig_declarations: "com_android_server_accessibility_flags",
		}

services/accessibility/accessibility.aconfig

0 → 100644

+7 −0

Original line number	Diff line number	Diff line
		package: "com.android.server.accessibility"
		flag {
		name: "proxy_use_apps_on_virtual_device_listener"
		namespace: "accessibility"
		description: "Fixes race condition described in b/286587811"
		bug: "286587811"
		}

services/accessibility/java/com/android/server/accessibility/ProxyManager.java

+104 −12

Original line number	Diff line number	Diff line
		@@ -33,6 +33,7 @@ import android.os.Handler;
		import android.os.IBinder;
		import android.os.RemoteCallbackList;
		import android.os.RemoteException;
		import android.util.ArraySet;
		import android.util.IntArray;
		import android.util.Slog;
		import android.util.SparseArray;
		@@ -42,6 +43,7 @@ import android.view.accessibility.AccessibilityEvent;
		import android.view.accessibility.AccessibilityManager;
		import android.view.accessibility.IAccessibilityManagerClient;

		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.util.IntPair;
		import com.android.server.LocalServices;
		import com.android.server.companion.virtual.VirtualDeviceManagerInternal;
		@@ -96,6 +98,9 @@ public class ProxyManager {

		private final SystemSupport mSystemSupport;

		private VirtualDeviceManagerInternal.AppsOnVirtualDeviceListener
		mAppsOnVirtualDeviceListener;

		/**
		* Callbacks into AccessibilityManagerService.
		*/
		@@ -174,6 +179,16 @@ public class ProxyManager {

		synchronized (mLock) {
		mProxyA11yServiceConnections.put(displayId, connection);
		if (Flags.proxyUseAppsOnVirtualDeviceListener()) {
		if (mAppsOnVirtualDeviceListener == null) {
		mAppsOnVirtualDeviceListener = allRunningUids ->
		notifyProxyOfRunningAppsChange(allRunningUids);
		final VirtualDeviceManagerInternal localVdm = getLocalVdm();
		if (localVdm != null) {
		localVdm.registerAppsOnVirtualDeviceListener(mAppsOnVirtualDeviceListener);
		}
		}
		}
		}

		// If the client dies, make sure to remove the connection.
		@@ -276,11 +291,21 @@ public class ProxyManager {
		}
		}
		});
		// If there isn't an existing proxy for the device id, reset clients. Resetting
		// If there isn't an existing proxy for the device id, reset app clients. Resetting
		// will usually happen, since in most cases there will only be one proxy for a
		// device.
		if (!isProxyedDeviceId(deviceId)) {
		synchronized (mLock) {
		if (Flags.proxyUseAppsOnVirtualDeviceListener()) {
		if (mProxyA11yServiceConnections.size() == 0) {
		final VirtualDeviceManagerInternal localVdm = getLocalVdm();
		if (localVdm != null && mAppsOnVirtualDeviceListener != null) {
		localVdm.unregisterAppsOnVirtualDeviceListener(
		mAppsOnVirtualDeviceListener);
		mAppsOnVirtualDeviceListener = null;
		}
		}
		}
		mSystemSupport.removeDeviceIdLocked(deviceId);
		mLastStates.delete(deviceId);
		}
		@@ -307,7 +332,7 @@ public class ProxyManager {
		* Returns {@code true} if {@code deviceId} is being proxy-ed.
		*/
		public boolean isProxyedDeviceId(int deviceId) {
		if (deviceId == DEVICE_ID_DEFAULT && deviceId == DEVICE_ID_INVALID) {
		if (deviceId == DEVICE_ID_DEFAULT \|\| deviceId == DEVICE_ID_INVALID) {
		return false;
		}
		boolean isTrackingDeviceId;
		@@ -566,7 +591,7 @@ public class ProxyManager {
		* This is similar to onUserStateChangeLocked and onClientChangeLocked, but does not require an
		* A11yUserState and only checks proxy-relevant settings.
		*/
		public void onProxyChanged(int deviceId) {
		private void onProxyChanged(int deviceId, boolean forceUpdate) {
		if (DEBUG) {
		Slog.v(LOG_TAG, "onProxyChanged called for deviceId: " + deviceId);
		}
		@@ -584,7 +609,7 @@ public class ProxyManager {
		// Calls A11yManager#setRelevantEventTypes (test these)
		updateRelevantEventTypesLocked(deviceId);
		// Calls A11yManager#setState
		scheduleUpdateProxyClientsIfNeededLocked(deviceId);
		scheduleUpdateProxyClientsIfNeededLocked(deviceId, forceUpdate);
		//Calls A11yManager#notifyServicesStateChanged(timeout)
		scheduleNotifyProxyClientsOfServicesStateChangeLocked(deviceId);
		// Calls A11yManager#setFocusAppearance
		@@ -593,17 +618,26 @@ public class ProxyManager {
		}
		}

		/**
		* Handles proxy changes, but does not force an update of app clients.
		*/
		public void onProxyChanged(int deviceId) {
		onProxyChanged(deviceId, false);
		}

		/**
		* Updates the states of the app AccessibilityManagers.
		*/
		private void scheduleUpdateProxyClientsIfNeededLocked(int deviceId) {
		private void scheduleUpdateProxyClientsIfNeededLocked(int deviceId, boolean forceUpdate) {
		final int proxyState = getStateLocked(deviceId);
		if (DEBUG) {
		Slog.v(LOG_TAG, "State for device id " + deviceId + " is " + proxyState);
		Slog.v(LOG_TAG, "Last state for device id " + deviceId + " is "
		+ getLastSentStateLocked(deviceId));
		Slog.v(LOG_TAG, "force update: " + forceUpdate);
		}
		if ((getLastSentStateLocked(deviceId)) != proxyState) {
		if ((getLastSentStateLocked(deviceId)) != proxyState
		\|\| (Flags.proxyUseAppsOnVirtualDeviceListener() && forceUpdate)) {
		setLastStateLocked(deviceId, proxyState);
		mMainHandler.post(() -> {
		synchronized (mLock) {
		@@ -792,7 +826,7 @@ public class ProxyManager {
		}

		/**
		* Updates the device ids of IAccessibilityManagerClients if needed.
		* Updates the device ids of IAccessibilityManagerClients if needed after a proxy change.
		*/
		private void updateDeviceIdsIfNeededLocked(int deviceId,
		@NonNull RemoteCallbackList<IAccessibilityManagerClient> clients) {
		@@ -804,6 +838,26 @@ public class ProxyManager {
		for (int i = 0; i < clients.getRegisteredCallbackCount(); i++) {
		final AccessibilityManagerService.Client client =
		((AccessibilityManagerService.Client) clients.getRegisteredCallbackCookie(i));
		if (Flags.proxyUseAppsOnVirtualDeviceListener()) {
		if (deviceId == DEVICE_ID_DEFAULT \|\| deviceId == DEVICE_ID_INVALID) {
		continue;
		}
		boolean uidBelongsToDevice =
		localVdm.getDeviceIdsForUid(client.mUid).contains(deviceId);
		if (client.mDeviceId != deviceId && uidBelongsToDevice) {
		if (DEBUG) {
		Slog.v(LOG_TAG, "Packages moved to device id " + deviceId + " are "
		+ Arrays.toString(client.mPackageNames));
		}
		client.mDeviceId = deviceId;
		} else if (client.mDeviceId == deviceId && !uidBelongsToDevice) {
		client.mDeviceId = DEVICE_ID_DEFAULT;
		if (DEBUG) {
		Slog.v(LOG_TAG, "Packages moved to the default device from device id "
		+ deviceId + " are " + Arrays.toString(client.mPackageNames));
		}
		}
		} else {
		if (deviceId != DEVICE_ID_DEFAULT && deviceId != DEVICE_ID_INVALID
		&& localVdm.getDeviceIdsForUid(client.mUid).contains(deviceId)) {
		if (DEBUG) {
		@@ -814,6 +868,39 @@ public class ProxyManager {
		}
		}
		}
		}

		@VisibleForTesting
		void notifyProxyOfRunningAppsChange(Set<Integer> allRunningUids) {
		if (DEBUG) {
		Slog.v(LOG_TAG, "notifyProxyOfRunningAppsChange: " + allRunningUids);
		}
		synchronized (mLock) {
		if (mProxyA11yServiceConnections.size() == 0) {
		return;
		}
		final VirtualDeviceManagerInternal localVdm = getLocalVdm();
		if (localVdm == null) {
		return;
		}
		final ArraySet<Integer> deviceIdsToUpdate = new ArraySet<>();
		for (int i = 0; i < mProxyA11yServiceConnections.size(); i++) {
		final ProxyAccessibilityServiceConnection proxy =
		mProxyA11yServiceConnections.valueAt(i);
		if (proxy != null) {
		final int proxyDeviceId = proxy.getDeviceId();
		for (Integer uid : allRunningUids) {
		if (localVdm.getDeviceIdsForUid(uid).contains(proxyDeviceId)) {
		deviceIdsToUpdate.add(proxyDeviceId);
		}
		}
		}
		}
		for (Integer proxyDeviceId : deviceIdsToUpdate) {
		onProxyChanged(proxyDeviceId, true);
		}
		}
		}

		/**
		* Clears all proxy caches.
		@@ -843,6 +930,11 @@ public class ProxyManager {
		return mLocalVdm;
		}

		@VisibleForTesting
		void setLocalVirtualDeviceManager(VirtualDeviceManagerInternal localVdm) {
		mLocalVdm = localVdm;
		}

		/**
		* Prints information belonging to each display that is controlled by an
		* AccessibilityDisplayProxy.

services/tests/servicestests/src/com/android/server/accessibility/ProxyManagerTest.java

+51 −10

Original line number	Diff line number	Diff line
		@@ -23,7 +23,14 @@ import static com.android.server.accessibility.ProxyAccessibilityServiceConnecti
		import static com.android.server.accessibility.ProxyManager.PROXY_COMPONENT_CLASS_NAME;
		import static com.android.server.accessibility.ProxyManager.PROXY_COMPONENT_PACKAGE_NAME;

		import static com.google.common.truth.Truth.assertThat;

		import static org.junit.Assert.fail;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyInt;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		import android.accessibilityservice.AccessibilityGestureEvent;
		import android.accessibilityservice.AccessibilityServiceInfo;
		@@ -40,6 +47,7 @@ import android.graphics.Region;
		import android.os.IBinder;
		import android.os.RemoteCallbackList;
		import android.os.RemoteException;
		import android.platform.test.flag.junit.SetFlagsRule;
		import android.util.ArraySet;
		import android.view.KeyEvent;
		import android.view.MotionEvent;
		@@ -48,6 +56,8 @@ import android.view.accessibility.AccessibilityManager;
		import android.view.accessibility.IAccessibilityManagerClient;
		import android.view.inputmethod.EditorInfo;

		import androidx.test.InstrumentationRegistry;

		import com.android.internal.R;
		import com.android.internal.inputmethod.IAccessibilityInputMethodSession;
		import com.android.internal.inputmethod.IAccessibilityInputMethodSessionCallback;
		@@ -58,20 +68,12 @@ import com.android.server.accessibility.test.MessageCapturingHandler;
		import com.android.server.companion.virtual.VirtualDeviceManagerInternal;
		import com.android.server.wm.WindowManagerInternal;

		import static com.google.common.truth.Truth.assertThat;

		import androidx.test.InstrumentationRegistry;

		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyInt;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		import org.junit.After;
		import org.junit.Before;
		import org.junit.Rule;
		import org.junit.Test;
		import org.mockito.Mock;
		import org.mockito.Mockito;
		import org.mockito.MockitoAnnotations;

		import java.util.ArrayList;
		@@ -87,6 +89,10 @@ public class ProxyManagerTest {
		private static final int DEVICE_ID = 10;
		private static final int STREAMED_CALLING_UID = 9876;

		@Rule
		public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();


		@Mock private Context mMockContext;
		@Mock private AccessibilitySecurityPolicy mMockSecurityPolicy;
		@Mock private AccessibilityWindowManager mMockA11yWindowManager;
		@@ -110,6 +116,8 @@ public class ProxyManagerTest {
		MockitoAnnotations.initMocks(this);
		final Resources resources = InstrumentationRegistry.getContext().getResources();

		mSetFlagsRule.enableFlags(Flags.FLAG_PROXY_USE_APPS_ON_VIRTUAL_DEVICE_LISTENER);

		mFocusStrokeWidthDefaultValue =
		resources.getDimensionPixelSize(R.dimen.accessibility_focus_highlight_stroke_width);
		mFocusColorDefaultValue = resources.getColor(R.color.accessibility_focus_highlight_color);
		@@ -217,6 +225,39 @@ public class ProxyManagerTest {
		verify(mMockProxySystemSupport).notifyClearAccessibilityCacheLocked();
		}

		/**
		* Tests that the manager's AppsOnVirtualDeviceListener implementation propagates the running
		* app changes to the proxy device.
		*/
		@Test
		public void testUpdateProxyOfRunningAppsChange_changedUidIsStreamedApp_propagatesChange() {
		final VirtualDeviceManagerInternal localVdm =
		Mockito.mock(VirtualDeviceManagerInternal.class);
		when(localVdm.getDeviceIdsForUid(anyInt())).thenReturn(new ArraySet(Set.of(DEVICE_ID)));

		mProxyManager.setLocalVirtualDeviceManager(localVdm);
		registerProxy(DISPLAY_ID);
		verify(localVdm).registerAppsOnVirtualDeviceListener(any());

		final ArraySet<Integer> runningUids = new ArraySet(Set.of(STREAMED_CALLING_UID));

		// Flush any existing messages. The messages after this come from onProxyChanged.
		mMessageCapturingHandler.sendAllMessages();

		// The virtual device has been updated with the streamed app's UID, so the proxy is
		// updated.
		mProxyManager.notifyProxyOfRunningAppsChange(runningUids);

		verify(localVdm).getDeviceIdsForUid(STREAMED_CALLING_UID);
		verify(mMockProxySystemSupport).getCurrentUserClientsLocked();
		verify(mMockProxySystemSupport).getGlobalClientsLocked();
		// Messages to notify IAccessibilityManagerClients should be posted.
		assertThat(mMessageCapturingHandler.hasMessages()).isTrue();

		mProxyManager.unregisterProxy(DISPLAY_ID);
		verify(localVdm).unregisterAppsOnVirtualDeviceListener(any());
		}

		/**
		* Tests that getting the first device id for an app uid, such as when an app queries for
		* device-specific state, returns the right device id.