From fbc5547e5f02c4f4ee1c2a0fa71de1ac96d9eb0f Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Mon, 19 Nov 2018 21:54:10 -0800 Subject: [PATCH] Remove dependency on TreeMap from getSortedInputMethodAndSubtypeList() InputMethodAndSubtypeList#getSortedInputMethodAndSubtypeList() currently sorts ImeSubtypeListItem as follows: 1. Create a sorted list of IMEs by TreeMap with the sorting key imi.loadLabel(mPm) + "/" + imi.getId() 2. Generate list of ImeSubtypeListItem objects. 3. Do stable sort ImeSubtypeListItem lists with the following order mImeName mIsSystemLocale mIsSystemLanguage mSubtypeName 4. Return the sorted ImeSubtypeListItem list. Since in the step 2 everything is done sequentially and the result for each IME will not be affected by other IMEs, we can in theory unify the step 1 into the step 3 so that we can perform sorting only once. In order to avoid unnecessary behavior change in this CL, with this CL the step 1 will be removed and the sorting order in the step 3 will be updated as follows: mImeName mIsSystemLocale mIsSystemLanguage mSubtypeName mImi.getId() This should be sufficient to preserve the current behavior because: a) if two ImeSubtypeListItem items were not classified as the same in the step 3, the step 1 basically did not matter and, b) if two ImeSubtypeListItem items were classified as the same in the step 3, those two items would have preserved the sort order in the step 1. Here mImeName == imi.loadLabel(mPm) so if this is the case only mImi.getId() matters in this scenario. Therefore this CL should not have any user-visible behavior change. Fix: 64612471 Test: atest InputMethodSubtypeSwitchingControllerTest Change-Id: I4204275f1fc2040d98b9c55a38ab8b8d50e65126 --- ...InputMethodSubtypeSwitchingController.java | 31 +++++-------------- ...tMethodSubtypeSwitchingControllerTest.java | 18 ++++++++--- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/services/core/java/com/android/server/inputmethod/InputMethodSubtypeSwitchingController.java b/services/core/java/com/android/server/inputmethod/InputMethodSubtypeSwitchingController.java index 77b2fee82dc8a..27c59d462b8e2 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodSubtypeSwitchingController.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodSubtypeSwitchingController.java @@ -31,13 +31,11 @@ import com.android.server.inputmethod.InputMethodUtils.InputMethodSettings; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Objects; -import java.util.TreeMap; /** * InputMethodSubtypeSwitchingController controls the switching behavior of the subtypes. @@ -113,6 +111,7 @@ final class InputMethodSubtypeSwitchingController { *
  • {@link #mIsSystemLocale}
    • *
  • {@link #mIsSystemLanguage}
    • *
  • {@link #mSubtypeName}
    • + *
  • {@link #mImi} with {@link InputMethodInfo#getId()}
    • * * Note: this class has a natural ordering that is inconsistent with {@link #equals(Object). * This method doesn't compare {@link #mSubtypeId} but {@link #equals(Object)} does. @@ -137,7 +136,11 @@ final class InputMethodSubtypeSwitchingController { if (result != 0) { return result; } - return compareNullableCharSequences(mSubtypeName, other.mSubtypeName); + result = compareNullableCharSequences(mSubtypeName, other.mSubtypeName); + if (result != 0) { + return result; + } + return mImi.getId().compareTo(other.mImi.getId()); } @Override @@ -179,24 +182,6 @@ final class InputMethodSubtypeSwitchingController { mSystemLocaleStr = locale != null ? locale.toString() : ""; } - private final TreeMap> mSortedImmis = - new TreeMap<>( - new Comparator() { - @Override - public int compare(InputMethodInfo imi1, InputMethodInfo imi2) { - if (imi2 == null) - return 0; - if (imi1 == null) - return 1; - if (mPm == null) { - return imi1.getId().compareTo(imi2.getId()); - } - CharSequence imiId1 = imi1.loadLabel(mPm) + "/" + imi1.getId(); - CharSequence imiId2 = imi2.loadLabel(mPm) + "/" + imi2.getId(); - return imiId1.toString().compareTo(imiId2.toString()); - } - }); - public List getSortedInputMethodAndSubtypeList( boolean includeAuxiliarySubtypes, boolean isScreenLocked) { final ArrayList imList = new ArrayList<>(); @@ -212,9 +197,7 @@ final class InputMethodSubtypeSwitchingController { } includeAuxiliarySubtypes = false; } - mSortedImmis.clear(); - mSortedImmis.putAll(immis); - for (InputMethodInfo imi : mSortedImmis.keySet()) { + for (InputMethodInfo imi : immis.keySet()) { if (imi == null) { continue; } diff --git a/services/tests/servicestests/src/com/android/server/inputmethod/InputMethodSubtypeSwitchingControllerTest.java b/services/tests/servicestests/src/com/android/server/inputmethod/InputMethodSubtypeSwitchingControllerTest.java index 9424461849186..6ab48e53648ca 100644 --- a/services/tests/servicestests/src/com/android/server/inputmethod/InputMethodSubtypeSwitchingControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/inputmethod/InputMethodSubtypeSwitchingControllerTest.java @@ -375,20 +375,30 @@ public class InputMethodSubtypeSwitchingControllerTest { final ComponentName imeZ1 = new ComponentName("com.example.imeZ", "Ime1"); { final List items = Arrays.asList( - // Subtypes of IME "X". + // Subtypes of two IMEs that have the same display name "X". // Subtypes that has the same locale of the system's. createDummyItem(imeX1, "X", "E", "en_US", 0, "en_US"), + createDummyItem(imeX2, "X", "E", "en_US", 0, "en_US"), createDummyItem(imeX1, "X", "Z", "en_US", 3, "en_US"), + createDummyItem(imeX2, "X", "Z", "en_US", 3, "en_US"), createDummyItem(imeX1, "X", "", "en_US", 6, "en_US"), + createDummyItem(imeX2, "X", "", "en_US", 6, "en_US"), // Subtypes that has the same language of the system's. createDummyItem(imeX1, "X", "E", "en", 1, "en_US"), + createDummyItem(imeX2, "X", "E", "en", 1, "en_US"), createDummyItem(imeX1, "X", "Z", "en", 4, "en_US"), + createDummyItem(imeX2, "X", "Z", "en", 4, "en_US"), createDummyItem(imeX1, "X", "", "en", 7, "en_US"), + createDummyItem(imeX2, "X", "", "en", 7, "en_US"), // Subtypes that has different language than the system's. createDummyItem(imeX1, "X", "A", "hi_IN", 27, "en_US"), + createDummyItem(imeX2, "X", "A", "hi_IN", 27, "en_US"), createDummyItem(imeX1, "X", "E", "ja", 2, "en_US"), + createDummyItem(imeX2, "X", "E", "ja", 2, "en_US"), createDummyItem(imeX1, "X", "Z", "ja", 5, "en_US"), + createDummyItem(imeX2, "X", "Z", "ja", 5, "en_US"), createDummyItem(imeX1, "X", "", "ja", 8, "en_US"), + createDummyItem(imeX2, "X", "", "ja", 8, "en_US"), // Subtypes of IME "Y". // Subtypes that has the same locale of the system's. @@ -449,11 +459,11 @@ public class InputMethodSubtypeSwitchingControllerTest { } { - // Currently ComponentName is not used for sorting. + // Check if ComponentName is also taken into account when comparing two items. final ImeSubtypeListItem ime1 = createDummyItem(imeX1, "X", "A", "ja_JP", 0, "en_US"); final ImeSubtypeListItem ime2 = createDummyItem(imeX2, "X", "A", "ja_JP", 0, "en_US"); - assertTrue(ime1.compareTo(ime2) == 0); - assertTrue(ime2.compareTo(ime1) == 0); + assertTrue(ime1.compareTo(ime2) < 0); + assertTrue(ime2.compareTo(ime1) > 0); // But those aren't equal to each other. assertFalse(ime1.equals(ime2)); assertFalse(ime2.equals(ime1));