Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 31877e0d authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Decouple UserDataRepository from ImfLock 

UserDataRepository is essentially a concurrent map between userId and
UserData. Decoupling it from ImfLock gives us more code flexibility
without sacrificing correctness and thread safety.

Bug: 352387655
Test: presubmit
Flag: EXEMPT refactor
Change-Id: Iec577722c0d1817a55102f34011db943b9076a4e
parent c173829c

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

+2 −2
Original line number Diff line number Diff line
@@ -345,8 +345,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;

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

+30 −11
Original line number Diff line number Diff line
@@ -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) {

        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();

        }

    }



    @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();

                            }

                        });

                    }

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

+5 −11
Original line number Diff line number Diff line
@@ -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);

        }



        // ... 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 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));

        }

        return collected;

    }