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

Commit 5a647b69 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Remove unnecessary internal lock 

Previously, InputMethodSubtypeSwitchingController has relied on
its own internal lock for #getNextInputMethod and
class has to be invalidated whenever
InputMethodManagerService#mMethodMap is updated, any method of
InputMethodSubtypeSwitchingController should be called under
the global lock of InputMethodManagerService#mMethodMap.

As a consequence, we can conclude that
InputMethodSubtypeSwitchingController does not need its own
internal lock.

This CL also adds additional synchronization blocks into
the constructor of InputMethodManagerService to address the
existing inconsistency that methods with *Locked suffix are
called without the lock actually.

BUG: 7043015
Change-Id: I9d4d3d7232c984432185c10c13fb726a6158cac8
parent 69b43b49

core/java/com/android/internal/inputmethod/InputMethodSubtypeSwitchingController.java

+20 −17
Original line number Diff line number Diff line
@@ -37,6 +37,10 @@ import java.util.TreeMap; 


/**

 * InputMethodSubtypeSwitchingController controls the switching behavior of the subtypes.

 * <p>

 * This class is designed to be used from and only from {@link InputMethodManagerService} by using

 * {@link InputMethodManagerService#mMethodMap} as a global lock.

 * </p>

 */

public class InputMethodSubtypeSwitchingController {

    private static final String TAG = InputMethodSubtypeSwitchingController.class.getSimpleName();
@@ -196,12 +200,11 @@ public class InputMethodSubtypeSwitchingController { 
        }

    }



    private final Object mLock = new Object();

    private final InputMethodSettings mSettings;

    private InputMethodAndSubtypeList mSubtypeList;



    @VisibleForTesting

    public static ImeSubtypeListItem getNextInputMethodImpl(List<ImeSubtypeListItem> imList,

    public static ImeSubtypeListItem getNextInputMethodLockedImpl(List<ImeSubtypeListItem> imList,

            boolean onlyCurrentIme, InputMethodInfo imi, InputMethodSubtype subtype) {

        if (imi == null) {

            return null;
@@ -245,34 +248,34 @@ public class InputMethodSubtypeSwitchingController { 
        return null;

    }



    public InputMethodSubtypeSwitchingController(InputMethodSettings settings) {

    private InputMethodSubtypeSwitchingController(InputMethodSettings settings, Context context) {

        mSettings = settings;

        resetCircularListLocked(context);

    }



    public static InputMethodSubtypeSwitchingController createInstanceLocked(

            InputMethodSettings settings, Context context) {

        return new InputMethodSubtypeSwitchingController(settings, context);

    }



    // TODO: write unit tests for this method and the logic that determines the next subtype

    public void onCommitText(InputMethodInfo imi, InputMethodSubtype subtype) {

    public void onCommitTextLocked(InputMethodInfo imi, InputMethodSubtype subtype) {

        // TODO: Implement this.

    }



    public void resetCircularListLocked(Context context) {

        synchronized(mLock) {

        mSubtypeList = new InputMethodAndSubtypeList(context, mSettings);

    }

    }



    public ImeSubtypeListItem getNextInputMethod(

    public ImeSubtypeListItem getNextInputMethodLocked(

            boolean onlyCurrentIme, InputMethodInfo imi, InputMethodSubtype subtype) {

        synchronized(mLock) {

            return getNextInputMethodImpl(mSubtypeList.getSortedInputMethodAndSubtypeList(),

        return getNextInputMethodLockedImpl(mSubtypeList.getSortedInputMethodAndSubtypeList(),

                onlyCurrentIme, imi, subtype);

    }

    }



    public List<ImeSubtypeListItem> getSortedInputMethodAndSubtypeList(boolean showSubtypes,

    public List<ImeSubtypeListItem> getSortedInputMethodAndSubtypeListLocked(boolean showSubtypes,

            boolean inputShown, boolean isScreenLocked) {

        synchronized(mLock) {

        return mSubtypeList.getSortedInputMethodAndSubtypeList(

                showSubtypes, inputShown, isScreenLocked);

    }

}
 
}

core/tests/inputmethodtests/src/android/os/InputMethodSubtypeSwitchingControllerTest.java

+14 −14
Original line number Diff line number Diff line
@@ -105,44 +105,44 @@ public class InputMethodSubtypeSwitchingControllerTest extends InstrumentationTe 


        // "switchAwareLatinIme/en_US" -> "switchAwareLatinIme/es_US"

        currentIme = imList.get(0);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(1), nextIme);

        // "switchAwareLatinIme/es_US" -> "switchAwareLatinIme/fr"

        currentIme = imList.get(1);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(2), nextIme);

        // "switchAwareLatinIme/fr" -> "switchAwareJapaneseIme/ja_JP"

        currentIme = imList.get(2);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(5), nextIme);

        // "switchAwareJapaneseIme/ja_JP" -> "switchAwareLatinIme/en_US"

        currentIme = imList.get(5);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(0), nextIme);



        // "nonSwitchAwareLatinIme/en_UK" -> "nonSwitchAwareLatinIme/hi"

        currentIme = imList.get(3);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(4), nextIme);

        // "nonSwitchAwareLatinIme/hi" -> "nonSwitchAwareJapaneseIme/ja_JP"

        currentIme = imList.get(4);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(6), nextIme);

        // "nonSwitchAwareJapaneseIme/ja_JP" -> "nonSwitchAwareLatinIme/en_UK"

        currentIme = imList.get(6);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(3), nextIme);
