Merge "Decouple UserDataRepository from ImfLock" into main

3 files changed, 37 insertions, 24 deletions

diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
index 1cf591d3999f..731c1ea28ade 100644
--- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
+++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
@@ -346,8 +346,8 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
     private int mCurrentUserId;
 
     /** Holds all user related data */
-    @GuardedBy("ImfLock.class")
-    private UserDataRepository mUserDataRepository;
+    @SharedByAllUsersField
+    private final UserDataRepository mUserDataRepository;
 
     final WindowManagerInternal mWindowManagerInternal;
     private final ActivityManagerInternal mActivityManagerInternal;
diff --git a/services/core/java/com/android/server/inputmethod/UserDataRepository.java b/services/core/java/com/android/server/inputmethod/UserDataRepository.java
index 5cd980f2150b..292f9f8e3501 100644
--- a/services/core/java/com/android/server/inputmethod/UserDataRepository.java
+++ b/services/core/java/com/android/server/inputmethod/UserDataRepository.java
@@ -16,6 +16,7 @@
 
 package com.android.server.inputmethod;
 
+import android.annotation.AnyThread;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
@@ -31,31 +32,46 @@ import com.android.internal.inputmethod.IRemoteAccessibilityInputConnection;
 import com.android.internal.inputmethod.IRemoteInputConnection;
 import com.android.server.pm.UserManagerInternal;
 
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Consumer;
 import java.util.function.IntFunction;
 
 final class UserDataRepository {
 
-    @GuardedBy("ImfLock.class")
+    private final ReentrantReadWriteLock mUserDataLock = new ReentrantReadWriteLock();
+
+    @GuardedBy("mUserDataLock")
     private final SparseArray<UserData> mUserData = new SparseArray<>();
 
     private final IntFunction<InputMethodBindingController> mBindingControllerFactory;
 
-    @GuardedBy("ImfLock.class")
+    @AnyThread
     @NonNull
     UserData getOrCreate(@UserIdInt int userId) {
-        UserData userData = mUserData.get(userId);
-        if (userData == null) {
-            userData = new UserData(userId, mBindingControllerFactory.apply(userId));
-            mUserData.put(userId, userData);
+        mUserDataLock.writeLock().lock();
+        try {
+            UserData userData = mUserData.get(userId);
+            if (userData == null) {
+                userData = new UserData(userId, mBindingControllerFactory.apply(userId));
+                mUserData.put(userId, userData);
+            }
+            return userData;
+        } finally {
+            mUserDataLock.writeLock().unlock();
         }
-        return userData;
     }
 
-    @GuardedBy("ImfLock.class")
+    @AnyThread
     void forAllUserData(Consumer<UserData> consumer) {
-        for (int i = 0; i < mUserData.size(); i++) {
-            consumer.accept(mUserData.valueAt(i));
+        final SparseArray<UserData> copiedArray;
+        mUserDataLock.readLock().lock();
+        try {
+            copiedArray = mUserData.clone();
+        } finally {
+            mUserDataLock.readLock().unlock();
+        }
+        for (int i = 0; i < copiedArray.size(); i++) {
+            consumer.accept(copiedArray.valueAt(i));
         }
     }
 
@@ -69,8 +85,11 @@ final class UserDataRepository {
                     public void onUserRemoved(UserInfo user) {
                         final int userId = user.id;
                         handler.post(() -> {
-                            synchronized (ImfLock.class) {
+                            mUserDataLock.writeLock().lock();
+                            try {
                                 mUserData.remove(userId);
+                            } finally {
+                                mUserDataLock.writeLock().unlock();
                             }
                         });
                     }
diff --git a/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/UserDataRepositoryTest.java b/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/UserDataRepositoryTest.java
index 9ca4f1daa46c..e07a05566f1d 100644
--- a/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/UserDataRepositoryTest.java
+++ b/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/UserDataRepositoryTest.java
@@ -115,10 +115,8 @@ public final class UserDataRepositoryTest {
         final var listener = captor.getValue();
 
         // Add one UserData ...
-        synchronized (ImfLock.class) {
-            final var userData = repository.getOrCreate(ANY_USER_ID);
-            assertThat(userData.mUserId).isEqualTo(ANY_USER_ID);
-        }
+        final var userData = repository.getOrCreate(ANY_USER_ID);
+        assertThat(userData.mUserId).isEqualTo(ANY_USER_ID);
 
         // ... and then call onUserRemoved
         assertThat(collectUserData(repository)).hasSize(1);
@@ -136,10 +134,8 @@ public final class UserDataRepositoryTest {
         final var repository = new UserDataRepository(mHandler,
                 mMockUserManagerInternal, mBindingControllerFactory);
 
-        synchronized (ImfLock.class) {
-            final var userData = repository.getOrCreate(ANY_USER_ID);
-            assertThat(userData.mUserId).isEqualTo(ANY_USER_ID);
-        }
+        final var userData = repository.getOrCreate(ANY_USER_ID);
+        assertThat(userData.mUserId).isEqualTo(ANY_USER_ID);
 
         final var allUserData = collectUserData(repository);
         assertThat(allUserData).hasSize(1);
@@ -151,9 +147,7 @@ public final class UserDataRepositoryTest {
 
     private List<UserDataRepository.UserData> collectUserData(UserDataRepository repository) {
         final var collected = new ArrayList<UserDataRepository.UserData>();
-        synchronized (ImfLock.class) {
-            repository.forAllUserData(userData -> collected.add(userData));
-        }
+        repository.forAllUserData(userData -> collected.add(userData));
         return collected;
     }