Distinguish devices with same name in chooser on Android

chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
index 04c717a3dabccea2e552339519830ea81fa48c85..d6b6ac311d7fab392e839eabed0b321727abe571 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.util.MathUtils;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.widget.TextViewWithClickableSpans;
 
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -136,6 +137,9 @@ public class ItemChooserDialog {
         // A set of keys that are marked as disabled in the dialog.
         private Set<String> mDisabledEntries = new HashSet<String>();
 
+        // Device names are counted in a map.
+        private HashMap<String, Integer> mDeviceNameMap = new HashMap<String, Integer>();

[Yaron, 2016/06/29 21:18:19]

doesn't seem like "deviceName" should be part of "ItemChooserDialog". It should
be more abstract

[juncai, 2016/06/29 22:48:38]

Done.

+
         public ItemAdapter(Context context, int resource) {
             super(context, resource);
 
@@ -148,6 +152,28 @@ public class ItemChooserDialog {
         }
 
         @Override
+        public void add(ItemChooserRow item) {
+            String description = item.mDescription;
+            Integer counter = mDeviceNameMap.get(description);

[Yaron, 2016/06/29 21:18:19]

Nit: I think it's better to code against primitive types and let the autoboxing
happen just when dealing with container objects. For example, the addition below
will likely incur additional overhead vs a primitive

[juncai, 2016/06/29 22:48:38]

Done.

+            mDeviceNameMap.put(description, counter == null ? 1 : counter + 1);
+            super.add(item);
+        }
+
+        @Override
+        public void remove(ItemChooserRow item) {
+            String description = item.mDescription;
+            Integer counter = mDeviceNameMap.get(description);
+            if (counter != null) {
+                if (counter.intValue() == 1) {
+                    mDeviceNameMap.remove(description);
+                } else {
+                    mDeviceNameMap.put(description, counter - 1);
+                }
+            }
+            super.remove(item);
+        }
+
+        @Override
         public void clear() {
             mSelectedItem = ListView.INVALID_POSITION;
             mConfirmButton.setEnabled(false);
@@ -165,6 +191,21 @@ public class ItemChooserDialog {
         }
 
         /**
+         * Returns the description of the |position|th item. For items with
+         * the same description, their unique keys are appended to distinguish
+         * them.
+         */
+        public String getItemDescription(int position) {
+            ItemChooserRow item = getItem(position);
+            String description = item.mDescription;
+            Integer counter = mDeviceNameMap.get(description);
+            return counter.intValue() == 1
+                    ? description
+                    : mActivity.getString(
+                              R.string.item_chooser_item_name_with_id, description, item.mKey);
+        }
+
+        /**
          * Sets whether the itam is enabled. Disabled items are grayed out.
          * @param id The id of the item to affect.
          * @param enabled Whether the item should be enabled or not.
@@ -218,8 +259,7 @@ public class ItemChooserDialog {
                 }
             }
 
-            ItemChooserRow item = getItem(position);
-            view.setText(item.mDescription);
+            view.setText(getItemDescription(position));
             return view;
         }
 
@@ -453,4 +493,12 @@ public class ItemChooserDialog {
     public ItemAdapter getItemAdapterForTesting() {
         return mItemAdapter;
     }
+
+    /**
+     * Returns the description of |position|th item. For use with tests only.
+     */
+    @VisibleForTesting
+    public String getItemDescriptionForTesting(int position) {

[Yaron, 2016/06/29 21:18:19]

Seems unnecessary. You can just use |getItemAdapterForTesting| and call
getItemDescription on that.

[juncai, 2016/06/29 22:48:38]

Done.

+        return mItemAdapter.getItemDescription(position);
+    }
 }