@@ -158,46 +158,46 @@ public class InputMethodSubtypeSwitchingControllerTest extends InstrumentationTe 


        // "switchAwareLatinIme/en_US" -> "switchAwareLatinIme/es_US"

        currentIme = imList.get(0);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(1), nextIme);

        // "switchAwareLatinIme/es_US" -> "switchAwareLatinIme/fr"

        currentIme = imList.get(1);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(2), nextIme);

        // "switchAwareLatinIme/fr" -> "switchAwareLatinIme/en_US"

        currentIme = imList.get(2);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(0), nextIme);



        // "nonSwitchAwareLatinIme/en_UK" -> "nonSwitchAwareLatinIme/hi"

        currentIme = imList.get(3);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(4), nextIme);

        // "nonSwitchAwareLatinIme/hi" -> "switchAwareLatinIme/en_UK"

        currentIme = imList.get(4);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertEquals(imList.get(3), nextIme);



        // "switchAwareJapaneseIme/ja_JP" -> null

        currentIme = imList.get(5);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertNull(nextIme);



        // "nonSwitchAwareJapaneseIme/ja_JP" -> null

        currentIme = imList.get(6);

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodImpl(

        nextIme = InputMethodSubtypeSwitchingController.getNextInputMethodLockedImpl(

                imList, ONLY_CURRENT_IME, currentIme.mImi, createDummySubtype(

                        currentIme.mSubtypeName.toString()));

        assertNull(nextIme);

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

+22 −12
Original line number Diff line number Diff line
@@ -692,8 +692,10 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
                mRes, context.getContentResolver(), mMethodMap, mMethodList, userId);

        updateCurrentProfileIds();

        mFileManager = new InputMethodFileManager(mMethodMap, userId);

        mSwitchingController = new InputMethodSubtypeSwitchingController(mSettings);

        mSwitchingController.resetCircularListLocked(context);

        synchronized (mMethodMap) {

            mSwitchingController = InputMethodSubtypeSwitchingController.createInstanceLocked(

                    mSettings, context);

        }



        // Just checking if defaultImiId is empty or not

        final String defaultImiId = mSettings.getSelectedInputMethod();
@@ -702,17 +704,23 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
        }

        mImeSelectedOnBoot = !TextUtils.isEmpty(defaultImiId);



        synchronized (mMethodMap) {

            buildInputMethodListLocked(mMethodList, mMethodMap,

                    !mImeSelectedOnBoot /* resetDefaultEnabledIme */);

        }

        mSettings.enableAllIMEsIfThereIsNoEnabledIME();



        if (!mImeSelectedOnBoot) {

            Slog.w(TAG, "No IME selected. Choose the most applicable IME.");

            synchronized (mMethodMap) {

                resetDefaultImeLocked(context);

            }

        }



        mSettingsObserver = new SettingsObserver(mHandler);

        synchronized (mMethodMap) {

            updateFromSettingsLocked(true);

        }



        // IMMS wants to receive Intent.ACTION_LOCALE_CHANGED in order to update the current IME

        // according to the new system locale.
@@ -2174,7 +2182,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
            return false;

        }

        synchronized (mMethodMap) {

            final ImeSubtypeListItem nextSubtype = mSwitchingController.getNextInputMethod(

            final ImeSubtypeListItem nextSubtype = mSwitchingController.getNextInputMethodLocked(

                    onlyCurrentIme, mMethodMap.get(mCurMethodId), mCurrentSubtype);

            if (nextSubtype == null) {

                return false;
@@ -2190,7 +2198,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
            return false;

        }

        synchronized (mMethodMap) {

            final ImeSubtypeListItem nextSubtype = mSwitchingController.getNextInputMethod(

            final ImeSubtypeListItem nextSubtype = mSwitchingController.getNextInputMethodLocked(

                    false /* onlyCurrentIme */, mMethodMap.get(mCurMethodId), mCurrentSubtype);

            if (nextSubtype == null) {

                return false;
@@ -2273,9 +2281,11 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
        if (DEBUG) {

            Slog.d(TAG, "Got the notification of commitText");

        }

        synchronized (mMethodMap) {

            final InputMethodInfo imi = mMethodMap.get(mCurMethodId);

            if (imi != null) {

            mSwitchingController.onCommitText(imi, mCurrentSubtype);

                mSwitchingController.onCommitTextLocked(imi, mCurrentSubtype);

            }

        }

    }
@@ -2698,7 +2708,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
            hideInputMethodMenuLocked();



            final List<ImeSubtypeListItem> imList =

                    mSwitchingController.getSortedInputMethodAndSubtypeList(

                    mSwitchingController.getSortedInputMethodAndSubtypeListLocked(

                            showSubtypes, mInputShown, isScreenLocked);



            if (lastInputMethodSubtypeId == NOT_A_SUBTYPE_ID) {