Fix PhysicalKeyboardFragment crash bug.

The root cause of crash bug #27749932 is that the state mismatch between
when a Loader is created and when the Loader object finishes background
task.  We can easily reproduce this crash by:
  1. Pair two hardware keyboard A and B.
  2. Open Physical Keyboard settings.
  3. Press the power button to turn off the display.
  4. Move keyboard A far away so that it is unpaired.
  5. Press the power button to turn on the display.
  6. Unlock the device.

One of the reasons PhysicalKeyboardFragment was unstable is that loader
ID reuse.  PhysicalKeyboardFragment starts background data loading
because of many events such as #onResume() and #onInputDeviceAdded() but
there are chances that loader ID was reused because we specified
hardware keyboard device index as the loader ID.  This was dangerous
also because device index can change when a device is added and removed.
With his CL each loader object has an unique ID and
PhysicalKeyboardFragment keeps tracking the list of active Loader IDs
only from which PhysicalKeyboardFragment should accept data.

Also, this CL removes dependencies on PhysicalKeyboardFragment from each
loader object so that we can have a clear boundary of responsibility
between data loader and data consumer.

Bug: 27749932
Change-Id: I53fcb2426d028a492c775bb2b4ec6a5419e33bb4

src/com/android/settings/inputmethod/PhysicalKeyboardFragment.java

@@ -16,6 +16,8 @@
 
 package com.android.settings.inputmethod;
 
+import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.app.Activity;
 import android.app.LoaderManager;
 import android.content.AsyncTaskLoader;
@@ -33,11 +35,13 @@ import android.support.v14.preference.SwitchPreference;
 import android.support.v7.preference.Preference;
 import android.support.v7.preference.Preference.OnPreferenceChangeListener;
 import android.support.v7.preference.PreferenceCategory;
-import android.util.Pair;
+import android.support.v7.preference.PreferenceScreen;
+import android.text.TextUtils;
 import android.view.InputDevice;
 import android.view.inputmethod.InputMethodInfo;
 import android.view.inputmethod.InputMethodManager;
 import android.view.inputmethod.InputMethodSubtype;
+
 import com.android.internal.inputmethod.InputMethodUtils;
 import com.android.internal.logging.MetricsProto.MetricsEvent;
 import com.android.internal.util.Preconditions;
@@ -45,27 +49,33 @@ import com.android.settings.R;
 import com.android.settings.Settings;
 import com.android.settings.SettingsPreferenceFragment;
 
+import libcore.util.Objects;
+
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
+import java.util.HashSet;
-import java.util.Map;
 
 public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
