From eb2d1e65b52268ce790379af8d7314f818c68cbd Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 29 Mar 2022 15:54:00 +0000 Subject: [PATCH 1/2] Make virtual input device creation synchronous To ensure that virtual device creation is synchronized with changing other system parameters (such as changing the display of the mouse pointer), we must make virtual input device creation synchronous. This is required to ensure that the VirtualMouse#getCursorPosition API returns the correct value as soon as the virtual mouse device is created. The system only holds on to a PointerController when there is an input device that can control the pointer that is connected. This is so that we don't need to hold on to cursor graphics resources when there's no mouse or touchpad connected. This means we can only synchronize updating the pointer display once we know such an input device is connected. In this CL, we update the virtual input device creation to wait on the binder thread until the system recognizes the new virtual input device. We also clean up the virtual device creation logic to ensure the proper cleanup is done if creation fails at any point. Bug: 216792538 Test: atest VirtualDeviceManagerServiceTest InputControllerTest Test: atest VirtualMouseTest Change-Id: I3b893d23cacec988eb49c9fd3a79cebe31a59c05 Merged-In: I3b893d23cacec988eb49c9fd3a79cebe31a59c05 --- .../companion/virtual/InputController.java | 219 +++++++++++++----- .../companion/virtual/VirtualDeviceImpl.java | 3 +- .../virtual/InputControllerTest.java | 24 +- .../virtual/InputManagerMockHelper.java | 101 ++++++++ .../VirtualDeviceManagerServiceTest.java | 14 +- 5 files changed, 297 insertions(+), 64 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/companion/virtual/InputManagerMockHelper.java diff --git a/services/companion/java/com/android/server/companion/virtual/InputController.java b/services/companion/java/com/android/server/companion/virtual/InputController.java index 9d4b50be41fb1..80182d26003d7 100644 --- a/services/companion/java/com/android/server/companion/virtual/InputController.java +++ b/services/companion/java/com/android/server/companion/virtual/InputController.java @@ -22,6 +22,7 @@ import android.annotation.StringDef; import android.graphics.Point; import android.graphics.PointF; import android.hardware.display.DisplayManagerInternal; +import android.hardware.input.InputDeviceIdentifier; import android.hardware.input.InputManager; import android.hardware.input.InputManagerInternal; import android.hardware.input.VirtualKeyEvent; @@ -29,11 +30,13 @@ import android.hardware.input.VirtualMouseButtonEvent; import android.hardware.input.VirtualMouseRelativeEvent; import android.hardware.input.VirtualMouseScrollEvent; import android.hardware.input.VirtualTouchEvent; +import android.os.Handler; import android.os.IBinder; import android.os.RemoteException; import android.util.ArrayMap; import android.util.Slog; import android.view.Display; +import android.view.InputDevice; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; @@ -44,7 +47,11 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.Iterator; import java.util.Map; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; /** Controls virtual input devices, including device lifecycle and event dispatch. */ class InputController { @@ -72,20 +79,27 @@ class InputController { @GuardedBy("mLock") final Map mInputDeviceDescriptors = new ArrayMap<>(); + private final Handler mHandler; private final NativeWrapper mNativeWrapper; private final DisplayManagerInternal mDisplayManagerInternal; private final InputManagerInternal mInputManagerInternal; + private final DeviceCreationThreadVerifier mThreadVerifier; - InputController(@NonNull Object lock) { - this(lock, new NativeWrapper()); + InputController(@NonNull Object lock, @NonNull Handler handler) { + this(lock, new NativeWrapper(), handler, + // Verify that virtual devices are not created on the handler thread. + () -> !handler.getLooper().isCurrentThread()); } @VisibleForTesting - InputController(@NonNull Object lock, @NonNull NativeWrapper nativeWrapper) { + InputController(@NonNull Object lock, @NonNull NativeWrapper nativeWrapper, + @NonNull Handler handler, @NonNull DeviceCreationThreadVerifier threadVerifier) { mLock = lock; + mHandler = handler; mNativeWrapper = nativeWrapper; mDisplayManagerInternal = LocalServices.getService(DisplayManagerInternal.class); mInputManagerInternal = LocalServices.getService(InputManagerInternal.class); + mThreadVerifier = threadVerifier; } void close() { @@ -108,23 +122,13 @@ class InputController { @NonNull IBinder deviceToken, int displayId) { final String phys = createPhys(PHYS_TYPE_KEYBOARD); - setUniqueIdAssociation(displayId, phys); - final int fd = mNativeWrapper.openUinputKeyboard(deviceName, vendorId, productId, phys); - if (fd < 0) { - throw new RuntimeException( - "A native error occurred when creating keyboard: " + -fd); - } - final BinderDeathRecipient binderDeathRecipient = new BinderDeathRecipient(deviceToken); - synchronized (mLock) { - mInputDeviceDescriptors.put(deviceToken, - new InputDeviceDescriptor(fd, binderDeathRecipient, - InputDeviceDescriptor.TYPE_KEYBOARD, displayId, phys)); - } try { - deviceToken.linkToDeath(binderDeathRecipient, /* flags= */ 0); - } catch (RemoteException e) { - // TODO(b/215608394): remove and close InputDeviceDescriptor - throw new RuntimeException("Could not create virtual keyboard", e); + createDeviceInternal(InputDeviceDescriptor.TYPE_KEYBOARD, deviceName, vendorId, + productId, deviceToken, displayId, phys, + () -> mNativeWrapper.openUinputKeyboard(deviceName, vendorId, productId, phys)); + } catch (DeviceCreationException e) { + throw new RuntimeException( + "Failed to create virtual keyboard device '" + deviceName + "'.", e); } } @@ -134,25 +138,15 @@ class InputController { @NonNull IBinder deviceToken, int displayId) { final String phys = createPhys(PHYS_TYPE_MOUSE); - setUniqueIdAssociation(displayId, phys); - final int fd = mNativeWrapper.openUinputMouse(deviceName, vendorId, productId, phys); - if (fd < 0) { - throw new RuntimeException( - "A native error occurred when creating mouse: " + -fd); - } - final BinderDeathRecipient binderDeathRecipient = new BinderDeathRecipient(deviceToken); - synchronized (mLock) { - mInputDeviceDescriptors.put(deviceToken, - new InputDeviceDescriptor(fd, binderDeathRecipient, - InputDeviceDescriptor.TYPE_MOUSE, displayId, phys)); - mInputManagerInternal.setVirtualMousePointerDisplayId(displayId); - } try { - deviceToken.linkToDeath(binderDeathRecipient, /* flags= */ 0); - } catch (RemoteException e) { - // TODO(b/215608394): remove and close InputDeviceDescriptor - throw new RuntimeException("Could not create virtual mouse", e); + createDeviceInternal(InputDeviceDescriptor.TYPE_MOUSE, deviceName, vendorId, productId, + deviceToken, displayId, phys, + () -> mNativeWrapper.openUinputMouse(deviceName, vendorId, productId, phys)); + } catch (DeviceCreationException e) { + throw new RuntimeException( + "Failed to create virtual mouse device: '" + deviceName + "'.", e); } + mInputManagerInternal.setVirtualMousePointerDisplayId(displayId); } void createTouchscreen(@NonNull String deviceName, @@ -162,24 +156,14 @@ class InputController { int displayId, @NonNull Point screenSize) { final String phys = createPhys(PHYS_TYPE_TOUCHSCREEN); - setUniqueIdAssociation(displayId, phys); - final int fd = mNativeWrapper.openUinputTouchscreen(deviceName, vendorId, productId, phys, - screenSize.y, screenSize.x); - if (fd < 0) { - throw new RuntimeException( - "A native error occurred when creating touchscreen: " + -fd); - } - final BinderDeathRecipient binderDeathRecipient = new BinderDeathRecipient(deviceToken); - synchronized (mLock) { - mInputDeviceDescriptors.put(deviceToken, - new InputDeviceDescriptor(fd, binderDeathRecipient, - InputDeviceDescriptor.TYPE_TOUCHSCREEN, displayId, phys)); - } try { - deviceToken.linkToDeath(binderDeathRecipient, /* flags= */ 0); - } catch (RemoteException e) { - // TODO(b/215608394): remove and close InputDeviceDescriptor - throw new RuntimeException("Could not create virtual touchscreen", e); + createDeviceInternal(InputDeviceDescriptor.TYPE_TOUCHSCREEN, deviceName, vendorId, + productId, deviceToken, displayId, phys, + () -> mNativeWrapper.openUinputTouchscreen(deviceName, vendorId, productId, + phys, screenSize.y, screenSize.x)); + } catch (DeviceCreationException e) { + throw new RuntimeException( + "Failed to create virtual touchscreen device '" + deviceName + "'.", e); } } @@ -510,4 +494,133 @@ class InputController { unregisterInputDevice(mDeviceToken); } } + + /** A helper class used to wait for an input device to be registered. */ + private class WaitForDevice implements AutoCloseable { + private final CountDownLatch mDeviceAddedLatch = new CountDownLatch(1); + private final InputManager.InputDeviceListener mListener; + + WaitForDevice(String deviceName, int vendorId, int productId) { + mListener = new InputManager.InputDeviceListener() { + @Override + public void onInputDeviceAdded(int deviceId) { + final InputDevice device = InputManager.getInstance().getInputDevice( + deviceId); + Objects.requireNonNull(device, "Newly added input device was null."); + if (!device.getName().equals(deviceName)) { + return; + } + final InputDeviceIdentifier id = device.getIdentifier(); + if (id.getVendorId() != vendorId || id.getProductId() != productId) { + return; + } + mDeviceAddedLatch.countDown(); + } + + @Override + public void onInputDeviceRemoved(int deviceId) { + + } + + @Override + public void onInputDeviceChanged(int deviceId) { + + } + }; + InputManager.getInstance().registerInputDeviceListener(mListener, mHandler); + } + + /** Note: This must not be called from {@link #mHandler}'s thread. */ + void waitForDeviceCreation() throws DeviceCreationException { + try { + if (!mDeviceAddedLatch.await(1, TimeUnit.MINUTES)) { + throw new DeviceCreationException( + "Timed out waiting for virtual device to be created."); + } + } catch (InterruptedException e) { + throw new DeviceCreationException( + "Interrupted while waiting for virtual device to be created.", e); + } + } + + @Override + public void close() { + InputManager.getInstance().unregisterInputDeviceListener(mListener); + } + } + + /** An internal exception that is thrown to indicate an error when opening a virtual device. */ + private static class DeviceCreationException extends Exception { + DeviceCreationException(String message) { + super(message); + } + DeviceCreationException(String message, Exception cause) { + super(message, cause); + } + } + + /** + * Creates a virtual input device synchronously, and waits for the notification that the device + * was added. + * + * Note: Input device creation is expected to happen on a binder thread, and the calling thread + * will be blocked until the input device creation is successful. This should not be called on + * the handler's thread. + * + * @throws DeviceCreationException Throws this exception if anything unexpected happens in the + * process of creating the device. This method will take care + * to restore the state of the system in the event of any + * unexpected behavior. + */ + private void createDeviceInternal(@InputDeviceDescriptor.Type int type, String deviceName, + int vendorId, int productId, IBinder deviceToken, int displayId, String phys, + Supplier deviceOpener) + throws DeviceCreationException { + if (!mThreadVerifier.isValidThread()) { + throw new IllegalStateException( + "Virtual device creation should happen on an auxiliary thread (e.g. binder " + + "thread) and not from the handler's thread."); + } + + final int fd; + final BinderDeathRecipient binderDeathRecipient; + + setUniqueIdAssociation(displayId, phys); + try (WaitForDevice waiter = new WaitForDevice(deviceName, vendorId, productId)) { + fd = deviceOpener.get(); + if (fd < 0) { + throw new DeviceCreationException( + "A native error occurred when creating touchscreen: " + -fd); + } + // The fd is valid from here, so ensure that all failures close the fd after this point. + try { + waiter.waitForDeviceCreation(); + + binderDeathRecipient = new BinderDeathRecipient(deviceToken); + try { + deviceToken.linkToDeath(binderDeathRecipient, /* flags= */ 0); + } catch (RemoteException e) { + throw new DeviceCreationException( + "Client died before virtual device could be created.", e); + } + } catch (DeviceCreationException e) { + mNativeWrapper.closeUinput(fd); + throw e; + } + } catch (DeviceCreationException e) { + InputManager.getInstance().removeUniqueIdAssociation(phys); + throw e; + } + + synchronized (mLock) { + mInputDeviceDescriptors.put(deviceToken, + new InputDeviceDescriptor(fd, binderDeathRecipient, type, displayId, phys)); + } + } + + @VisibleForTesting + interface DeviceCreationThreadVerifier { + /** Returns true if the calling thread is a valid thread for device creation. */ + boolean isValidThread(); + } } diff --git a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java index de14ef61a075a..9802b9783da26 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java @@ -166,7 +166,8 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub mAppToken = token; mParams = params; if (inputController == null) { - mInputController = new InputController(mVirtualDeviceLock); + mInputController = new InputController( + mVirtualDeviceLock, context.getMainThreadHandler()); } else { mInputController = inputController; } diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java index b4bb04d2b1b43..92e7a86876e95 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java @@ -21,20 +21,21 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import android.hardware.display.DisplayManagerInternal; import android.hardware.input.IInputManager; -import android.hardware.input.InputManager; import android.hardware.input.InputManagerInternal; import android.os.Binder; +import android.os.Handler; import android.os.IBinder; import android.platform.test.annotations.Presubmit; +import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; import android.view.Display; import android.view.DisplayInfo; -import androidx.test.runner.AndroidJUnit4; - import com.android.server.LocalServices; import org.junit.Before; @@ -44,7 +45,8 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; @Presubmit -@RunWith(AndroidJUnit4.class) +@RunWith(AndroidTestingRunner.class) +@TestableLooper.RunWithLooper(setAsMainLooper = true) public class InputControllerTest { @Mock @@ -56,11 +58,14 @@ public class InputControllerTest { @Mock private IInputManager mIInputManagerMock; + private InputManagerMockHelper mInputManagerMockHelper; private InputController mInputController; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + mInputManagerMockHelper = new InputManagerMockHelper( + TestableLooper.get(this), mNativeWrapperMock, mIInputManagerMock); doNothing().when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); LocalServices.removeServiceForTest(InputManagerInternal.class); @@ -72,10 +77,10 @@ public class InputControllerTest { LocalServices.removeServiceForTest(DisplayManagerInternal.class); LocalServices.addService(DisplayManagerInternal.class, mDisplayManagerInternalMock); - InputManager.resetInstance(mIInputManagerMock); - doNothing().when(mIInputManagerMock).addUniqueIdAssociation(anyString(), anyString()); - doNothing().when(mIInputManagerMock).removeUniqueIdAssociation(anyString()); - mInputController = new InputController(new Object(), mNativeWrapperMock); + // Allow virtual devices to be created on the looper thread for testing. + final InputController.DeviceCreationThreadVerifier threadVerifier = () -> true; + mInputController = new InputController(new Object(), mNativeWrapperMock, + new Handler(TestableLooper.get(this).getLooper()), threadVerifier); } @Test @@ -83,6 +88,7 @@ public class InputControllerTest { final IBinder deviceToken = new Binder(); mInputController.createMouse("name", /*vendorId= */ 1, /*productId= */ 1, deviceToken, /* displayId= */ 1); + verify(mNativeWrapperMock).openUinputMouse(eq("name"), eq(1), eq(1), anyString()); verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(1)); doReturn(1).when(mInputManagerInternalMock).getVirtualMousePointerDisplayId(); mInputController.unregisterInputDevice(deviceToken); @@ -95,10 +101,12 @@ public class InputControllerTest { final IBinder deviceToken = new Binder(); mInputController.createMouse("name", /*vendorId= */ 1, /*productId= */ 1, deviceToken, /* displayId= */ 1); + verify(mNativeWrapperMock).openUinputMouse(eq("name"), eq(1), eq(1), anyString()); verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(1)); final IBinder deviceToken2 = new Binder(); mInputController.createMouse("name", /*vendorId= */ 1, /*productId= */ 1, deviceToken2, /* displayId= */ 2); + verify(mNativeWrapperMock, times(2)).openUinputMouse(eq("name"), eq(1), eq(1), anyString()); verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(2)); mInputController.unregisterInputDevice(deviceToken); verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(1)); diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/InputManagerMockHelper.java b/services/tests/servicestests/src/com/android/server/companion/virtual/InputManagerMockHelper.java new file mode 100644 index 0000000000000..aa2d97e389284 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/InputManagerMockHelper.java @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.companion.virtual; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.notNull; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.when; + +import android.hardware.input.IInputDevicesChangedListener; +import android.hardware.input.IInputManager; +import android.hardware.input.InputManager; +import android.os.RemoteException; +import android.testing.TestableLooper; +import android.view.InputDevice; + +import org.mockito.invocation.InvocationOnMock; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.stream.IntStream; + +/** + * A test utility class used to share the logic for setting up {@link InputManager}'s callback for + * when a virtual input device being added. + */ +class InputManagerMockHelper { + private final TestableLooper mTestableLooper; + private final InputController.NativeWrapper mNativeWrapperMock; + private final IInputManager mIInputManagerMock; + private final List mDevices = new ArrayList<>(); + private IInputDevicesChangedListener mDevicesChangedListener; + + InputManagerMockHelper(TestableLooper testableLooper, + InputController.NativeWrapper nativeWrapperMock, IInputManager iInputManagerMock) + throws Exception { + mTestableLooper = testableLooper; + mNativeWrapperMock = nativeWrapperMock; + mIInputManagerMock = iInputManagerMock; + + doAnswer(this::handleNativeOpenInputDevice).when(mNativeWrapperMock).openUinputMouse( + anyString(), anyInt(), anyInt(), anyString()); + doAnswer(this::handleNativeOpenInputDevice).when(mNativeWrapperMock).openUinputKeyboard( + anyString(), anyInt(), anyInt(), anyString()); + doAnswer(this::handleNativeOpenInputDevice).when(mNativeWrapperMock).openUinputTouchscreen( + anyString(), anyInt(), anyInt(), anyString(), anyInt(), anyInt()); + + doAnswer(inv -> { + mDevicesChangedListener = inv.getArgument(0); + return null; + }).when(mIInputManagerMock).registerInputDevicesChangedListener(notNull()); + when(mIInputManagerMock.getInputDeviceIds()).thenReturn(new int[0]); + doAnswer(inv -> mDevices.get(inv.getArgument(0))) + .when(mIInputManagerMock).getInputDevice(anyInt()); + doNothing().when(mIInputManagerMock).addUniqueIdAssociation(anyString(), anyString()); + doNothing().when(mIInputManagerMock).removeUniqueIdAssociation(anyString()); + + // Set a new instance of InputManager for testing that uses the IInputManager mock as the + // interface to the server. + InputManager.resetInstance(mIInputManagerMock); + } + + private Void handleNativeOpenInputDevice(InvocationOnMock inv) { + Objects.requireNonNull(mDevicesChangedListener, + "InputController did not register an InputDevicesChangedListener."); + // We only use a subset of the fields of InputDevice in InputController. + final InputDevice device = new InputDevice(mDevices.size() /*id*/, 1 /*generation*/, 0, + inv.getArgument(0) /*name*/, inv.getArgument(1) /*vendorId*/, + inv.getArgument(2) /*productId*/, inv.getArgument(3) /*descriptor*/, + true /*isExternal*/, 0 /*sources*/, 0 /*keyboardType*/, + null /*keyCharacterMap*/, false /*hasVibrator*/, false /*hasMic*/, + false /*hasButtonUnderPad*/, false /*hasSensor*/, false /*hasBattery*/); + mDevices.add(device); + try { + mDevicesChangedListener.onInputDevicesChanged( + mDevices.stream().flatMapToInt( + d -> IntStream.of(d.getId(), d.getGeneration())).toArray()); + } catch (RemoteException ignored) { + } + // Process the device added notification. + mTestableLooper.processAllMessages(); + return null; + } +} diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java index 808f8c2cc6267..22152a1953b92 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java @@ -54,6 +54,7 @@ import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; import android.graphics.Point; import android.hardware.display.DisplayManagerInternal; +import android.hardware.input.IInputManager; import android.hardware.input.InputManagerInternal; import android.hardware.input.VirtualKeyEvent; import android.hardware.input.VirtualMouseButtonEvent; @@ -118,6 +119,7 @@ public class VirtualDeviceManagerServiceTest { private static final int FLAG_CANNOT_DISPLAY_ON_REMOTE_DEVICES = 0x00000; private Context mContext; + private InputManagerMockHelper mInputManagerMockHelper; private VirtualDeviceImpl mDeviceImpl; private InputController mInputController; private AssociationInfo mAssociationInfo; @@ -146,6 +148,8 @@ public class VirtualDeviceManagerServiceTest { private IAudioConfigChangedCallback mConfigChangedCallback; @Mock private ApplicationInfo mApplicationInfoMock; + @Mock + IInputManager mIInputManagerMock; private ArraySet getBlockedActivities() { ArraySet blockedActivities = new ArraySet<>(); @@ -170,7 +174,7 @@ public class VirtualDeviceManagerServiceTest { } @Before - public void setUp() { + public void setUp() throws Exception { MockitoAnnotations.initMocks(this); LocalServices.removeServiceForTest(DisplayManagerInternal.class); @@ -199,7 +203,13 @@ public class VirtualDeviceManagerServiceTest { new Handler(TestableLooper.get(this).getLooper())); when(mContext.getSystemService(Context.POWER_SERVICE)).thenReturn(mPowerManager); - mInputController = new InputController(new Object(), mNativeWrapperMock); + mInputManagerMockHelper = new InputManagerMockHelper( + TestableLooper.get(this), mNativeWrapperMock, mIInputManagerMock); + // Allow virtual devices to be created on the looper thread for testing. + final InputController.DeviceCreationThreadVerifier threadVerifier = () -> true; + mInputController = new InputController(new Object(), mNativeWrapperMock, + new Handler(TestableLooper.get(this).getLooper()), threadVerifier); + mAssociationInfo = new AssociationInfo(1, 0, null, MacAddress.BROADCAST_ADDRESS, "", null, true, false, 0, 0); From d4fd3a112b4cb46da17dc996daf745131664cc65 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Thu, 10 Mar 2022 14:39:46 +0000 Subject: [PATCH 2/2] Synchronize pointer display change requests Previously, when InputManagerService requests for PointerController to change the pointer display, there was no way to know when the request was completed or whether it succeeded. This could lead to a few issues: - WM's MousePositionTracker's coordinates would not be updated until the next mouse event was generated, meaning the position would be out of sync. - The creation of a virtual mouse device moves the pointer to a specific displayId. In order to test this behavior, we would need to sleep in the test code to wait for the system to update the pointer display and position, resulting in generally flaky tests. Here, we add a way to synchonize changes to the pointer display so that InputMangerService can know the current pointer display with certainty. PointerController, which is updated in the InputReader thread, is the source of truth of the pointer display. We add a policy call to notify IMS when the pointer display changes. When the pointer display is changed, the cursor position on the updated display is also updated so that the VirtualMouse#getCursorPosition() API is synchronized to the pointer display change. Bug: 216792538 Test: atest FrameworksServicesTests:InputManagerServiceTests Test: atest PointerIconTest Change-Id: I578fd1aba9335e2e078d749321e55a6d05299f3b Merged-In: I578fd1aba9335e2e078d749321e55a6d05299f3b --- .../hardware/input/InputManagerInternal.java | 9 +- libs/input/PointerController.cpp | 7 + libs/input/PointerController.h | 1 + libs/input/PointerControllerContext.h | 1 + libs/input/tests/PointerController_test.cpp | 40 +++- .../server/input/InputManagerService.java | 135 +++++++++-- .../input/NativeInputManagerService.java | 6 + .../server/wm/InputManagerCallback.java | 16 ++ .../server/wm/WindowManagerService.java | 44 +++- ...droid_server_input_InputManagerService.cpp | 69 +++--- .../virtual/InputControllerTest.java | 3 +- .../VirtualDeviceManagerServiceTest.java | 2 +- .../server/input/InputManagerServiceTests.kt | 222 ++++++++++++++++++ 13 files changed, 488 insertions(+), 67 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt diff --git a/core/java/android/hardware/input/InputManagerInternal.java b/core/java/android/hardware/input/InputManagerInternal.java index b37c27c2a9e78..fc6bc555e823b 100644 --- a/core/java/android/hardware/input/InputManagerInternal.java +++ b/core/java/android/hardware/input/InputManagerInternal.java @@ -75,8 +75,15 @@ public abstract class InputManagerInternal { /** * Sets the display id that the MouseCursorController will be forced to target. Pass * {@link android.view.Display#INVALID_DISPLAY} to clear the override. + * + * Note: This method generally blocks until the pointer display override has propagated. + * When setting a new override, the caller should ensure that an input device that can control + * the mouse pointer is connected. If a new override is set when no such input device is + * connected, the caller may be blocked for an arbitrary period of time. + * + * @return true if the pointer displayId was set successfully, or false if it fails. */ - public abstract void setVirtualMousePointerDisplayId(int pointerDisplayId); + public abstract boolean setVirtualMousePointerDisplayId(int pointerDisplayId); /** * Gets the display id that the MouseCursorController is being forced to target. Returns diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index 1dc74e5f7740d..10ea6512c7244 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -106,6 +106,7 @@ PointerController::PointerController(const sp& PointerController::~PointerController() { mDisplayInfoListener->onPointerControllerDestroyed(); mUnregisterWindowInfosListener(mDisplayInfoListener); + mContext.getPolicy()->onPointerDisplayIdChanged(ADISPLAY_ID_NONE, 0, 0); } std::mutex& PointerController::getLock() const { @@ -255,6 +256,12 @@ void PointerController::setDisplayViewport(const DisplayViewport& viewport) { getAdditionalMouseResources = true; } mCursorController.setDisplayViewport(viewport, getAdditionalMouseResources); + if (viewport.displayId != mLocked.pointerDisplayId) { + float xPos, yPos; + mCursorController.getPosition(&xPos, &yPos); + mContext.getPolicy()->onPointerDisplayIdChanged(viewport.displayId, xPos, yPos); + mLocked.pointerDisplayId = viewport.displayId; + } } void PointerController::updatePointerIcon(int32_t iconId) { diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h index 2e6e851ee15ab..eab030f71e1ab 100644 --- a/libs/input/PointerController.h +++ b/libs/input/PointerController.h @@ -104,6 +104,7 @@ private: struct Locked { Presentation presentation; + int32_t pointerDisplayId = ADISPLAY_ID_NONE; std::vector mDisplayInfos; std::unordered_map spotControllers; diff --git a/libs/input/PointerControllerContext.h b/libs/input/PointerControllerContext.h index 26a65a47471d2..c2bc1e020279e 100644 --- a/libs/input/PointerControllerContext.h +++ b/libs/input/PointerControllerContext.h @@ -79,6 +79,7 @@ public: std::map* outAnimationResources, int32_t displayId) = 0; virtual int32_t getDefaultPointerIconId() = 0; virtual int32_t getCustomPointerIconId() = 0; + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) = 0; }; /* diff --git a/libs/input/tests/PointerController_test.cpp b/libs/input/tests/PointerController_test.cpp index dae1fccec8043..f9752ed155dff 100644 --- a/libs/input/tests/PointerController_test.cpp +++ b/libs/input/tests/PointerController_test.cpp @@ -56,9 +56,11 @@ public: std::map* outAnimationResources, int32_t displayId) override; virtual int32_t getDefaultPointerIconId() override; virtual int32_t getCustomPointerIconId() override; + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) override; bool allResourcesAreLoaded(); bool noResourcesAreLoaded(); + std::optional getLastReportedPointerDisplayId() { return latestPointerDisplayId; } private: void loadPointerIconForType(SpriteIcon* icon, int32_t cursorType); @@ -66,6 +68,7 @@ private: bool pointerIconLoaded{false}; bool pointerResourcesLoaded{false}; bool additionalMouseResourcesLoaded{false}; + std::optional latestPointerDisplayId; }; void MockPointerControllerPolicyInterface::loadPointerIcon(SpriteIcon* icon, int32_t) { @@ -126,12 +129,19 @@ void MockPointerControllerPolicyInterface::loadPointerIconForType(SpriteIcon* ic icon->hotSpotX = hotSpot.first; icon->hotSpotY = hotSpot.second; } + +void MockPointerControllerPolicyInterface::onPointerDisplayIdChanged(int32_t displayId, + float /*xPos*/, + float /*yPos*/) { + latestPointerDisplayId = displayId; +} + class PointerControllerTest : public Test { protected: PointerControllerTest(); ~PointerControllerTest(); - void ensureDisplayViewportIsSet(); + void ensureDisplayViewportIsSet(int32_t displayId = ADISPLAY_ID_DEFAULT); sp mPointerSprite; sp mPolicy; @@ -168,9 +178,9 @@ PointerControllerTest::~PointerControllerTest() { mThread.join(); } -void PointerControllerTest::ensureDisplayViewportIsSet() { +void PointerControllerTest::ensureDisplayViewportIsSet(int32_t displayId) { DisplayViewport viewport; - viewport.displayId = ADISPLAY_ID_DEFAULT; + viewport.displayId = displayId; viewport.logicalRight = 1600; viewport.logicalBottom = 1200; viewport.physicalRight = 800; @@ -255,6 +265,30 @@ TEST_F(PointerControllerTest, doesNotGetResourcesBeforeSettingViewport) { ensureDisplayViewportIsSet(); } +TEST_F(PointerControllerTest, notifiesPolicyWhenPointerDisplayChanges) { + EXPECT_FALSE(mPolicy->getLastReportedPointerDisplayId()) + << "A pointer display change does not occur when PointerController is created."; + + ensureDisplayViewportIsSet(ADISPLAY_ID_DEFAULT); + + const auto lastReportedPointerDisplayId = mPolicy->getLastReportedPointerDisplayId(); + ASSERT_TRUE(lastReportedPointerDisplayId) + << "The policy is notified of a pointer display change when the viewport is first set."; + EXPECT_EQ(ADISPLAY_ID_DEFAULT, *lastReportedPointerDisplayId) + << "Incorrect pointer display notified."; + + ensureDisplayViewportIsSet(42); + + EXPECT_EQ(42, *mPolicy->getLastReportedPointerDisplayId()) + << "The policy is notified when the pointer display changes."; + + // Release the PointerController. + mPointerController = nullptr; + + EXPECT_EQ(ADISPLAY_ID_NONE, *mPolicy->getLastReportedPointerDisplayId()) + << "The pointer display changes to invalid when PointerController is destroyed."; +} + class PointerControllerWindowInfoListenerTest : public Test {}; class TestPointerController : public PointerController { diff --git a/services/core/java/com/android/server/input/InputManagerService.java b/services/core/java/com/android/server/input/InputManagerService.java index 8ab0b931be11f..9b78068497c8f 100644 --- a/services/core/java/com/android/server/input/InputManagerService.java +++ b/services/core/java/com/android/server/input/InputManagerService.java @@ -164,6 +164,7 @@ public class InputManagerService extends IInputManager.Stub private static final int MSG_UPDATE_KEYBOARD_LAYOUTS = 4; private static final int MSG_RELOAD_DEVICE_ALIASES = 5; private static final int MSG_DELIVER_TABLET_MODE_CHANGED = 6; + private static final int MSG_POINTER_DISPLAY_ID_CHANGED = 7; private static final int DEFAULT_VIBRATION_MAGNITUDE = 192; @@ -276,11 +277,24 @@ public class InputManagerService extends IInputManager.Stub @GuardedBy("mAssociationLock") private final Map mUniqueIdAssociations = new ArrayMap<>(); + // Guards per-display input properties and properties relating to the mouse pointer. + // Threads can wait on this lock to be notified the next time the display on which the mouse + // pointer is shown has changed. private final Object mAdditionalDisplayInputPropertiesLock = new Object(); - // Forces the MouseCursorController to target a specific display id. + // Forces the PointerController to target a specific display id. @GuardedBy("mAdditionalDisplayInputPropertiesLock") private int mOverriddenPointerDisplayId = Display.INVALID_DISPLAY; + + // PointerController is the source of truth of the pointer display. This is the value of the + // latest pointer display id reported by PointerController. + @GuardedBy("mAdditionalDisplayInputPropertiesLock") + private int mAcknowledgedPointerDisplayId = Display.INVALID_DISPLAY; + // This is the latest display id that IMS has requested PointerController to use. If there are + // no devices that can control the pointer, PointerController may end up disregarding this + // value. + @GuardedBy("mAdditionalDisplayInputPropertiesLock") + private int mRequestedPointerDisplayId = Display.INVALID_DISPLAY; @GuardedBy("mAdditionalDisplayInputPropertiesLock") private final SparseArray mAdditionalDisplayInputProperties = new SparseArray<>(); @@ -289,7 +303,6 @@ public class InputManagerService extends IInputManager.Stub @GuardedBy("mAdditionalDisplayInputPropertiesLock") private PointerIcon mIcon; - // Holds all the registered gesture monitors that are implemented as spy windows. The spy // windows are mapped by their InputChannel tokens. @GuardedBy("mInputMonitors") @@ -383,6 +396,10 @@ public class InputManagerService extends IInputManager.Stub NativeInputManagerService getNativeService(InputManagerService service) { return new NativeInputManagerService.NativeImpl(service, mContext, mLooper.getQueue()); } + + void registerLocalService(InputManagerInternal localService) { + LocalServices.addService(InputManagerInternal.class, localService); + } } public InputManagerService(Context context) { @@ -406,7 +423,7 @@ public class InputManagerService extends IInputManager.Stub mDoubleTouchGestureEnableFile = TextUtils.isEmpty(doubleTouchGestureEnablePath) ? null : new File(doubleTouchGestureEnablePath); - LocalServices.addService(InputManagerInternal.class, new LocalService()); + injector.registerLocalService(new LocalService()); } public void setWindowManagerCallbacks(WindowManagerCallbacks callbacks) { @@ -556,6 +573,8 @@ public class InputManagerService extends IInputManager.Stub vArray[i] = viewports.get(i); } mNative.setDisplayViewports(vArray); + // Always attempt to update the pointer display when viewports change. + updatePointerDisplayId(); if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) { final AdditionalDisplayInputProperties properties = @@ -1961,10 +1980,43 @@ public class InputManagerService extends IInputManager.Stub return result; } - private void setVirtualMousePointerDisplayId(int displayId) { + /** + * Update the display on which the mouse pointer is shown. + * If there is an overridden display for the mouse pointer, use that. Otherwise, query + * WindowManager for the pointer display. + * + * @return true if the pointer displayId changed, false otherwise. + */ + private boolean updatePointerDisplayId() { + synchronized (mAdditionalDisplayInputPropertiesLock) { + final int pointerDisplayId = mOverriddenPointerDisplayId != Display.INVALID_DISPLAY + ? mOverriddenPointerDisplayId : mWindowManagerCallbacks.getPointerDisplayId(); + if (mRequestedPointerDisplayId == pointerDisplayId) { + return false; + } + mRequestedPointerDisplayId = pointerDisplayId; + mNative.setPointerDisplayId(pointerDisplayId); + return true; + } + } + + private void handlePointerDisplayIdChanged(PointerDisplayIdChangedArgs args) { + synchronized (mAdditionalDisplayInputPropertiesLock) { + mAcknowledgedPointerDisplayId = args.mPointerDisplayId; + // Notify waiting threads that the display of the mouse pointer has changed. + mAdditionalDisplayInputPropertiesLock.notifyAll(); + } + mWindowManagerCallbacks.notifyPointerDisplayIdChanged( + args.mPointerDisplayId, args.mXPosition, args.mYPosition); + } + + private boolean setVirtualMousePointerDisplayIdBlocking(int displayId) { + // Indicates whether this request is for removing the override. + final boolean removingOverride = displayId == Display.INVALID_DISPLAY; + synchronized (mAdditionalDisplayInputPropertiesLock) { mOverriddenPointerDisplayId = displayId; - if (displayId != Display.INVALID_DISPLAY) { + if (!removingOverride) { final AdditionalDisplayInputProperties properties = mAdditionalDisplayInputProperties.get(displayId); if (properties != null) { @@ -1972,9 +2024,30 @@ public class InputManagerService extends IInputManager.Stub updatePointerIconVisibleLocked(properties.pointerIconVisible); } } + if (!updatePointerDisplayId() && mAcknowledgedPointerDisplayId == displayId) { + // The requested pointer display is already set. + return true; + } + if (removingOverride && mAcknowledgedPointerDisplayId == Display.INVALID_DISPLAY) { + // The pointer display override is being removed, but the current pointer display + // is already invalid. This can happen when the PointerController is destroyed as a + // result of the removal of all input devices that can control the pointer. + return true; + } + try { + // The pointer display changed, so wait until the change has propagated. + mAdditionalDisplayInputPropertiesLock.wait(5_000 /*mills*/); + } catch (InterruptedException ignored) { + } + // This request succeeds in two cases: + // - This request was to remove the override, in which case the new pointer display + // could be anything that WM has set. + // - We are setting a new override, in which case the request only succeeds if the + // reported new displayId is the one we requested. This check ensures that if two + // competing overrides are requested in succession, the caller can be notified if one + // of them fails. + return removingOverride || mAcknowledgedPointerDisplayId == displayId; } - // TODO(b/215597605): trigger MousePositionTracker update - mNative.notifyPointerDisplayIdChanged(); } private int getVirtualMousePointerDisplayId() { @@ -3154,18 +3227,6 @@ public class InputManagerService extends IInputManager.Stub return mContext.createDisplayContext(display); } - // Native callback. - @SuppressWarnings("unused") - private int getPointerDisplayId() { - synchronized (mAdditionalDisplayInputPropertiesLock) { - // Prefer the override to all other displays. - if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) { - return mOverriddenPointerDisplayId; - } - } - return mWindowManagerCallbacks.getPointerDisplayId(); - } - // Native callback. @SuppressWarnings("unused") private String[] getKeyboardLayoutOverlay(InputDeviceIdentifier identifier) { @@ -3206,6 +3267,26 @@ public class InputManagerService extends IInputManager.Stub return null; } + private static class PointerDisplayIdChangedArgs { + final int mPointerDisplayId; + final float mXPosition; + final float mYPosition; + PointerDisplayIdChangedArgs(int pointerDisplayId, float xPosition, float yPosition) { + mPointerDisplayId = pointerDisplayId; + mXPosition = xPosition; + mYPosition = yPosition; + } + } + + // Native callback. + @SuppressWarnings("unused") + @VisibleForTesting + void onPointerDisplayIdChanged(int pointerDisplayId, float xPosition, float yPosition) { + mHandler.obtainMessage(MSG_POINTER_DISPLAY_ID_CHANGED, + new PointerDisplayIdChangedArgs(pointerDisplayId, xPosition, + yPosition)).sendToTarget(); + } + /** * Callback interface implemented by the Window Manager. */ @@ -3329,6 +3410,14 @@ public class InputManagerService extends IInputManager.Stub */ @Nullable SurfaceControl createSurfaceForGestureMonitor(String name, int displayId); + + /** + * Notify WindowManagerService when the display of the mouse pointer changes. + * @param displayId The display on which the mouse pointer is shown. + * @param x The x coordinate of the mouse pointer. + * @param y The y coordinate of the mouse pointer. + */ + void notifyPointerDisplayIdChanged(int displayId, float x, float y); } /** @@ -3381,6 +3470,9 @@ public class InputManagerService extends IInputManager.Stub boolean inTabletMode = (boolean) args.arg1; deliverTabletModeChanged(whenNanos, inTabletMode); break; + case MSG_POINTER_DISPLAY_ID_CHANGED: + handlePointerDisplayIdChanged((PointerDisplayIdChangedArgs) msg.obj); + break; } } } @@ -3631,8 +3723,9 @@ public class InputManagerService extends IInputManager.Stub } @Override - public void setVirtualMousePointerDisplayId(int pointerDisplayId) { - InputManagerService.this.setVirtualMousePointerDisplayId(pointerDisplayId); + public boolean setVirtualMousePointerDisplayId(int pointerDisplayId) { + return InputManagerService.this + .setVirtualMousePointerDisplayIdBlocking(pointerDisplayId); } @Override diff --git a/services/core/java/com/android/server/input/NativeInputManagerService.java b/services/core/java/com/android/server/input/NativeInputManagerService.java index 2169155343cd4..81882d277a992 100644 --- a/services/core/java/com/android/server/input/NativeInputManagerService.java +++ b/services/core/java/com/android/server/input/NativeInputManagerService.java @@ -176,6 +176,9 @@ public interface NativeInputManagerService { void cancelCurrentTouch(); + /** Set the displayId on which the mouse cursor should be shown. */ + void setPointerDisplayId(int displayId); + /** The native implementation of InputManagerService methods. */ class NativeImpl implements NativeInputManagerService { /** Pointer to native input manager service object, used by native code. */ @@ -388,5 +391,8 @@ public interface NativeInputManagerService { @Override public native void cancelCurrentTouch(); + + @Override + public native void setPointerDisplayId(int displayId); } } diff --git a/services/core/java/com/android/server/wm/InputManagerCallback.java b/services/core/java/com/android/server/wm/InputManagerCallback.java index 67dd89ee295c2..33cdd2e98113d 100644 --- a/services/core/java/com/android/server/wm/InputManagerCallback.java +++ b/services/core/java/com/android/server/wm/InputManagerCallback.java @@ -270,6 +270,22 @@ final class InputManagerCallback implements InputManagerService.WindowManagerCal } } + @Override + public void notifyPointerDisplayIdChanged(int displayId, float x, float y) { + synchronized (mService.mGlobalLock) { + mService.setMousePointerDisplayId(displayId); + if (displayId == Display.INVALID_DISPLAY) return; + + final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); + if (dc == null) { + Slog.wtf(TAG, "The mouse pointer was moved to display " + displayId + + " that does not have a valid DisplayContent."); + return; + } + mService.restorePointerIconLocked(dc, x, y); + } + } + /** Waits until the built-in input devices have been configured. */ public boolean waitForInputDevicesReady(long timeoutMillis) { synchronized (mInputDevicesReadyMonitor) { diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 7b77fd0683cde..46a45099bf364 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -7198,18 +7198,42 @@ public class WindowManagerService extends IWindowManager.Stub private float mLatestMouseX; private float mLatestMouseY; - void updatePosition(float x, float y) { + /** + * The display that the pointer (mouse cursor) is currently shown on. This is updated + * directly by InputManagerService when the pointer display changes. + */ + private int mPointerDisplayId = INVALID_DISPLAY; + + /** + * Update the mouse cursor position as a result of a mouse movement. + * @return true if the position was successfully updated, false otherwise. + */ + boolean updatePosition(int displayId, float x, float y) { synchronized (this) { mLatestEventWasMouse = true; + + if (displayId != mPointerDisplayId) { + // The display of the position update does not match the display on which the + // mouse pointer is shown, so do not update the position. + return false; + } mLatestMouseX = x; mLatestMouseY = y; + return true; + } + } + + void setPointerDisplayId(int displayId) { + synchronized (this) { + mPointerDisplayId = displayId; } } @Override public void onPointerEvent(MotionEvent motionEvent) { if (motionEvent.isFromSource(InputDevice.SOURCE_MOUSE)) { - updatePosition(motionEvent.getRawX(), motionEvent.getRawY()); + updatePosition(motionEvent.getDisplayId(), motionEvent.getRawX(), + motionEvent.getRawY()); } else { synchronized (this) { mLatestEventWasMouse = false; @@ -7219,6 +7243,7 @@ public class WindowManagerService extends IWindowManager.Stub }; void updatePointerIcon(IWindow client) { + int pointerDisplayId; float mouseX, mouseY; synchronized(mMousePositionTracker) { @@ -7227,6 +7252,7 @@ public class WindowManagerService extends IWindowManager.Stub } mouseX = mMousePositionTracker.mLatestMouseX; mouseY = mMousePositionTracker.mLatestMouseY; + pointerDisplayId = mMousePositionTracker.mPointerDisplayId; } synchronized (mGlobalLock) { @@ -7243,6 +7269,10 @@ public class WindowManagerService extends IWindowManager.Stub if (displayContent == null) { return; } + if (pointerDisplayId != displayContent.getDisplayId()) { + // Do not let the pointer icon be updated by a window on a different display. + return; + } WindowState windowUnderPointer = displayContent.getTouchableWinAtPointLocked(mouseX, mouseY); if (windowUnderPointer != callingWin) { @@ -7260,7 +7290,11 @@ public class WindowManagerService extends IWindowManager.Stub void restorePointerIconLocked(DisplayContent displayContent, float latestX, float latestY) { // Mouse position tracker has not been getting updates while dragging, update it now. - mMousePositionTracker.updatePosition(latestX, latestY); + if (!mMousePositionTracker.updatePosition( + displayContent.getDisplayId(), latestX, latestY)) { + // The mouse position could not be updated, so ignore this request. + return; + } WindowState windowUnderPointer = displayContent.getTouchableWinAtPointLocked(latestX, latestY); @@ -7284,6 +7318,10 @@ public class WindowManagerService extends IWindowManager.Stub } } + void setMousePointerDisplayId(int displayId) { + mMousePositionTracker.setPointerDisplayId(displayId); + } + /** * Update a tap exclude region in the window identified by the provided id. Touches down on this * region will not: diff --git a/services/core/jni/com_android_server_input_InputManagerService.cpp b/services/core/jni/com_android_server_input_InputManagerService.cpp index 3c5ebe7c62ee8..32adac7f282b0 100644 --- a/services/core/jni/com_android_server_input_InputManagerService.cpp +++ b/services/core/jni/com_android_server_input_InputManagerService.cpp @@ -107,6 +107,7 @@ static struct { jmethodID interceptKeyBeforeDispatching; jmethodID dispatchUnhandledKey; jmethodID checkInjectEventsPermission; + jmethodID onPointerDisplayIdChanged; jmethodID onPointerDownOutsideFocus; jmethodID getVirtualKeyQuietTimeMillis; jmethodID getExcludedDeviceNames; @@ -120,7 +121,6 @@ static struct { jmethodID getLongPressTimeout; jmethodID getPointerLayer; jmethodID getPointerIcon; - jmethodID getPointerDisplayId; jmethodID getKeyboardLayoutOverlay; jmethodID getDeviceAlias; jmethodID getTouchCalibrationForInputDevice; @@ -277,6 +277,7 @@ public: void setFocusedDisplay(int32_t displayId); void setInputDispatchMode(bool enabled, bool frozen); void setSystemUiLightsOut(bool lightsOut); + void setPointerDisplayId(int32_t displayId); void setPointerSpeed(int32_t speed); void setPointerAcceleration(float acceleration); void setInputDeviceEnabled(uint32_t deviceId, bool enabled); @@ -288,7 +289,6 @@ public: void requestPointerCapture(const sp& windowToken, bool enabled); void setCustomPointerIcon(const SpriteIcon& icon); void setMotionClassifierEnabled(bool enabled); - void notifyPointerDisplayIdChanged(); /* --- InputReaderPolicyInterface implementation --- */ @@ -346,6 +346,7 @@ public: std::map* outAnimationResources, int32_t displayId); virtual int32_t getDefaultPointerIconId(); virtual int32_t getCustomPointerIconId(); + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos); private: sp mInputManager; @@ -394,7 +395,6 @@ private: void updateInactivityTimeoutLocked(); void handleInterceptActions(jint wmActions, nsecs_t when, uint32_t& policyFlags); void ensureSpriteControllerLocked(); - int32_t getPointerDisplayId(); sp getParentSurfaceForPointers(int displayId); static bool checkAndClearExceptionFromCallback(JNIEnv* env, const char* methodName); @@ -498,13 +498,9 @@ void NativeInputManager::setDisplayViewports(JNIEnv* env, jobjectArray viewportO } } - // Get the preferred pointer controller displayId. - int32_t pointerDisplayId = getPointerDisplayId(); - { // acquire lock AutoMutex _l(mLock); mLocked.viewports = viewports; - mLocked.pointerDisplayId = pointerDisplayId; std::shared_ptr controller = mLocked.pointerController.lock(); if (controller != nullptr) { controller->onDisplayViewportsUpdated(mLocked.viewports); @@ -666,15 +662,12 @@ std::shared_ptr NativeInputManager::obtainPointerCon return controller; } -int32_t NativeInputManager::getPointerDisplayId() { +void NativeInputManager::onPointerDisplayIdChanged(int32_t pointerDisplayId, float xPos, + float yPos) { JNIEnv* env = jniEnv(); - jint pointerDisplayId = env->CallIntMethod(mServiceObj, - gServiceClassInfo.getPointerDisplayId); - if (checkAndClearExceptionFromCallback(env, "getPointerDisplayId")) { - pointerDisplayId = ADISPLAY_ID_DEFAULT; - } - - return pointerDisplayId; + env->CallVoidMethod(mServiceObj, gServiceClassInfo.onPointerDisplayIdChanged, pointerDisplayId, + xPos, yPos); + checkAndClearExceptionFromCallback(env, "onPointerDisplayIdChanged"); } sp NativeInputManager::getParentSurfaceForPointers(int displayId) { @@ -1032,6 +1025,22 @@ void NativeInputManager::updateInactivityTimeoutLocked() REQUIRES(mLock) { : InactivityTimeout::NORMAL); } +void NativeInputManager::setPointerDisplayId(int32_t displayId) { + { // acquire lock + AutoMutex _l(mLock); + + if (mLocked.pointerDisplayId == displayId) { + return; + } + + ALOGI("Setting pointer display id to %d.", displayId); + mLocked.pointerDisplayId = displayId; + } // release lock + + mInputManager->getReader().requestRefreshConfiguration( + InputReaderConfiguration::CHANGE_DISPLAY_INFO); +} + void NativeInputManager::setPointerSpeed(int32_t speed) { { // acquire lock AutoMutex _l(mLock); @@ -1494,18 +1503,6 @@ void NativeInputManager::setMotionClassifierEnabled(bool enabled) { mInputManager->getClassifier().setMotionClassifierEnabled(enabled); } -void NativeInputManager::notifyPointerDisplayIdChanged() { - int32_t pointerDisplayId = getPointerDisplayId(); - - { // acquire lock - AutoMutex _l(mLock); - mLocked.pointerDisplayId = pointerDisplayId; - } // release lock - - mInputManager->getReader().requestRefreshConfiguration( - InputReaderConfiguration::CHANGE_DISPLAY_INFO); -} - // ---------------------------------------------------------------------------- static NativeInputManager* getNativeInputManager(JNIEnv* env, jobject clazz) { @@ -2199,11 +2196,6 @@ static void nativeNotifyPortAssociationsChanged(JNIEnv* env, jobject nativeImplO InputReaderConfiguration::CHANGE_DISPLAY_INFO); } -static void nativeNotifyPointerDisplayIdChanged(JNIEnv* env, jobject nativeImplObj) { - NativeInputManager* im = getNativeInputManager(env, nativeImplObj); - im->notifyPointerDisplayIdChanged(); -} - static void nativeSetDisplayEligibilityForPointerCapture(JNIEnv* env, jobject nativeImplObj, jint displayId, jboolean isEligible) { NativeInputManager* im = getNativeInputManager(env, nativeImplObj); @@ -2321,6 +2313,11 @@ static void nativeCancelCurrentTouch(JNIEnv* env, jobject nativeImplObj) { im->getInputManager()->getDispatcher().cancelCurrentTouch(); } +static void nativeSetPointerDisplayId(JNIEnv* env, jobject nativeImplObj, jint displayId) { + NativeInputManager* im = getNativeInputManager(env, nativeImplObj); + im->setPointerDisplayId(displayId); +} + // ---------------------------------------------------------------------------- static const JNINativeMethod gInputManagerMethods[] = { @@ -2393,7 +2390,6 @@ static const JNINativeMethod gInputManagerMethods[] = { {"canDispatchToDisplay", "(II)Z", (void*)nativeCanDispatchToDisplay}, {"notifyPortAssociationsChanged", "()V", (void*)nativeNotifyPortAssociationsChanged}, {"changeUniqueIdAssociation", "()V", (void*)nativeChangeUniqueIdAssociation}, - {"notifyPointerDisplayIdChanged", "()V", (void*)nativeNotifyPointerDisplayIdChanged}, {"setDisplayEligibilityForPointerCapture", "(IZ)V", (void*)nativeSetDisplayEligibilityForPointerCapture}, {"setMotionClassifierEnabled", "(Z)V", (void*)nativeSetMotionClassifierEnabled}, @@ -2403,6 +2399,7 @@ static const JNINativeMethod gInputManagerMethods[] = { {"disableSensor", "(II)V", (void*)nativeDisableSensor}, {"flushSensor", "(II)Z", (void*)nativeFlushSensor}, {"cancelCurrentTouch", "()V", (void*)nativeCancelCurrentTouch}, + {"setPointerDisplayId", "(I)V", (void*)nativeSetPointerDisplayId}, }; #define FIND_CLASS(var, className) \ @@ -2498,6 +2495,9 @@ int register_android_server_InputManager(JNIEnv* env) { GET_METHOD_ID(gServiceClassInfo.checkInjectEventsPermission, clazz, "checkInjectEventsPermission", "(II)Z"); + GET_METHOD_ID(gServiceClassInfo.onPointerDisplayIdChanged, clazz, "onPointerDisplayIdChanged", + "(IFF)V"); + GET_METHOD_ID(gServiceClassInfo.onPointerDownOutsideFocus, clazz, "onPointerDownOutsideFocus", "(Landroid/os/IBinder;)V"); @@ -2537,9 +2537,6 @@ int register_android_server_InputManager(JNIEnv* env) { GET_METHOD_ID(gServiceClassInfo.getPointerIcon, clazz, "getPointerIcon", "(I)Landroid/view/PointerIcon;"); - GET_METHOD_ID(gServiceClassInfo.getPointerDisplayId, clazz, - "getPointerDisplayId", "()I"); - GET_METHOD_ID(gServiceClassInfo.getKeyboardLayoutOverlay, clazz, "getKeyboardLayoutOverlay", "(Landroid/hardware/input/InputDeviceIdentifier;)[Ljava/lang/String;"); diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java index 92e7a86876e95..77cbb3a6398c7 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java @@ -19,7 +19,6 @@ package com.android.server.companion.virtual; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -67,7 +66,7 @@ public class InputControllerTest { mInputManagerMockHelper = new InputManagerMockHelper( TestableLooper.get(this), mNativeWrapperMock, mIInputManagerMock); - doNothing().when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); + doReturn(true).when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); LocalServices.removeServiceForTest(InputManagerInternal.class); LocalServices.addService(InputManagerInternal.class, mInputManagerInternalMock); diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java index 22152a1953b92..cbb9fd7c30dd6 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java @@ -180,7 +180,7 @@ public class VirtualDeviceManagerServiceTest { LocalServices.removeServiceForTest(DisplayManagerInternal.class); LocalServices.addService(DisplayManagerInternal.class, mDisplayManagerInternalMock); - doNothing().when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); + doReturn(true).when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); doNothing().when(mInputManagerInternalMock).setPointerAcceleration(anyFloat(), anyInt()); doNothing().when(mInputManagerInternalMock).setPointerIconVisible(anyBoolean(), anyInt()); LocalServices.removeServiceForTest(InputManagerInternal.class); diff --git a/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt new file mode 100644 index 0000000000000..cb97c9bf91a3b --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt @@ -0,0 +1,222 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.input + +import android.content.Context +import android.content.ContextWrapper +import android.hardware.display.DisplayViewport +import android.hardware.input.InputManagerInternal +import android.os.test.TestLooper +import android.platform.test.annotations.Presubmit +import android.view.Display +import androidx.test.InstrumentationRegistry +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +/** + * Tests for {@link InputManagerService}. + * + * Build/Install/Run: + * atest FrameworksServicesTests:InputManagerServiceTests + */ +@Presubmit +class InputManagerServiceTests { + + @get:Rule + val rule = MockitoJUnit.rule()!! + + @Mock + private lateinit var native: NativeInputManagerService + + @Mock + private lateinit var wmCallbacks: InputManagerService.WindowManagerCallbacks + + private lateinit var service: InputManagerService + private lateinit var localService: InputManagerInternal + private lateinit var context: Context + private lateinit var testLooper: TestLooper + + @Before + fun setup() { + context = spy(ContextWrapper(InstrumentationRegistry.getContext())) + testLooper = TestLooper() + service = + InputManagerService(object : InputManagerService.Injector(context, testLooper.looper) { + override fun getNativeService( + service: InputManagerService? + ): NativeInputManagerService { + return native + } + + override fun registerLocalService(service: InputManagerInternal?) { + localService = service!! + } + }) + assertTrue("Local service must be registered", this::localService.isInitialized) + service.setWindowManagerCallbacks(wmCallbacks) + } + + @Test + fun testPointerDisplayUpdatesWhenDisplayViewportsChanged() { + val displayId = 123 + `when`(wmCallbacks.pointerDisplayId).thenReturn(displayId) + val viewports = listOf() + localService.setDisplayViewports(viewports) + verify(native).setDisplayViewports(any(Array::class.java)) + verify(native).setPointerDisplayId(displayId) + + val x = 42f + val y = 314f + service.onPointerDisplayIdChanged(displayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(displayId, x, y) + } + + @Test + fun testSetVirtualMousePointerDisplayId() { + // Set the virtual mouse pointer displayId, and ensure that the calling thread is blocked + // until the native callback happens. + var countDownLatch = CountDownLatch(1) + val overrideDisplayId = 123 + Thread { + assertTrue("Setting virtual pointer display should succeed", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + countDownLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + service.onPointerDisplayIdChanged(overrideDisplayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(overrideDisplayId, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(overrideDisplayId) + + // Ensure that setting the same override again succeeds immediately. + assertTrue("Setting the same virtual mouse pointer displayId again should succeed", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + + // Ensure that we did not query WM for the pointerDisplayId when setting the override + verify(wmCallbacks, never()).pointerDisplayId + + // Unset the virtual mouse pointer displayId, and ensure that we query WM for the new + // pointer displayId and the calling thread is blocked until the native callback happens. + countDownLatch = CountDownLatch(1) + val pointerDisplayId = 42 + `when`(wmCallbacks.pointerDisplayId).thenReturn(pointerDisplayId) + Thread { + assertTrue("Unsetting virtual mouse pointer displayId should succeed", + localService.setVirtualMousePointerDisplayId(Display.INVALID_DISPLAY)) + countDownLatch.countDown() + }.start() + assertFalse("Unsetting virtual mouse pointer displayId should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + service.onPointerDisplayIdChanged(pointerDisplayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(pointerDisplayId, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(pointerDisplayId) + } + + @Test + fun testSetVirtualMousePointerDisplayId_unsuccessfulUpdate() { + // Set the virtual mouse pointer displayId, and ensure that the calling thread is blocked + // until the native callback happens. + val countDownLatch = CountDownLatch(1) + val overrideDisplayId = 123 + Thread { + assertFalse("Setting virtual pointer display should be unsuccessful", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + countDownLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + // Assume the native callback updates the pointerDisplayId to the incorrect value. + service.onPointerDisplayIdChanged(Display.INVALID_DISPLAY, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(Display.INVALID_DISPLAY, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(overrideDisplayId) + } + + @Test + fun testSetVirtualMousePointerDisplayId_competingRequests() { + val firstRequestSyncLatch = CountDownLatch(1) + doAnswer { + firstRequestSyncLatch.countDown() + }.`when`(native).setPointerDisplayId(anyInt()) + + val firstRequestLatch = CountDownLatch(1) + val firstOverride = 123 + Thread { + assertFalse("Setting virtual pointer display from thread 1 should be unsuccessful", + localService.setVirtualMousePointerDisplayId(firstOverride)) + firstRequestLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + firstRequestLatch.await(100, TimeUnit.MILLISECONDS)) + + assertTrue("Wait for first thread's request should succeed", + firstRequestSyncLatch.await(100, TimeUnit.MILLISECONDS)) + + val secondRequestLatch = CountDownLatch(1) + val secondOverride = 42 + Thread { + assertTrue("Setting virtual mouse pointer from thread 2 should be successful", + localService.setVirtualMousePointerDisplayId(secondOverride)) + secondRequestLatch.countDown() + }.start() + assertFalse("Setting virtual mouse pointer should block", + secondRequestLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + // Assume the native callback updates directly to the second request. + service.onPointerDisplayIdChanged(secondOverride, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(secondOverride, x, y) + assertTrue("Native callback unblocks first thread", + firstRequestLatch.await(100, TimeUnit.MILLISECONDS)) + assertTrue("Native callback unblocks second thread", + secondRequestLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native, times(2)).setPointerDisplayId(anyInt()) + } +} \ No newline at end of file