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
diff --git a/core/java/com/android/internal/inputmethod/InputMethodSubtypeSwitchingController.java b/core/java/com/android/internal/inputmethod/InputMethodSubtypeSwitchingController.java
index 6c4cb71..0f10b2b 100644
--- a/core/java/com/android/internal/inputmethod/InputMethodSubtypeSwitchingController.java
+++ b/core/java/com/android/internal/inputmethod/InputMethodSubtypeSwitchingController.java
@@ -37,6 +37,10 @@
/**
* 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 @@
}
}
- 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 @@
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);
- }
+ mSubtypeList = new InputMethodAndSubtypeList(context, mSettings);
}
- public ImeSubtypeListItem getNextInputMethod(
+ public ImeSubtypeListItem getNextInputMethodLocked(
boolean onlyCurrentIme, InputMethodInfo imi, InputMethodSubtype subtype) {
- synchronized(mLock) {
- return getNextInputMethodImpl(mSubtypeList.getSortedInputMethodAndSubtypeList(),
- onlyCurrentIme, imi, subtype);
- }
+ 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);
- }
+ return mSubtypeList.getSortedInputMethodAndSubtypeList(
+ showSubtypes, inputShown, isScreenLocked);
}
}
diff --git a/core/tests/inputmethodtests/src/android/os/InputMethodSubtypeSwitchingControllerTest.java b/core/tests/inputmethodtests/src/android/os/InputMethodSubtypeSwitchingControllerTest.java
index 2a4d921..0f343b1 100644
--- a/core/tests/inputmethodtests/src/android/os/InputMethodSubtypeSwitchingControllerTest.java
+++ b/core/tests/inputmethodtests/src/android/os/InputMethodSubtypeSwitchingControllerTest.java
@@ -105,44 +105,44 @@
// "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 @@
// "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);
diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java
index 2d270e7..fb69c86 100644
--- a/services/core/java/com/android/server/InputMethodManagerService.java
+++ b/services/core/java/com/android/server/InputMethodManagerService.java
@@ -692,8 +692,10 @@
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 @@
}
mImeSelectedOnBoot = !TextUtils.isEmpty(defaultImiId);
- buildInputMethodListLocked(mMethodList, mMethodMap,
- !mImeSelectedOnBoot /* resetDefaultEnabledIme */);
+ synchronized (mMethodMap) {
+ buildInputMethodListLocked(mMethodList, mMethodMap,
+ !mImeSelectedOnBoot /* resetDefaultEnabledIme */);
+ }
mSettings.enableAllIMEsIfThereIsNoEnabledIME();
if (!mImeSelectedOnBoot) {
Slog.w(TAG, "No IME selected. Choose the most applicable IME.");
- resetDefaultImeLocked(context);
+ synchronized (mMethodMap) {
+ resetDefaultImeLocked(context);
+ }
}
mSettingsObserver = new SettingsObserver(mHandler);
- updateFromSettingsLocked(true);
+ 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 @@
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 @@
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 @@
if (DEBUG) {
Slog.d(TAG, "Got the notification of commitText");
}
- final InputMethodInfo imi = mMethodMap.get(mCurMethodId);
- if (imi != null) {
- mSwitchingController.onCommitText(imi, mCurrentSubtype);
+ synchronized (mMethodMap) {
+ final InputMethodInfo imi = mMethodMap.get(mCurMethodId);
+ if (imi != null) {
+ mSwitchingController.onCommitTextLocked(imi, mCurrentSubtype);
+ }
}
}
@@ -2698,7 +2708,7 @@
hideInputMethodMenuLocked();
final List<ImeSubtypeListItem> imList =
- mSwitchingController.getSortedInputMethodAndSubtypeList(
+ mSwitchingController.getSortedInputMethodAndSubtypeListLocked(
showSubtypes, mInputShown, isScreenLocked);
if (lastInputMethodSubtypeId == NOT_A_SUBTYPE_ID) {