-        implements LoaderManager.LoaderCallbacks<PhysicalKeyboardFragment.Keyboards>,
+        implements InputManager.InputDeviceListener {
-        InputManager.InputDeviceListener {
 
     private static final int USER_SYSTEM = 0;
     private static final String KEYBOARD_ASSISTANCE_CATEGORY = "keyboard_assistance_category";
     private static final String SHOW_VIRTUAL_KEYBOARD_SWITCH = "show_virtual_keyboard_switch";
     private static final String IM_SUBTYPE_MODE_KEYBOARD = "keyboard";
 
-    private final HashMap<Integer, Pair<InputDeviceIdentifier, PreferenceCategory>> mLoaderReference
+    @NonNull
-            = new HashMap<>();
+    private final ArrayList<HardKeyboardDeviceInfo> mLastHardKeyboards = new ArrayList<>();
-    private final Map<InputMethodInfo, List<InputMethodSubtype>> mImiSubtypes = new HashMap<>();
+
+    @NonNull
+    private final HashSet<Integer> mLoaderIDs = new HashSet<>();
+    private int mNextLoaderId = 0;
+
     private InputManager mIm;
-    private InputMethodManager mImm;
+    @NonNull
     private PreferenceCategory mKeyboardAssistanceCategory;
+    @NonNull
     private SwitchPreference mShowVirtualKeyboardSwitch;
+    @NonNull
     private InputMethodUtils.InputMethodSettings mSettings;
 
     @Override
@@ -73,12 +83,11 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
         Activity activity = Preconditions.checkNotNull(getActivity());
         addPreferencesFromResource(R.xml.physical_keyboard_settings);
         mIm = Preconditions.checkNotNull(activity.getSystemService(InputManager.class));
-        mImm = Preconditions.checkNotNull(activity.getSystemService(InputMethodManager.class));
         mSettings = new InputMethodUtils.InputMethodSettings(
                 activity.getResources(),
                 getContentResolver(),
-                new HashMap<String, InputMethodInfo>(),
+                new HashMap<>(),
-                new ArrayList<InputMethodInfo>(),
+                new ArrayList<>(),
                 USER_SYSTEM,
                 false /* copyOnWrite */);
         mKeyboardAssistanceCategory = Preconditions.checkNotNull(
@@ -91,6 +100,8 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
     @Override
     public void onResume() {
         super.onResume();
+        clearLoader();
+        mLastHardKeyboards.clear();
         updateHardKeyboards();
         mIm.registerInputDeviceListener(this, null);
         mShowVirtualKeyboardSwitch.setOnPreferenceChangeListener(
@@ -101,26 +112,23 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
     @Override
     public void onPause() {
         super.onPause();
-        clearHardKeyboardsData();
+        clearLoader();
+        mLastHardKeyboards.clear();
         mIm.unregisterInputDeviceListener(this);
         mShowVirtualKeyboardSwitch.setOnPreferenceChangeListener(null);
         unregisterShowVirtualKeyboardSettingsObserver();
     }
 
-    @Override
+    public void onLoadFinishedInternal(final int loaderId, @NonNull final Keyboards data,
-    public Loader<Keyboards> onCreateLoader(int id, Bundle args) {
+            @NonNull final PreferenceCategory preferenceCategory) {
-        final InputDeviceIdentifier deviceId = mLoaderReference.get(id).first;
+        if (!mLoaderIDs.remove(loaderId)) {
-        return new KeyboardLayoutLoader(
+            // Already destroyed loader.  Ignore.
-                getActivity().getBaseContext(), mIm, mImiSubtypes, deviceId);
+            return;
-    }
+        }
 
-    @Override
+        final InputDeviceIdentifier deviceId = data.mInputDeviceIdentifier;
-    public void onLoadFinished(Loader<Keyboards> loader, Keyboards data) {
+        preferenceCategory.removeAll();
-        // TODO: Investigate why this is being called twice.
+        for (Keyboards.KeyboardInfo info : data.mKeyboardInfoList) {
-        final InputDeviceIdentifier deviceId = mLoaderReference.get(loader.getId()).first;
-        final PreferenceCategory category = mLoaderReference.get(loader.getId()).second;
-        category.removeAll();
-        for (Keyboards.KeyboardInfo info : data.mInfos) {
             Preference pref = new Preference(getPrefContext(), null);
             final InputMethodInfo imi = info.mImi;
             final InputMethodSubtype imSubtype = info.mImSubtype;
@@ -130,21 +138,15 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
                 if (layout != null) {
                     pref.setSummary(layout.getLabel());
                 }
-                pref.setOnPreferenceClickListener(new Preference.OnPreferenceClickListener() {
+                pref.setOnPreferenceClickListener(preference -> {
-                    @Override
+                    showKeyboardLayoutScreen(deviceId, imi, imSubtype);
-                    public boolean onPreferenceClick(Preference preference) {
+                    return true;
-                        showKeyboardLayoutScreen(deviceId, imi, imSubtype);
-                        return true;
-                    }
                 });
-                category.addPreference(pref);
+                preferenceCategory.addPreference(pref);
             }
         }
     }
 
-    @Override
-    public void onLoaderReset(Loader<Keyboards> loader) {}
-
     @Override
     public void onInputDeviceAdded(int deviceId) {
         updateHardKeyboards();
@@ -165,27 +167,42 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
         return MetricsEvent.PHYSICAL_KEYBOARDS;
     }
 
-    private void updateHardKeyboards() {
+    @NonNull
-        clearHardKeyboardsData();
+    private static ArrayList<HardKeyboardDeviceInfo> getHardKeyboards() {
-        loadInputMethodInfoSubtypes();
+        final ArrayList<HardKeyboardDeviceInfo> keyboards = new ArrayList<>();
-        final int[] devices = InputDevice.getDeviceIds();
+        final int[] devicesIds = InputDevice.getDeviceIds();
-        for (int deviceIndex = 0; deviceIndex < devices.length; deviceIndex++) {
+        for (int deviceId : devicesIds) {
-            InputDevice device = InputDevice.getDevice(devices[deviceIndex]);
+            final InputDevice device = InputDevice.getDevice(deviceId);
-            if (device != null
+            if (device != null && !device.isVirtual() && device.isFullKeyboard()) {
-                    && !device.isVirtual()
+                keyboards.add(new HardKeyboardDeviceInfo(device.getName(), device.getIdentifier()));
-                    && device.isFullKeyboard()) {
-                final PreferenceCategory category = new PreferenceCategory(getPrefContext(), null);
-                category.setTitle(device.getName());
-                category.setOrder(0);
-                mLoaderReference.put(deviceIndex, new Pair(device.getIdentifier(), category));
-                getPreferenceScreen().addPreference(category);
             }
         }
-        mKeyboardAssistanceCategory.setOrder(1);
+        return keyboards;
-        getPreferenceScreen().addPreference(mKeyboardAssistanceCategory);
+    }
+
+    private void updateHardKeyboards() {
+        final ArrayList<HardKeyboardDeviceInfo> newHardKeyboards = getHardKeyboards();
+        if (!Objects.equal(newHardKeyboards, mLastHardKeyboards)) {
+            clearLoader();
+            final PreferenceScreen preferenceScreen = getPreferenceScreen();
+            preferenceScreen.removeAll();
+            mLastHardKeyboards.clear();
+            mLastHardKeyboards.addAll(newHardKeyboards);
+            final int N = newHardKeyboards.size();
+            for (int i = 0; i < N; ++i) {
+                final HardKeyboardDeviceInfo deviceInfo = newHardKeyboards.get(i);
+                final PreferenceCategory category = new PreferenceCategory(getPrefContext(), null);
+                category.setTitle(deviceInfo.mDeviceName);
+                category.setOrder(0);
+                getLoaderManager().initLoader(mNextLoaderId, null,
+                        new Callbacks(getContext(), this, deviceInfo.mDeviceIdentifier, category));
+                mLoaderIDs.add(mNextLoaderId);
+                ++mNextLoaderId;
+                preferenceScreen.addPreference(category);
+            }
+            mKeyboardAssistanceCategory.setOrder(1);
+            preferenceScreen.addPreference(mKeyboardAssistanceCategory);
 
-        for (int deviceIndex : mLoaderReference.keySet()) {
-            getLoaderManager().initLoader(deviceIndex, null, this);
         }
         updateShowVirtualKeyboardSwitch();
     }
@@ -203,27 +220,11 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
         startActivity(intent);
     }
 
-    private void clearHardKeyboardsData() {
+    private void clearLoader() {
-        getPreferenceScreen().removeAll();
+        for (final int loaderId : mLoaderIDs) {
-        for (int index = 0; index < mLoaderReference.size(); index++) {
+            getLoaderManager().destroyLoader(loaderId);
-            getLoaderManager().destroyLoader(index);
-        }
-        mLoaderReference.clear();
-    }
-
-    private void loadInputMethodInfoSubtypes() {
-        mImiSubtypes.clear();
-        final List<InputMethodInfo> imis = mImm.getEnabledInputMethodList();
-        for (InputMethodInfo imi : imis) {
-            final List<InputMethodSubtype> subtypes = new ArrayList<>();
-            for (InputMethodSubtype subtype : mImm.getEnabledInputMethodSubtypeList(
-                    imi, true /* allowsImplicitlySelectedSubtypes */)) {
-                if (IM_SUBTYPE_MODE_KEYBOARD.equalsIgnoreCase(subtype.getMode())) {
-                    subtypes.add(subtype);
-                }
-            }
-            mImiSubtypes.put(imi, subtypes);
         }
+        mLoaderIDs.clear();
     }
 
     private void registerShowVirtualKeyboardSettingsObserver() {
@@ -260,44 +261,84 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
         }
     };
 
+    @NonNull
     static String getDisplayName(
-            Context context, InputMethodInfo imi, InputMethodSubtype imSubtype) {
+            @NonNull Context context, @NonNull InputMethodInfo imi,
-        CharSequence imSubtypeName =  imSubtype.getDisplayName(
+            @NonNull InputMethodSubtype imSubtype) {
-                context, imi.getPackageName(),
+        final CharSequence imSubtypeName = imSubtype.getDisplayName(
-                imi.getServiceInfo().applicationInfo);
+                context, imi.getPackageName(), imi.getServiceInfo().applicationInfo);
-        CharSequence imeName = imi.loadLabel(context.getPackageManager());
+        final CharSequence imeName = imi.loadLabel(context.getPackageManager());
         return String.format(
                 context.getString(R.string.physical_device_title), imSubtypeName, imeName);
     }
 
-    private static final class KeyboardLayoutLoader extends AsyncTaskLoader<Keyboards> {
+    private static final class Callbacks
+            implements LoaderManager.LoaderCallbacks<PhysicalKeyboardFragment.Keyboards> {
+        @NonNull
+        final Context mContext;
+        @NonNull
+        final PhysicalKeyboardFragment mPhysicalKeyboardFragment;
+        @NonNull
+        final InputDeviceIdentifier mInputDeviceIdentifier;
+        @NonNull
+        final PreferenceCategory mPreferenceCategory;
+        public Callbacks(
+                @NonNull Context context,
+                @NonNull PhysicalKeyboardFragment physicalKeyboardFragment,
+                @NonNull InputDeviceIdentifier inputDeviceIdentifier,
+                @NonNull PreferenceCategory preferenceCategory) {
+            mContext = context;
+            mPhysicalKeyboardFragment = physicalKeyboardFragment;
+            mInputDeviceIdentifier = inputDeviceIdentifier;
+            mPreferenceCategory = preferenceCategory;
+        }
 
-        private final Map<InputMethodInfo, List<InputMethodSubtype>> mImiSubtypes;
+        @Override
+        public Loader<Keyboards> onCreateLoader(int id, Bundle args) {
+            return new KeyboardLayoutLoader(mContext, mInputDeviceIdentifier);
+        }
+
+        @Override
+        public void onLoadFinished(Loader<Keyboards> loader, Keyboards data) {
+            mPhysicalKeyboardFragment.onLoadFinishedInternal(loader.getId(), data,
+                    mPreferenceCategory);
+        }
+
+        @Override
+        public void onLoaderReset(Loader<Keyboards> loader) {
+        }
+    }
+
+    private static final class KeyboardLayoutLoader extends AsyncTaskLoader<Keyboards> {
+        @NonNull
         private final InputDeviceIdentifier mInputDeviceIdentifier;
-        private final InputManager mIm;
 
         public KeyboardLayoutLoader(
-                Context context,
+                @NonNull Context context,
-                InputManager im,
+                @NonNull InputDeviceIdentifier inputDeviceIdentifier) {
-                Map<InputMethodInfo, List<InputMethodSubtype>> imiSubtypes,
-                InputDeviceIdentifier inputDeviceIdentifier) {
             super(context);
-            mIm = Preconditions.checkNotNull(im);
             mInputDeviceIdentifier = Preconditions.checkNotNull(inputDeviceIdentifier);
-            mImiSubtypes = new HashMap<>(imiSubtypes);
         }
 
         @Override
         public Keyboards loadInBackground() {
-            final Keyboards keyboards = new Keyboards();
+            final ArrayList<Keyboards.KeyboardInfo> keyboardInfoList = new ArrayList<>();
-            for (InputMethodInfo imi : mImiSubtypes.keySet()) {
+            final InputMethodManager imm = getContext().getSystemService(InputMethodManager.class);
-                for (InputMethodSubtype subtype : mImiSubtypes.get(imi)) {
+            final InputManager im = getContext().getSystemService(InputManager.class);
-                    final KeyboardLayout layout = mIm.getKeyboardLayoutForInputDevice(
+            if (imm != null && im != null) {
-                            mInputDeviceIdentifier, imi, subtype);
+                for (InputMethodInfo imi : imm.getEnabledInputMethodList()) {
-                    keyboards.mInfos.add(new Keyboards.KeyboardInfo(imi, subtype, layout));
+                    for (InputMethodSubtype subtype : imm.getEnabledInputMethodSubtypeList(
+                            imi, true /* allowsImplicitlySelectedSubtypes */)) {
+                        if (!IM_SUBTYPE_MODE_KEYBOARD.equalsIgnoreCase(subtype.getMode())) {
+                            continue;
+                        }
+                        final KeyboardLayout layout = im.getKeyboardLayoutForInputDevice(
+                                mInputDeviceIdentifier, imi, subtype);
+                        keyboardInfoList.add(new Keyboards.KeyboardInfo(imi, subtype, layout));
+                    }
                 }
             }
-            return keyboards;
+            return new Keyboards(mInputDeviceIdentifier, keyboardInfoList);
         }
 
         @Override
@@ -313,18 +354,70 @@ public final class PhysicalKeyboardFragment extends SettingsPreferenceFragment
         }
     }
 
-    public static final class Keyboards {
+    public static final class HardKeyboardDeviceInfo {
+        @NonNull
+        public final String mDeviceName;
+        @NonNull
+        public final InputDeviceIdentifier mDeviceIdentifier;
 
-        public final ArrayList<KeyboardInfo> mInfos = new ArrayList<>();
+        public HardKeyboardDeviceInfo(
+                @Nullable final String deviceName,
+                @NonNull final InputDeviceIdentifier deviceIdentifier) {
+            mDeviceName = deviceName != null ? deviceName : "";
+            mDeviceIdentifier = deviceIdentifier;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (o == this) return true;
+            if (o == null) return false;
+
+            if (!(o instanceof HardKeyboardDeviceInfo)) return false;
+
+            final HardKeyboardDeviceInfo that = (HardKeyboardDeviceInfo) o;
+            if (!TextUtils.equals(mDeviceName, that.mDeviceName)) {
+                return false;
+            }
+            if (mDeviceIdentifier.getVendorId() != that.mDeviceIdentifier.getVendorId()) {
+                return false;
+            }
+            if (mDeviceIdentifier.getProductId() != that.mDeviceIdentifier.getProductId()) {
+                return false;
+            }
+            if (!TextUtils.equals(mDeviceIdentifier.getDescriptor(),
+                    that.mDeviceIdentifier.getDescriptor())) {
+                return false;
+            }
+
+            return true;
+        }
+    }
+
+    public static final class Keyboards {
+        @NonNull
+        public final InputDeviceIdentifier mInputDeviceIdentifier;
+        @NonNull
+        public final ArrayList<KeyboardInfo> mKeyboardInfoList;
+
+        public Keyboards(
+                @NonNull final InputDeviceIdentifier inputDeviceIdentifier,
+                @NonNull final ArrayList<KeyboardInfo> keyboardInfoList) {
+            mInputDeviceIdentifier = inputDeviceIdentifier;
+            mKeyboardInfoList = keyboardInfoList;
+        }
 
         public static final class KeyboardInfo {
-
+            @NonNull
             public final InputMethodInfo mImi;
+            @NonNull
             public final InputMethodSubtype mImSubtype;
+            @NonNull
             public final KeyboardLayout mLayout;
 
             public KeyboardInfo(
-                    InputMethodInfo imi, InputMethodSubtype imSubtype, KeyboardLayout layout) {
+                    @NonNull final InputMethodInfo imi,
+                    @NonNull final InputMethodSubtype imSubtype,
+                    @NonNull final KeyboardLayout layout) {
                 mImi = imi;
                 mImSubtype = imSubtype;
                 mLayout = layout;