Notification Light: Fix apps with same name not showing up

Applications with the same name didn't show up in the
notification light add-app-dialog (with Facebook's "Messenager"
and Google's "Messenager" being the prominent example here).

This happend due to the fact that the list of applications
was sorted by names only and inserts with collisions were
dropped.

This commit refactor notification light settings to use packages
instead of application semantics. Further the list of packages
is now ordered by human readable name followed by package name,
which avoids the collision.

Additionally we handle situtations gracefully where a package
has more than one launcher activity (Again for example,
Google's "Messenger" and "Google+" is in the same package).

Bonus: potential race condition in the list adapter was
fixed, concurrent semantics were emphasized, unused imports
removed.

Change-Id: I87ad32f535ca866883df337432d1b0cecc9485a8

1 files changed, 137 insertions, 136 deletions

diff --git a/src/com/android/settings/notificationlight/NotificationLightSettings.java b/src/com/android/settings/notificationlight/NotificationLightSettings.java
index b189f23..b726a68 100644
--- a/src/com/android/settings/notificationlight/NotificationLightSettings.java
+++ b/src/com/android/settings/notificationlight/NotificationLightSettings.java
@@ -16,24 +16,12 @@
 
 package com.android.settings.notificationlight;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
-
 import android.app.AlertDialog;
 import android.app.Dialog;
-import android.app.Notification;
-import android.app.NotificationManager;
-import android.content.BroadcastReceiver;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.content.Intent;
-import android.content.IntentFilter;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
@@ -50,6 +38,7 @@ import android.preference.PreferenceScreen;
 import android.provider.Settings;
 import android.telephony.TelephonyManager;
 import android.text.TextUtils;
+import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuInflater;
@@ -62,11 +51,18 @@ import android.widget.BaseAdapter;
 import android.widget.ImageView;
 import android.widget.ListView;
 import android.widget.TextView;
-import android.widget.Toast;
 
 import com.android.settings.R;
 import com.android.settings.SettingsPreferenceFragment;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeSet;
+
 public class NotificationLightSettings extends SettingsPreferenceFragment implements
         Preference.OnPreferenceChangeListener, AdapterView.OnItemLongClickListener {
     private static final String TAG = "NotificationLightSettings";
@@ -74,7 +70,6 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
     private static final String NOTIFICATION_LIGHT_PULSE_DEFAULT_LED_ON = "notification_light_pulse_default_led_on";
     private static final String NOTIFICATION_LIGHT_PULSE_DEFAULT_LED_OFF = "notification_light_pulse_default_led_off";
     private static final String NOTIFICATION_LIGHT_PULSE_CUSTOM_ENABLE = "notification_light_pulse_custom_enable";
-    private static final String NOTIFICATION_LIGHT_PULSE_CUSTOM_VALUES = "notification_light_pulse_custom_values";
     private static final String NOTIFICATION_LIGHT_PULSE = "notification_light_pulse";
     private static final String NOTIFICATION_LIGHT_PULSE_CALL_COLOR = "notification_light_pulse_call_color";
     private static final String NOTIFICATION_LIGHT_PULSE_CALL_LED_ON = "notification_light_pulse_call_led_on";
@@ -94,20 +89,19 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
     private int mDefaultColor;
     private int mDefaultLedOn;
     private int mDefaultLedOff;
-    private List<ResolveInfo> mInstalledApps;
     private PackageManager mPackageManager;
     private boolean mCustomEnabled;
     private boolean mLightEnabled;
     private boolean mVoiceCapable;
-    private PreferenceGroup mAppList;
+    private PreferenceGroup mApplicationPrefList;
     private ApplicationLightPreference mDefaultPref;
     private ApplicationLightPreference mCallPref;
     private ApplicationLightPreference mVoicemailPref;
     private CheckBoxPreference mCustomEnabledPref;
     private Menu mMenu;
-    AppAdapter mAppAdapter;
-    private String mApplicationList;
-    private Map<String, Application> mApplications;
+    private PackageAdapter mPackageAdapter;
+    private String mPackageList;
+    private Map<String, Package> mPackages;
 
     @Override
     public void onCreate(Bundle savedInstanceState) {
@@ -124,13 +118,9 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
 
         // Get launch-able applications
         mPackageManager = getPackageManager();
-        final Intent mainIntent = new Intent(Intent.ACTION_MAIN, null);
-        mainIntent.addCategory(Intent.CATEGORY_LAUNCHER);
-        mInstalledApps = mPackageManager.queryIntentActivities(mainIntent, 0);
-        mAppAdapter = new AppAdapter(mInstalledApps);
-        mAppAdapter.update();
+        mPackageAdapter = new PackageAdapter();
 
-        mApplications = new HashMap<String, Application>();
+        mPackages = new HashMap<String, Package>();
 
         // Determine if the device has voice capabilities
         TelephonyManager tm = (TelephonyManager) getActivity().getSystemService(Context.TELEPHONY_SERVICE);
@@ -143,7 +133,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
     public void onResume() {
         super.onResume();
         refreshDefault();
-        refreshCustomApplications();
+        refreshCustomApplicationPrefs();
         setCustomEnabled();
         getListView().setOnItemLongClickListener(this);
     }
@@ -206,48 +196,39 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
             }
         }
 
-        mAppList = (PreferenceGroup) prefSet.findPreference("applications_list");
+        mApplicationPrefList = (PreferenceGroup) prefSet.findPreference("applications_list");
+        mApplicationPrefList.setOrderingAsAdded(false);
     }
 
-    private void refreshCustomApplications() {
+    private void refreshCustomApplicationPrefs() {
         Context context = getActivity();
 
-        if (!parseApplicationList()) {
+        if (!parsePackageList()) {
             return;
         }
 
         // Add the Application Preferences
-        final PreferenceScreen prefSet = getPreferenceScreen();
-        final PackageManager pm = getPackageManager();
-
-        if (mAppList != null) {
-            final Map<CharSequence, ApplicationLightPreference> prefs =
-                    new TreeMap<CharSequence, ApplicationLightPreference>();
+        if (mApplicationPrefList != null) {
+            mApplicationPrefList.removeAll();
 
-            mAppList.removeAll();
-
-            for (Application i : mApplications.values()) {
+            for (Package pkg : mPackages.values()) {
                 try {
-                    PackageInfo info = pm.getPackageInfo(i.name, PackageManager.GET_META_DATA);
+                    PackageInfo info = mPackageManager.getPackageInfo(pkg.name,
+                            PackageManager.GET_META_DATA);
                     ApplicationLightPreference pref =
-                            new ApplicationLightPreference(context, i.color, i.timeon, i.timeoff);
-                    final CharSequence label = info.applicationInfo.loadLabel(pm);
+                            new ApplicationLightPreference(context, pkg.color, pkg.timeon, pkg.timeoff);
 
-                    pref.setKey(i.name);
-                    pref.setTitle(label);
-                    pref.setIcon(info.applicationInfo.loadIcon(pm));
+                    pref.setKey(pkg.name);
+                    pref.setTitle(info.applicationInfo.loadLabel(mPackageManager));
+                    pref.setIcon(info.applicationInfo.loadIcon(mPackageManager));
                     pref.setPersistent(false);
                     pref.setOnPreferenceChangeListener(this);
 
-                    prefs.put(label, pref);
+                    mApplicationPrefList.addPreference(pref);
                 } catch (NameNotFoundException e) {
                     // Do nothing
                 }
             }
-
-            for (ApplicationLightPreference pref : prefs.values()) {
-                mAppList.addPreference(pref);
-            }
         }
     }
 
@@ -261,39 +242,39 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
         }
 
         // Custom applications
-        if (mAppList != null) {
-            mAppList.setEnabled(enabled);
+        if (mApplicationPrefList != null) {
+            mApplicationPrefList.setEnabled(enabled);
             setHasOptionsMenu(enabled);
         }
     }
 
-    private void addCustomApplication(String packageName) {
-        Application app = mApplications.get(packageName);
-        if (app == null) {
-            app = new Application(packageName, mDefaultColor, mDefaultLedOn, mDefaultLedOff);
-            mApplications.put(packageName, app);
-            saveApplicationList(false);
-            refreshCustomApplications();
+    private void addCustomApplicationPref(String packageName) {
+        Package pkg = mPackages.get(packageName);
+        if (pkg == null) {
+            pkg = new Package(packageName, mDefaultColor, mDefaultLedOn, mDefaultLedOff);
+            mPackages.put(packageName, pkg);
+            savePackageList(false);
+            refreshCustomApplicationPrefs();
         }
     }
 
-    private void removeCustomApplication(String packageName) {
-        if (mApplications.remove(packageName) != null) {
-            saveApplicationList(false);
-            refreshCustomApplications();
+    private void removeCustomApplicationPref(String packageName) {
+        if (mPackages.remove(packageName) != null) {
+            savePackageList(false);
+            refreshCustomApplicationPrefs();
         }
     }
 
-    private boolean parseApplicationList() {
+    private boolean parsePackageList() {
         final String baseString = Settings.System.getString(getContentResolver(),
                 Settings.System.NOTIFICATION_LIGHT_PULSE_CUSTOM_VALUES);
 
-        if (TextUtils.equals(mApplicationList, baseString)) {
+        if (TextUtils.equals(mPackageList, baseString)) {
             return false;
         }
 
-        mApplicationList = baseString;
-        mApplications.clear();
+        mPackageList = baseString;
+        mPackages.clear();
 
         if (baseString != null) {
             final String[] array = TextUtils.split(baseString, "\\|");
@@ -301,9 +282,9 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
                 if (TextUtils.isEmpty(item)) {
                     continue;
                 }
-                Application app = Application.fromString(item);
-                if (app != null) {
-                    mApplications.put(app.name, app);
+                Package pkg = Package.fromString(item);
+                if (pkg != null) {
+                    mPackages.put(pkg.name, pkg);
                 }
             }
         }
@@ -311,43 +292,43 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
         return true;
     }
 
-    private void saveApplicationList(boolean preferencesUpdated) {
+    private void savePackageList(boolean preferencesUpdated) {
         List<String> settings = new ArrayList<String>();
-        for (Application app : mApplications.values()) {
+        for (Package app : mPackages.values()) {
             settings.add(app.toString());
         }
         final String value = TextUtils.join("|", settings);
         if (preferencesUpdated) {
-            mApplicationList = value;
+            mPackageList = value;
         }
         Settings.System.putString(getContentResolver(),
                                   Settings.System.NOTIFICATION_LIGHT_PULSE_CUSTOM_VALUES, value);
     }
 
     /**
-     * Updates the default or application specific notification settings.
+     * Updates the default or package specific notification settings.
      *
-     * @param application Package name of application specific settings to update
+     * @param packageName Package name of application specific settings to update
      * @param color
      * @param timeon
      * @param timeoff
      */
-    protected void updateValues(String application, Integer color, Integer timeon, Integer timeoff) {
+    protected void updateValues(String packageName, Integer color, Integer timeon, Integer timeoff) {
         ContentResolver resolver = getContentResolver();
 
-        if (application.equals(DEFAULT_PREF)) {
+        if (packageName.equals(DEFAULT_PREF)) {
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_DEFAULT_COLOR, color);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_DEFAULT_LED_ON, timeon);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_DEFAULT_LED_OFF, timeoff);
             refreshDefault();
             return;
-        } else if (application.equals(MISSED_CALL_PREF)) {
+        } else if (packageName.equals(MISSED_CALL_PREF)) {
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_CALL_COLOR, color);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_CALL_LED_ON, timeon);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_CALL_LED_OFF, timeoff);
             refreshDefault();
             return;
-        } else if (application.equals(VOICEMAIL_PREF)) {
+        } else if (packageName.equals(VOICEMAIL_PREF)) {
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_VMAIL_COLOR, color);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_VMAIL_LED_ON, timeon);
             Settings.System.putInt(resolver, NOTIFICATION_LIGHT_PULSE_VMAIL_LED_OFF, timeoff);
@@ -355,20 +336,20 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
             return;
         }
 
-        // Find the custom app and sets its new values
-        Application app = mApplications.get(application);
+        // Find the custom package and sets its new values
+        Package app = mPackages.get(packageName);
         if (app != null) {
             app.color = color;
             app.timeon = timeon;
             app.timeoff = timeoff;
-            saveApplicationList(true);
+            savePackageList(true);
         }
     }
 
     public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id) {
         final Preference pref = (Preference) getPreferenceScreen().getRootAdapter().getItem(position);
 
-        if (mAppList.findPreference(pref.getKey()) != pref) {
+        if (mApplicationPrefList.findPreference(pref.getKey()) != pref) {
             return false;
         }
 
@@ -379,7 +360,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
                 .setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialog, int which) {
-                        removeCustomApplication(pref.getKey());
+                        removeCustomApplicationPref(pref.getKey());
                     }
                 })
                 .setNegativeButton(android.R.string.cancel, null);
@@ -439,7 +420,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
         switch (id) {
             case DIALOG_APPS:
                 final ListView list = new ListView(getActivity());
-                list.setAdapter(mAppAdapter);
+                list.setAdapter(mPackageAdapter);
 
                 builder.setTitle(R.string.profile_choose_app);
                 builder.setView(list);
@@ -449,8 +430,8 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
                     @Override
                     public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
                         // Add empty application definition, the user will be able to edit it later
-                        AppItem info = (AppItem) parent.getItemAtPosition(position);
-                        addCustomApplication(info.packageName);
+                        PackageItem info = (PackageItem) parent.getItemAtPosition(position);
+                        addCustomApplicationPref(info.packageName);
                         dialog.cancel();
                     }
                 });
@@ -464,7 +445,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
     /**
      * Application class
      */
-    private static class Application {
+    private static class Package {
         public String name;
         public Integer color;
         public Integer timeon;
@@ -477,16 +458,13 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
          * @param timeon
          * @param timeoff
          */
-        public Application(String name, Integer color, Integer timeon, Integer timeoff) {
+        public Package(String name, Integer color, Integer timeon, Integer timeoff) {
             this.name = name;
             this.color = color;
             this.timeon = timeon;
             this.timeoff = timeoff;
         }
 
-        public Application() {
-        }
-
         public String toString() {
             StringBuilder builder = new StringBuilder();
             builder.append(name);
@@ -499,7 +477,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
             return builder.toString();
         }
 
-        public static Application fromString(String value) {
+        public static Package fromString(String value) {
             if (TextUtils.isEmpty(value)) {
                 return null;
             }
@@ -512,7 +490,7 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
                 return null;
 
             try {
-                Application item = new Application(app[0], Integer.parseInt(values[0]), Integer
+                Package item = new Package(app[0], Integer.parseInt(values[0]), Integer
                         .parseInt(values[1]), Integer.parseInt(values[2]));
                 return item;
             } catch (NumberFormatException e) {
@@ -525,76 +503,92 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
     /**
      * AppItem class
      */
-    class AppItem implements Comparable<AppItem> {
+    private static class PackageItem implements Comparable<PackageItem> {
         CharSequence title;
+        TreeSet<CharSequence> activityTitles = new TreeSet<CharSequence>();
         String packageName;
         Drawable icon;
 
         @Override
-        public int compareTo(AppItem another) {
-            return this.title.toString().compareTo(another.title.toString());
+        public int compareTo(PackageItem another) {
+            int result = title.toString().compareToIgnoreCase(another.title.toString());
+            return result != 0 ? result : packageName.compareTo(another.packageName);
         }
     }
 
     /**
      * AppAdapter class
      */
-    class AppAdapter extends BaseAdapter {
-        protected List<ResolveInfo> mInstalledAppInfo;
-        protected List<AppItem> mInstalledApps = new LinkedList<AppItem>();
+    private class PackageAdapter extends BaseAdapter {
+        private List<PackageItem> mInstalledPackages = new LinkedList<PackageItem>();
 
         private void reloadList() {
             final Handler handler = new Handler();
             new Thread(new Runnable() {
-
                 @Override
                 public void run() {
-                    synchronized (mInstalledApps) {
-                        mInstalledApps.clear();
-                        for (ResolveInfo info : mInstalledAppInfo) {
-                            final AppItem item = new AppItem();
-                            item.title = info.loadLabel(mPackageManager);
-                            item.icon = info.loadIcon(mPackageManager);
-                            item.packageName = info.activityInfo.packageName;
-                            handler.post(new Runnable() {
-
-                                @Override
-                                public void run() {
-                                    int index = Collections.binarySearch(mInstalledApps, item);
-                                    if (index < 0) {
-                                        index = -index - 1;
-                                        mInstalledApps.add(index, item);
-                                    }
-                                    notifyDataSetChanged();
+                    synchronized (mInstalledPackages) {
+                        mInstalledPackages.clear();
+                    }
+
+                    final Intent mainIntent = new Intent(Intent.ACTION_MAIN, null);
+                    mainIntent.addCategory(Intent.CATEGORY_LAUNCHER);
+                    List<ResolveInfo> installedAppsInfo =
+                            mPackageManager.queryIntentActivities(mainIntent, 0);
+
+                    for (ResolveInfo info : installedAppsInfo) {
+                        ApplicationInfo appInfo = info.activityInfo.applicationInfo;
+
+                        final PackageItem item = new PackageItem();
+                        item.title = appInfo.loadLabel(mPackageManager);
+                        item.activityTitles.add(info.loadLabel(mPackageManager));
+                        item.icon = appInfo.loadIcon(mPackageManager);
+                        item.packageName = appInfo.packageName;
+
+                        handler.post(new Runnable() {
+                            @Override
+                            public void run() {
+                                // NO synchronize here: We know that mInstalledApps.clear()
+                                // was called and will never be called again.
+                                // At this point the only thread modifying mInstalledApp is main
+                                int index = Collections.binarySearch(mInstalledPackages, item);
+                                if (index < 0) {
+                                    mInstalledPackages.add(-index - 1, item);
+                                } else {
+                                    mInstalledPackages.get(index).activityTitles.addAll(item.activityTitles);
                                 }
-                            });
-                        }
+                                notifyDataSetChanged();
+                            }
+                        });
                     }
                 }
             }).start();
         }
 
-        public AppAdapter(List<ResolveInfo> installedAppsInfo) {
-            mInstalledAppInfo = installedAppsInfo;
-        }
-
-        public void update() {
+        public PackageAdapter() {
             reloadList();
         }
 
         @Override
         public int getCount() {
-            return mInstalledApps.size();
+            synchronized (mInstalledPackages) {
+                return mInstalledPackages.size();
+            }
         }
 
         @Override
-        public AppItem getItem(int position) {
-            return mInstalledApps.get(position);
+        public PackageItem getItem(int position) {
+            synchronized (mInstalledPackages) {
+                return mInstalledPackages.get(position);
+            }
         }
 
         @Override
         public long getItemId(int position) {
-            return mInstalledApps.get(position).packageName.hashCode();
+            synchronized (mInstalledPackages) {
+                // packageName is guaranteed to be unique in mInstalledPackages
+                return mInstalledPackages.get(position).packageName.hashCode();
+            }
         }
 
         @Override
@@ -611,18 +605,25 @@ public class NotificationLightSettings extends SettingsPreferenceFragment implem
                 holder.summary = (TextView) convertView.findViewById(com.android.internal.R.id.summary);
                 holder.icon = (ImageView) convertView.findViewById(R.id.icon);
             }
-            AppItem applicationInfo = getItem(position);
+            PackageItem applicationInfo = getItem(position);
 
-            if (holder.title != null) {
-                holder.title.setText(applicationInfo.title);
+            holder.title.setText(applicationInfo.title);
+            holder.icon.setImageDrawable(applicationInfo.icon);
+
+            boolean needSummary = applicationInfo.activityTitles.size() > 0;
+            if (applicationInfo.activityTitles.size() == 1) {
+                if (TextUtils.equals(applicationInfo.title, applicationInfo.activityTitles.first())) {
+                    needSummary = false;
+                }
             }
-            if (holder.summary != null) {
+
+            if (needSummary) {
+                holder.summary.setText(TextUtils.join(", ", applicationInfo.activityTitles));
+                holder.summary.setVisibility(View.VISIBLE);
+            } else {
                 holder.summary.setVisibility(View.GONE);
             }
-            if (holder.icon != null) {
-                Drawable loadIcon = applicationInfo.icon;
-                holder.icon.setImageDrawable(loadIcon);
-            }
+
             return convertView;
         }
     }