From 5a647b69be8ac8d40c33ed9abe63e41514699e5b Mon Sep 17 00:00:00 2001
From: Yohei Yukawa <yukawa@google.com>
Date: Thu, 22 May 2014 12:49:00 +0900
Subject: 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
---
 .../android/server/InputMethodManagerService.java  | 34 ++++++++++++++--------
 1 file changed, 22 insertions(+), 12 deletions(-)

(limited to 'services')

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 @@ 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);
 
-        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 @@ 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");
         }
-        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 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
             hideInputMethodMenuLocked();
 
             final List<ImeSubtypeListItem> imList =
-                    mSwitchingController.getSortedInputMethodAndSubtypeList(
+                    mSwitchingController.getSortedInputMethodAndSubtypeListLocked(
                             showSubtypes, mInputShown, isScreenLocked);
 
             if (lastInputMethodSubtypeId == NOT_A_SUBTYPE_ID) {
-- 
cgit v1.1