Merge "Move removeClient to ClientController" into main (f8fc22ce) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/inputmethod/ClientController.java

+51 −10

Original line number	Diff line number	Diff line
		@@ -25,9 +25,13 @@ import android.util.SparseArray;
		import android.view.inputmethod.InputBinding;

		import com.android.internal.annotations.GuardedBy;
		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.inputmethod.IInputMethodClient;
		import com.android.internal.inputmethod.IRemoteInputConnection;

		import java.util.ArrayList;
		import java.util.List;

		/**
		* Store and manage {@link InputMethodManagerService} clients. This class was designed to be a
		* singleton in {@link InputMethodManagerService} since it stores information about all clients,
		@@ -37,9 +41,7 @@ import com.android.internal.inputmethod.IRemoteInputConnection;
		* As part of the re-architecture plan (described in go/imms-rearchitecture-plan), the following
		* fields and methods will be moved out from IMMS and placed here:
		* <ul>
		* <li>mCurClient (ClientState)</li>
		* <li>mClients (ArrayMap of ClientState indexed by IBinder)</li>
		* <li>mLastSwitchUserId</li>
		* </ul>
		* <p>
		* Nested Classes (to move from IMMS):
		@@ -54,7 +56,6 @@ import com.android.internal.inputmethod.IRemoteInputConnection;
		* <li>removeClient</li>
		* <li>verifyClientAndPackageMatch</li>
		* <li>setImeTraceEnabledForAllClients (make it reactive)</li>
		* <li>unbindCurrentClient</li>
		* </ul>
		*/
		// TODO(b/314150112): Update the Javadoc above, by removing the re-architecture steps, once this
		@@ -65,18 +66,32 @@ final class ClientController {
		@GuardedBy("ImfLock.class")
		final ArrayMap<IBinder, ClientState> mClients = new ArrayMap<>();

		@GuardedBy("ImfLock.class")
		private final List<ClientControllerCallback> mCallbacks = new ArrayList<>();

		private final PackageManagerInternal mPackageManagerInternal;

		interface ClientControllerCallback {

		void onClientRemoved(ClientState client);
		}

		ClientController(PackageManagerInternal packageManagerInternal) {
		mPackageManagerInternal = packageManagerInternal;
		}

		@GuardedBy("ImfLock.class")
		void addClient(IInputMethodClientInvoker clientInvoker,
		IRemoteInputConnection inputConnection,
		int selfReportedDisplayId, IBinder.DeathRecipient deathRecipient, int callerUid,
		ClientState addClient(IInputMethodClientInvoker clientInvoker,
		IRemoteInputConnection inputConnection, int selfReportedDisplayId, int callerUid,
		int callerPid) {
		// TODO: Optimize this linear search.
		final IBinder.DeathRecipient deathRecipient = () -> {
		// Exceptionally holding ImfLock here since this is a internal lambda expression.
		synchronized (ImfLock.class) {
		removeClientAsBinder(clientInvoker.asBinder());
		}
		};

		// TODO(b/319457906): Optimize this linear search.
		final int numClients = mClients.size();
		for (int i = 0; i < numClients; ++i) {
		final ClientState state = mClients.valueAt(i);
		@@ -101,14 +116,40 @@ final class ClientController {
		// have the client crash. Thus we do not verify the display ID at all here. Instead we
		// later check the display ID every time the client needs to interact with the specified
		// display.
		mClients.put(clientInvoker.asBinder(), new ClientState(clientInvoker, inputConnection,
		callerUid, callerPid, selfReportedDisplayId, deathRecipient));
		final ClientState cs = new ClientState(clientInvoker, inputConnection,
		callerUid, callerPid, selfReportedDisplayId, deathRecipient);
		mClients.put(clientInvoker.asBinder(), cs);
		return cs;
		}

		@VisibleForTesting
		@GuardedBy("ImfLock.class")
		boolean removeClient(IInputMethodClient client) {
		return removeClientAsBinder(client.asBinder());
		}

		@GuardedBy("ImfLock.class")
		private boolean removeClientAsBinder(IBinder binder) {
		final ClientState cs = mClients.remove(binder);
		if (cs == null) {
		return false;
		}
		binder.unlinkToDeath(cs.mClientDeathRecipient, 0 /* flags */);
		for (int i = 0; i < mCallbacks.size(); i++) {
		mCallbacks.get(i).onClientRemoved(cs);
		}
		return true;
		}

		@GuardedBy("ImfLock.class")
		void addClientControllerCallback(ClientControllerCallback callback) {
		mCallbacks.add(callback);
		}

		@GuardedBy("ImfLock.class")
		boolean verifyClientAndPackageMatch(
		@NonNull IInputMethodClient client, @NonNull String packageName) {
		ClientState cs = mClients.get(client.asBinder());
		final ClientState cs = mClients.get(client.asBinder());
		if (cs == null) {
		throw new IllegalArgumentException("unknown client " + client.asBinder());
		}

services/core/java/com/android/server/inputmethod/InputMethodManagerService.java

+43 −43

Original line number	Diff line number	Diff line
		@@ -478,7 +478,6 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		/**
		* The client that is currently bound to an input method.
		*/
		// TODO(b/314150112): Move this to ClientController.
		@Nullable
		private ClientState mCurClient;

		@@ -1676,7 +1675,11 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub

		mVisibilityStateComputer = new ImeVisibilityStateComputer(this);
		mVisibilityApplier = new DefaultImeVisibilityApplier(this);

		mClientController = new ClientController(mPackageManagerInternal);
		synchronized (ImfLock.class) {
		mClientController.addClientControllerCallback(c -> onClientRemoved(c));
		}

		mPreventImeStartupUnlessTextEditor = mRes.getBoolean(
		com.android.internal.R.bool.config_preventImeStartupUnlessTextEditor);
		@@ -2168,28 +2171,23 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		// actually running.
		final int callerUid = Binder.getCallingUid();
		final int callerPid = Binder.getCallingPid();

		// TODO(b/314150112): Move the death recipient logic to ClientController when moving
		// removeClient method.
		final IBinder.DeathRecipient deathRecipient = () -> removeClient(client);
		final IInputMethodClientInvoker clientInvoker =
		IInputMethodClientInvoker.create(client, mHandler);
		synchronized (ImfLock.class) {
		mClientController.addClient(clientInvoker, inputConnection, selfReportedDisplayId,
		deathRecipient, callerUid, callerPid);
		callerUid, callerPid);
		}
		}

		// TODO(b/314150112): Move this to ClientController.
		void removeClient(IInputMethodClient client) {
		// TODO(b/314150112): Move this method to InputMethodBindingController
		/**
		* Hide the IME if the removed user is the current user.
		*/
		private void onClientRemoved(ClientController.ClientState client) {
		synchronized (ImfLock.class) {
		ClientState cs = mClientController.mClients.remove(client.asBinder());
		if (cs != null) {
		client.asBinder().unlinkToDeath(cs.mClientDeathRecipient, 0 /* flags */);
		clearClientSessionLocked(cs);
		clearClientSessionForAccessibilityLocked(cs);

		if (mCurClient == cs) {
		clearClientSessionLocked(client);
		clearClientSessionForAccessibilityLocked(client);
		if (mCurClient == client) {
		hideCurrentInputLocked(mCurFocusedWindow, null /* statsToken /, 0 / flags */,
		null /* resultReceiver */, SoftInputShowHideReason.HIDE_REMOVE_CLIENT);
		if (mBoundToMethod) {
		@@ -2205,21 +2203,19 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		mBoundToAccessibility = false;
		mCurClient = null;
		}
		if (mCurFocusedWindowClient == cs) {
		if (mCurFocusedWindowClient == client) {
		mCurFocusedWindowClient = null;
		mCurFocusedWindowEditorInfo = null;
		}
		}
		}
		}

		// TODO(b/314150112): Move this to ClientController.
		@GuardedBy("ImfLock.class")
		void unbindCurrentClientLocked(@UnbindReason int unbindClientReason) {
		if (mCurClient != null) {
		if (DEBUG) {
		Slog.v(TAG, "unbindCurrentInputLocked: client="
		+ mCurClient.mClient.asBinder());
		Slog.v(TAG, "unbindCurrentInputLocked: client=" + mCurClient.mClient.asBinder());
		}
		if (mBoundToMethod) {
		mBoundToMethod = false;
		@@ -2312,7 +2308,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		final StartInputInfo info = new StartInputInfo(mSettings.getCurrentUserId(),
		getCurTokenLocked(),
		mCurTokenDisplayId, getCurIdLocked(), startInputReason, restarting,
		UserHandle.getUserId(mCurClient.mUid), mCurClient.mSelfReportedDisplayId,
		UserHandle.getUserId(mCurClient.mUid),
		mCurClient.mSelfReportedDisplayId,
		mCurFocusedWindow, mCurEditorInfo, mCurFocusedWindowSoftInputMode,
		getSequenceNumberLocked());
		mImeTargetWindowMap.put(startInputToken, mCurFocusedWindow);
		@@ -2323,14 +2320,14 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		// same-user scenarios.
		// That said ignoring cross-user scenario will never affect IMEs that do not have
		// INTERACT_ACROSS_USERS(_FULL) permissions, which is actually almost always the case.
		if (mSettings.getCurrentUserId() == UserHandle.getUserId(mCurClient.mUid)) {
		if (mSettings.getCurrentUserId() == UserHandle.getUserId(
		mCurClient.mUid)) {
		mPackageManagerInternal.grantImplicitAccess(mSettings.getCurrentUserId(),
		null /* intent */, UserHandle.getAppId(getCurMethodUidLocked()),
		mCurClient.mUid, true /* direct */);
		}

		@InputMethodNavButtonFlags
		final int navButtonFlags = getInputMethodNavButtonFlagsLocked();
		@InputMethodNavButtonFlags final int navButtonFlags = getInputMethodNavButtonFlagsLocked();
		final SessionState session = mCurClient.mCurSession;
		setEnabledSessionLocked(session);
		session.mMethod.startInput(startInputToken, mCurInputConnection, mCurEditorInfo, restarting,
		@@ -2750,8 +2747,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		&& curMethod.asBinder() == method.asBinder()) {
		if (mCurClient != null) {
		clearClientSessionLocked(mCurClient);
		mCurClient.mCurSession = new SessionState(mCurClient,
		method, session, channel);
		mCurClient.mCurSession = new SessionState(
		mCurClient, method, session, channel);
		InputBindResult res = attachNewInputLocked(
		StartInputReason.SESSION_CREATED_BY_IME, true);
		attachNewAccessibilityLocked(StartInputReason.SESSION_CREATED_BY_IME, true);
		@@ -5776,8 +5773,10 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		// TODO(b/305829876): Implement user ID verification
		if (mCurClient != null) {
		clearClientSessionForAccessibilityLocked(mCurClient, accessibilityConnectionId);
		mCurClient.mAccessibilitySessions.put(accessibilityConnectionId,
		new AccessibilitySessionState(mCurClient, accessibilityConnectionId,
		mCurClient.mAccessibilitySessions.put(
		accessibilityConnectionId,
		new AccessibilitySessionState(mCurClient,
		accessibilityConnectionId,
		session));

		attachNewAccessibilityLocked(StartInputReason.SESSION_CREATED_BY_ACCESSIBILITY,
		@@ -5811,7 +5810,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
		}
		// A11yManagerService unbinds the disabled accessibility service. We don't need
		// to do it here.
		mCurClient.mClient.onUnbindAccessibilityService(getSequenceNumberLocked(),
		mCurClient.mClient.onUnbindAccessibilityService(
		getSequenceNumberLocked(),
		accessibilityConnectionId);
		}
		// We only have sessions when we bound to an input method. Remove this session

services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/ClientControllerTest.java

+73 −7

Original line number	Diff line number	Diff line
		@@ -15,9 +15,16 @@
		*/
		package com.android.server.inputmethod;

		import static com.android.server.inputmethod.ClientController.ClientControllerCallback;
		import static com.android.server.inputmethod.ClientController.ClientState;

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

		import static org.junit.Assert.assertThrows;
		import static org.mockito.Mockito.any;
		import static org.mockito.Mockito.eq;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		import android.content.pm.PackageManagerInternal;
		@@ -38,6 +45,8 @@ import org.junit.Test;
		import org.mockito.Mock;
		import org.mockito.MockitoAnnotations;

		import java.util.concurrent.CountDownLatch;
		import java.util.concurrent.TimeUnit;

		// This test is designed to run on both device and host (Ravenwood) side.
		public final class ClientControllerTest {
		@@ -58,9 +67,6 @@ public final class ClientControllerTest {
		@Mock
		private IRemoteInputConnection mConnection;

		@Mock
		private IBinder.DeathRecipient mDeathRecipient;

		private Handler mHandler;

		private ClientController mController;
		@@ -68,9 +74,10 @@ public final class ClientControllerTest {
		@Before
		public void setUp() {
		MockitoAnnotations.initMocks(this);
		when(mClient.asBinder()).thenReturn((IBinder) mClient);

		mHandler = new Handler(Looper.getMainLooper());
		mController = new ClientController(mMockPackageManagerInternal);
		when(mClient.asBinder()).thenReturn((IBinder) mClient);
		}

		@Test
		@@ -80,18 +87,77 @@ public final class ClientControllerTest {
		var invoker = IInputMethodClientInvoker.create(mClient, mHandler);

		synchronized (ImfLock.class) {
		mController.addClient(invoker, mConnection, ANY_DISPLAY_ID, mDeathRecipient,
		ANY_CALLER_UID, ANY_CALLER_PID);
		mController.addClient(invoker, mConnection, ANY_DISPLAY_ID, ANY_CALLER_UID,
		ANY_CALLER_PID);

		SecurityException thrown = assertThrows(SecurityException.class,
		() -> {
		synchronized (ImfLock.class) {
		mController.addClient(invoker, mConnection, ANY_DISPLAY_ID,
		mDeathRecipient, ANY_CALLER_UID, ANY_CALLER_PID);
		ANY_CALLER_UID, ANY_CALLER_PID);
		}
		});
		assertThat(thrown.getMessage()).isEqualTo(
		"uid=1/pid=1/displayId=0 is already registered");
		}
		}

		@Test
		// TODO(b/314150112): Enable host side mode for this test once b/315544364 is fixed.
		@IgnoreUnderRavenwood(blockedBy = {InputBinding.class, IInputMethodClientInvoker.class})
		public void testAddClient() throws Exception {
		synchronized (ImfLock.class) {
		var invoker = IInputMethodClientInvoker.create(mClient, mHandler);
		var added = mController.addClient(invoker, mConnection, ANY_DISPLAY_ID, ANY_CALLER_UID,
		ANY_CALLER_PID);

		verify(invoker.asBinder()).linkToDeath(any(IBinder.DeathRecipient.class), eq(0));
		assertThat(mController.mClients).containsEntry(invoker.asBinder(), added);
		}
		}

		@Test
		// TODO(b/314150112): Enable host side mode for this test once b/315544364 is fixed.
		@IgnoreUnderRavenwood(blockedBy = {InputBinding.class, IInputMethodClientInvoker.class})
		public void testRemoveClient() {
		var callback = new TestClientControllerCallback();
		ClientState added;
		synchronized (ImfLock.class) {
		mController.addClientControllerCallback(callback);

		var invoker = IInputMethodClientInvoker.create(mClient, mHandler);
		added = mController.addClient(invoker, mConnection, ANY_DISPLAY_ID, ANY_CALLER_UID,
		ANY_CALLER_PID);
		assertThat(mController.mClients).containsEntry(invoker.asBinder(), added);
		assertThat(mController.removeClient(mClient)).isTrue();
		}

		// Test callback
		var removed = callback.waitForRemovedClient(5, TimeUnit.SECONDS);
		assertThat(removed).isSameInstanceAs(added);
		}

		private static class TestClientControllerCallback implements ClientControllerCallback {

		private final CountDownLatch mLatch = new CountDownLatch(1);

		private ClientState mRemoved;

		@Override
		public void onClientRemoved(ClientState removed) {
		mRemoved = removed;
		mLatch.countDown();
		}

		ClientState waitForRemovedClient(long timeout, TimeUnit unit) {
		try {
		assertWithMessage("ClientController callback wasn't called on user removed").that(
		mLatch.await(timeout, unit)).isTrue();
		} catch (InterruptedException e) {
		Thread.currentThread().interrupt();
		throw new IllegalStateException("Unexpected thread interruption", e);
		}
		return mRemoved;
		}
		}
		}