Merge "IMS: Refactor additional display input properties to avoid deadlock" into tm-dev (3c232f03) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/input/InputManagerService.java

+121 −122

Original line number	Diff line number	Diff line
		@@ -146,6 +146,7 @@ import java.util.Locale;
		import java.util.Map;
		import java.util.Objects;
		import java.util.OptionalInt;
		import java.util.function.Consumer;

		/** The system implementation of {@link IInputManager} that manages input devices. */
		public class InputManagerService extends IInputManager.Stub
		@@ -168,6 +169,8 @@ public class InputManagerService extends IInputManager.Stub
		private static final int MSG_POINTER_DISPLAY_ID_CHANGED = 7;

		private static final int DEFAULT_VIBRATION_MAGNITUDE = 192;
		private static final AdditionalDisplayInputProperties
		DEFAULT_ADDITIONAL_DISPLAY_INPUT_PROPERTIES = new AdditionalDisplayInputProperties();

		/**
		* We know the issue and are working to fix it, so suppressing the toast to not annoy
		@@ -281,6 +284,7 @@ public class InputManagerService extends IInputManager.Stub
		// 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.
		// WARNING: Do not call other services outside of input while holding this lock.
		private final Object mAdditionalDisplayInputPropertiesLock = new Object();

		// Forces the PointerController to target a specific display id.
		@@ -299,6 +303,11 @@ public class InputManagerService extends IInputManager.Stub
		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private final SparseArray<AdditionalDisplayInputProperties> mAdditionalDisplayInputProperties =
		new SparseArray<>();
		// This contains the per-display properties that are currently applied by native code. It should
		// be kept in sync with the properties for mRequestedPointerDisplayId.
		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private final AdditionalDisplayInputProperties mCurrentDisplayProperties =
		new AdditionalDisplayInputProperties();
		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private int mIconType = PointerIcon.TYPE_NOT_SPECIFIED;
		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		@@ -571,27 +580,19 @@ public class InputManagerService extends IInputManager.Stub
		}

		private void setDisplayViewportsInternal(List<DisplayViewport> viewports) {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		final DisplayViewport[] vArray = new DisplayViewport[viewports.size()];
		for (int i = viewports.size() - 1; i >= 0; --i) {
		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 =
		mAdditionalDisplayInputProperties.get(mOverriddenPointerDisplayId);
		if (properties != null) {
		updatePointerIconVisibleLocked(properties.pointerIconVisible);
		updatePointerAccelerationLocked(properties.pointerAcceleration);
		return;
		}
		// Attempt to update the pointer display when viewports change when there is no override.
		// Take care to not make calls to window manager while holding internal locks.
		final int pointerDisplayId = mWindowManagerCallbacks.getPointerDisplayId();
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		if (mOverriddenPointerDisplayId == Display.INVALID_DISPLAY) {
		updatePointerDisplayIdLocked(pointerDisplayId);
		}
		updatePointerIconVisibleLocked(
		AdditionalDisplayInputProperties.DEFAULT_POINTER_ICON_VISIBLE);
		updatePointerAccelerationLocked(IInputConstants.DEFAULT_POINTER_ACCELERATION);
		}
		}

		@@ -1743,12 +1744,7 @@ public class InputManagerService extends IInputManager.Stub
		mPointerIconDisplayContext = null;
		}

		synchronized (mAdditionalDisplayInputPropertiesLock) {
		setPointerIconVisible(AdditionalDisplayInputProperties.DEFAULT_POINTER_ICON_VISIBLE,
		displayId);
		setPointerAcceleration(AdditionalDisplayInputProperties.DEFAULT_POINTER_ACCELERATION,
		displayId);
		}
		updateAdditionalDisplayInputProperties(displayId, AdditionalDisplayInputProperties::reset);

		mNative.displayRemoved(displayId);
		}
		@@ -1835,57 +1831,13 @@ public class InputManagerService extends IInputManager.Stub
		}

		private void setPointerAcceleration(float acceleration, int displayId) {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(displayId);
		if (properties == null) {
		properties = new AdditionalDisplayInputProperties();
		mAdditionalDisplayInputProperties.put(displayId, properties);
		}
		properties.pointerAcceleration = acceleration;
		if (properties.allDefaults()) {
		mAdditionalDisplayInputProperties.remove(displayId);
		}
		if (mOverriddenPointerDisplayId == displayId) {
		updatePointerAccelerationLocked(acceleration);
		}
		}
		}

		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private void updatePointerAccelerationLocked(float acceleration) {
		mNative.setPointerAcceleration(acceleration);
		updateAdditionalDisplayInputProperties(displayId,
		properties -> properties.pointerAcceleration = acceleration);
		}

		private void setPointerIconVisible(boolean visible, int displayId) {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(displayId);
		if (properties == null) {
		properties = new AdditionalDisplayInputProperties();
		mAdditionalDisplayInputProperties.put(displayId, properties);
		}
		properties.pointerIconVisible = visible;
		if (properties.allDefaults()) {
		mAdditionalDisplayInputProperties.remove(displayId);
		}
		if (mOverriddenPointerDisplayId == displayId) {
		updatePointerIconVisibleLocked(visible);
		}
		}
		}

		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private void updatePointerIconVisibleLocked(boolean visible) {
		if (visible) {
		if (mIconType == PointerIcon.TYPE_CUSTOM) {
		mNative.setCustomPointerIcon(mIcon);
		} else {
		mNative.setPointerIconType(mIconType);
		}
		} else {
		mNative.setPointerIconType(PointerIcon.TYPE_NULL);
		}
		updateAdditionalDisplayInputProperties(displayId,
		properties -> properties.pointerIconVisible = visible);
		}

		private void registerPointerSpeedSettingObserver() {
		@@ -2023,23 +1975,19 @@ public class InputManagerService extends IInputManager.Stub

		/**
		* 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();
		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private boolean updatePointerDisplayIdLocked(int pointerDisplayId) {
		if (mRequestedPointerDisplayId == pointerDisplayId) {
		return false;
		}
		mRequestedPointerDisplayId = pointerDisplayId;
		mNative.setPointerDisplayId(pointerDisplayId);
		applyAdditionalDisplayInputProperties();
		return true;
		}
		}

		private void handlePointerDisplayIdChanged(PointerDisplayIdChangedArgs args) {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		@@ -2051,25 +1999,23 @@ public class InputManagerService extends IInputManager.Stub
		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;
		private boolean setVirtualMousePointerDisplayIdBlocking(int overrideDisplayId) {
		final boolean isRemovingOverride = overrideDisplayId == Display.INVALID_DISPLAY;

		// Take care to not make calls to window manager while holding internal locks.
		final int resolvedDisplayId = isRemovingOverride
		? mWindowManagerCallbacks.getPointerDisplayId()
		: overrideDisplayId;

		synchronized (mAdditionalDisplayInputPropertiesLock) {
		mOverriddenPointerDisplayId = displayId;
		if (!removingOverride) {
		final AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(displayId);
		if (properties != null) {
		updatePointerAccelerationLocked(properties.pointerAcceleration);
		updatePointerIconVisibleLocked(properties.pointerIconVisible);
		}
		}
		if (!updatePointerDisplayId() && mAcknowledgedPointerDisplayId == displayId) {
		mOverriddenPointerDisplayId = overrideDisplayId;

		if (!updatePointerDisplayIdLocked(resolvedDisplayId)
		&& mAcknowledgedPointerDisplayId == resolvedDisplayId) {
		// The requested pointer display is already set.
		return true;
		}
		if (removingOverride && mAcknowledgedPointerDisplayId == Display.INVALID_DISPLAY) {
		if (isRemovingOverride && 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.
		@@ -2087,7 +2033,7 @@ public class InputManagerService extends IInputManager.Stub
		// 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;
		return isRemovingOverride \|\| mAcknowledgedPointerDisplayId == overrideDisplayId;
		}
		}

		@@ -2393,17 +2339,12 @@ public class InputManagerService extends IInputManager.Stub
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		mIcon = null;
		mIconType = iconType;
		if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) {
		final AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(mOverriddenPointerDisplayId);
		if (properties == null \|\| properties.pointerIconVisible) {
		mNative.setPointerIconType(mIconType);
		}
		} else {

		if (!mCurrentDisplayProperties.pointerIconVisible) return;

		mNative.setPointerIconType(mIconType);
		}
		}
		}

		// Binder call
		@Override
		@@ -2412,19 +2353,12 @@ public class InputManagerService extends IInputManager.Stub
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		mIconType = PointerIcon.TYPE_CUSTOM;
		mIcon = icon;
		if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) {
		final AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(mOverriddenPointerDisplayId);
		if (properties == null \|\| properties.pointerIconVisible) {
		// Only set the icon if it is not currently hidden; otherwise, it will be set
		// once it's no longer hidden.
		mNative.setCustomPointerIcon(mIcon);
		}
		} else {

		if (!mCurrentDisplayProperties.pointerIconVisible) return;

		mNative.setCustomPointerIcon(mIcon);
		}
		}
		}

		/**
		* Add a runtime association between the input port and the display port. This overrides any
		@@ -3852,14 +3786,79 @@ public class InputManagerService extends IInputManager.Stub
		(float) IInputConstants.DEFAULT_POINTER_ACCELERATION;

		// The pointer acceleration for this display.
		public float pointerAcceleration = DEFAULT_POINTER_ACCELERATION;
		public float pointerAcceleration;

		// Whether the pointer icon should be visible or hidden on this display.
		public boolean pointerIconVisible = DEFAULT_POINTER_ICON_VISIBLE;
		public boolean pointerIconVisible;

		AdditionalDisplayInputProperties() {
		reset();
		}

		public boolean allDefaults() {
		return Float.compare(pointerAcceleration, DEFAULT_POINTER_ACCELERATION) == 0
		&& pointerIconVisible == DEFAULT_POINTER_ICON_VISIBLE;
		}

		public void reset() {
		pointerAcceleration = DEFAULT_POINTER_ACCELERATION;
		pointerIconVisible = DEFAULT_POINTER_ICON_VISIBLE;
		}
		}

		private void applyAdditionalDisplayInputProperties() {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(mRequestedPointerDisplayId);
		if (properties == null) properties = DEFAULT_ADDITIONAL_DISPLAY_INPUT_PROPERTIES;
		applyAdditionalDisplayInputPropertiesLocked(properties);
		}
		}

		@GuardedBy("mAdditionalDisplayInputPropertiesLock")
		private void applyAdditionalDisplayInputPropertiesLocked(
		AdditionalDisplayInputProperties properties) {
		// Handle changes to each of the individual properties.
		if (properties.pointerIconVisible != mCurrentDisplayProperties.pointerIconVisible) {
		mCurrentDisplayProperties.pointerIconVisible = properties.pointerIconVisible;
		if (properties.pointerIconVisible) {
		if (mIconType == PointerIcon.TYPE_CUSTOM) {
		Objects.requireNonNull(mIcon);
		mNative.setCustomPointerIcon(mIcon);
		} else {
		mNative.setPointerIconType(mIconType);
		}
		} else {
		mNative.setPointerIconType(PointerIcon.TYPE_NULL);
		}
		}

		if (properties.pointerAcceleration != mCurrentDisplayProperties.pointerAcceleration) {
		mCurrentDisplayProperties.pointerAcceleration = properties.pointerAcceleration;
		mNative.setPointerAcceleration(properties.pointerAcceleration);
		}
		}

		private void updateAdditionalDisplayInputProperties(int displayId,
		Consumer<AdditionalDisplayInputProperties> updater) {
		synchronized (mAdditionalDisplayInputPropertiesLock) {
		AdditionalDisplayInputProperties properties =
		mAdditionalDisplayInputProperties.get(displayId);
		if (properties == null) {
		properties = new AdditionalDisplayInputProperties();
		mAdditionalDisplayInputProperties.put(displayId, properties);
		}
		updater.accept(properties);
		if (properties.allDefaults()) {
		mAdditionalDisplayInputProperties.remove(displayId);
		}
		if (displayId != mRequestedPointerDisplayId) {
		Log.i(TAG, "Not applying additional properties for display " + displayId
		+ " because the pointer is currently targeting display "
		+ mRequestedPointerDisplayId + ".");
		return;
		}
		applyAdditionalDisplayInputPropertiesLocked(properties);
		}
		}
		}

services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt

+60 −1

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@ import org.mockito.ArgumentMatchers.anyInt
		import org.mockito.ArgumentMatchers.eq
		import org.mockito.Mock
		import org.mockito.Mockito.`when`
		import org.mockito.Mockito.clearInvocations
		import org.mockito.Mockito.doAnswer
		import org.mockito.Mockito.never
		import org.mockito.Mockito.spy
		@@ -226,7 +227,8 @@ class InputManagerServiceTests {

		@Test
		fun onDisplayRemoved_resetAllAdditionalInputProperties() {
		localService.setVirtualMousePointerDisplayId(10)
		setVirtualMousePointerDisplayIdAndVerify(10)

		localService.setPointerIconVisible(false, 10)
		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NULL))
		localService.setPointerAcceleration(5f, 10)
		@@ -237,9 +239,66 @@ class InputManagerServiceTests {
		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NOT_SPECIFIED))
		verify(native).setPointerAcceleration(
		eq(IInputConstants.DEFAULT_POINTER_ACCELERATION.toFloat()))
		verifyNoMoreInteractions(native)

		// This call should not block because the virtual mouse pointer override was never removed.
		localService.setVirtualMousePointerDisplayId(10)

		verify(native).setPointerDisplayId(eq(10))
		verifyNoMoreInteractions(native)
		}

		@Test
		fun updateAdditionalInputPropertiesForOverrideDisplay() {
		setVirtualMousePointerDisplayIdAndVerify(10)

		localService.setPointerIconVisible(false, 10)
		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NULL))
		localService.setPointerAcceleration(5f, 10)
		verify(native).setPointerAcceleration(eq(5f))

		localService.setPointerIconVisible(true, 10)
		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NOT_SPECIFIED))
		localService.setPointerAcceleration(1f, 10)
		verify(native).setPointerAcceleration(eq(1f))

		// Verify that setting properties on a different display is not propagated until the
		// pointer is moved to that display.
		localService.setPointerIconVisible(false, 20)
		localService.setPointerAcceleration(6f, 20)
		verifyNoMoreInteractions(native)

		clearInvocations(native)
		setVirtualMousePointerDisplayIdAndVerify(20)

		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NULL))
		verify(native).setPointerAcceleration(eq(6f))
		}

		@Test
		fun setAdditionalInputPropertiesBeforeOverride() {
		localService.setPointerIconVisible(false, 10)
		localService.setPointerAcceleration(5f, 10)

		verifyNoMoreInteractions(native)

		setVirtualMousePointerDisplayIdAndVerify(10)

		verify(native).setPointerIconType(eq(PointerIcon.TYPE_NULL))
		verify(native).setPointerAcceleration(eq(5f))
		}

		private fun setVirtualMousePointerDisplayIdAndVerify(overrideDisplayId: Int) {
		val thread = Thread { localService.setVirtualMousePointerDisplayId(overrideDisplayId) }
		thread.start()

		// Allow some time for the set override call to park while waiting for the native callback.
		Thread.sleep(100 /millis/)
		verify(native).setPointerDisplayId(overrideDisplayId)

		service.onPointerDisplayIdChanged(overrideDisplayId, 0f, 0f)
		testLooper.dispatchNext()
		verify(wmCallbacks).notifyPointerDisplayIdChanged(overrideDisplayId, 0f, 0f)
		thread.join(100 /millis/)
		}
		}