bluetooth-chooser: Refactor and clean up bluetooth chooser

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 50de53333e99111ccdac0bd2f397b8138da8ea76..3d484a611e11bc7bd15b0654d402052a43d41d34 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
@@ -26,7 +26,6 @@ import android.widget.ListView;
 import android.widget.ProgressBar;
 import android.widget.TextView;
 
-import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.util.MathUtils;
@@ -71,17 +70,10 @@ public class ItemChooserDialog {
             mDescription = description;
         }
 
-        @Override
-        public boolean equals(Object obj) {
-            if (!(obj instanceof ItemChooserRow)) return false;
-            if (this == obj) return true;
-            ItemChooserRow item = (ItemChooserRow) obj;
-            return mKey.equals(item.mKey) && mDescription.equals(item.mDescription);
-        }
-
-        @Override
-        public int hashCode() {
-            return mKey.hashCode() + mDescription.hashCode();
+        public boolean hasContents(String key, String description) {

[Ian Wen, 2016/10/19 00:28:46]

Nit: how about renaming the function to "sameAs" or "hasContentAs" or
"hasSameContent"?

[ortuno, 2016/10/19 01:59:33]

Done.

+            if (!mKey.equals(key)) return false;
+            if (!mDescription.equals(description)) return false;
+            return true;
         }
     }
 
@@ -120,6 +112,15 @@ public class ItemChooserDialog {
         }
     }
 
+    // Item holder for performance boost.

[Ian Wen, 2016/10/19 00:28:46]

Nit: use javadoc style instead of //.

[ortuno, 2016/10/19 01:59:33]

Done.

+    private static class ViewHolder {
+        private TextView mTextView;
+
+        public ViewHolder(View view) {
+            mTextView = (TextView) view.findViewById(R.id.description);
+        }
+    }
+
     /**
      * The various states the dialog can represent.
      */
@@ -132,12 +133,6 @@ public class ItemChooserDialog {
             implements AdapterView.OnItemClickListener {
         private final LayoutInflater mInflater;
 
-        // The background color of the highlighted item.
-        private final int mBackgroundHighlightColor;
-
-        // The color of the non-highlighted text.
-        private final int mDefaultTextColor;
-
         // The zero-based index of the item currently selected in the dialog,
         // or -1 (INVALID_POSITION) if nothing is selected.
         private int mSelectedItem = ListView.INVALID_POSITION;
@@ -155,11 +150,6 @@ public class ItemChooserDialog {
             super(context, resource);
 
             mInflater = LayoutInflater.from(context);
-
-            mBackgroundHighlightColor = ApiCompatibilityUtils.getColor(getContext().getResources(),
-                    R.color.light_active_color);
-            mDefaultTextColor = ApiCompatibilityUtils.getColor(getContext().getResources(),
-                    R.color.default_text_color);
         }
 
         @Override
@@ -176,31 +166,34 @@ public class ItemChooserDialog {
             return isEmpty;
         }
 
-        public void addOrUpdate(ItemChooserRow item) {
-            ItemChooserRow oldItem = mKeyToItemMap.get(item.mKey);
+        public void addOrUpdate(String key, String description) {
+            ItemChooserRow oldItem = mKeyToItemMap.get(key);
             if (oldItem != null) {
-                if (oldItem.equals(item)) {
+                if (oldItem.hasContents(key, description)) {
                     // No need to update anything.
                     return;
                 }
-                if (!oldItem.mDescription.equals(item.mDescription)) {
+
+                if (!oldItem.mDescription.equals(description)) {
                     removeFromDescriptionsMap(oldItem.mDescription);
-                    oldItem.mDescription = item.mDescription;
+                    oldItem.mDescription = description;
                     addToDescriptionsMap(oldItem.mDescription);
                 }
+
                 notifyDataSetChanged();
                 return;
             }
-            ItemChooserRow result = mKeyToItemMap.put(item.mKey, item);
+
+            ItemChooserRow newItem = new ItemChooserRow(key, description);
+            ItemChooserRow result = mKeyToItemMap.put(key, newItem);

[Ian Wen, 2016/10/19 00:28:46]

Nit: remove #188 and #189 as it's apparent. Or change to assert
mKeyToItemMap.containsKey(key) == false and put it above #187;

[ortuno, 2016/10/19 01:59:33]

Maybe I'm missing something but I don't think I can remove #188 since I still
need to add the item to the map. I changed the assert and moved it above #187
let me know if you meant something different.

             assert result == null;
 
-            addToDescriptionsMap(item.mDescription);
-            add(item);
+            addToDescriptionsMap(newItem.mDescription);
+            add(newItem);
         }
 
-        @Override
-        public void remove(ItemChooserRow item) {
-            ItemChooserRow oldItem = mKeyToItemMap.remove(item.mKey);
+        public void remove(String key) {
+            ItemChooserRow oldItem = mKeyToItemMap.remove(key);
             if (oldItem == null) return;
             int oldItemPosition = getPosition(oldItem);
             // If the removed item is the item that is currently selected, deselect it
@@ -292,37 +285,26 @@ public class ItemChooserDialog {
 
         @Override
         public View getView(int position, View convertView, ViewGroup parent) {
-            TextView view;
-            if (convertView instanceof TextView) {
-                view = (TextView) convertView;
+            ViewHolder row;
+            if (convertView == null) {
+                convertView = mInflater.inflate(R.layout.item_chooser_dialog_row, parent, false);
+                row = new ViewHolder(convertView);
+                convertView.setTag(row);
             } else {
-                view = (TextView) mInflater.inflate(
-                        R.layout.item_chooser_dialog_row, parent, false);
+                row = (ViewHolder) convertView.getTag();
             }
 
-            // Set highlighting for currently selected item.
-            if (position == mSelectedItem) {
-                view.setBackgroundColor(mBackgroundHighlightColor);
-                view.setTextColor(Color.WHITE);
-            } else {
-                view.setBackground(null);
-                if (!isEnabled(position)) {
-                    view.setTextColor(ApiCompatibilityUtils.getColor(getContext().getResources(),
-                            R.color.primary_text_disabled_material_light));
-                } else {
-                    view.setTextColor(mDefaultTextColor);
-                }
-            }
+            row.mTextView.setEnabled(isEnabled(position));
+            row.mTextView.setText(getDisplayText(position));
 
-            view.setText(getDisplayText(position));
-            return view;
+            return convertView;
         }
 
         @Override
         public void onItemClick(AdapterView<?> adapter, View view, int position, long id) {
             mSelectedItem = position;
             mConfirmButton.setEnabled(true);
-            mItemAdapter.notifyDataSetChanged();
+            notifyDataSetChanged();
         }
 
         private void addToDescriptionsMap(String description) {
@@ -421,6 +403,7 @@ public class ItemChooserDialog {
         mItemAdapter = new ItemAdapter(mActivity, R.layout.item_chooser_dialog_row);
         mItemAdapter.setNotifyOnChange(true);
         mListView.setAdapter(mItemAdapter);
+        mListView.setChoiceMode(ListView.CHOICE_MODE_SINGLE);
         mListView.setEmptyView(mEmptyMessage);
         mListView.setOnItemClickListener(mItemAdapter);
         mListView.setDivider(null);
@@ -486,24 +469,25 @@ public class ItemChooserDialog {
     }
 
     /**
-    * Add an item to the end of the list to show in the dialog if the item
-    * was not in the chooser. Otherwise update the items description.
-    *
-    * @param item The item to be added to the end of the chooser or updated.
-    */
-    public void addOrUpdateItem(ItemChooserRow item) {
+     * Adds an item to the end of the list to show in the dialog if the item
+     * was not in the chooser. Otherwise update the items description.

[Ian Wen, 2016/10/19 00:28:46]

Nit: s/update/updates

[ortuno, 2016/10/19 01:59:33]

Done.

+     *
+     * @param key Unique identifier for that item.
+     * @param description Text in the row.
+     */
+    public void addOrUpdateItem(String key, String description) {
         mProgressBar.setVisibility(View.GONE);
-        mItemAdapter.addOrUpdate(item);
+        mItemAdapter.addOrUpdate(key, description);
         setState(State.PROGRESS_UPDATE_AVAILABLE);
     }
 
     /**
-    * Remove an item that is shown in the dialog.
-    *
-    * @param item The item to be removed in the chooser.
-    */
-    public void removeItemFromList(ItemChooserRow item) {
-        mItemAdapter.remove(item);
+     * Remove an item that is shown in the dialog.

[Ian Wen, 2016/10/19 00:28:46]

Nit: s/Remove/Removes

[ortuno, 2016/10/19 01:59:33]

Done.

+     *
+     * @param key Unique identifier for the item.
+     */
+    public void removeItemFromList(String key) {
+        mItemAdapter.remove(key);
         setState(State.DISCOVERY_IDLE);
     }
 
@@ -517,11 +501,11 @@ public class ItemChooserDialog {
 
     /**
      * Sets whether the item is enabled.
-     * @param id The id of the item to affect.
+     * @param key Unique indetifier for the item.
      * @param enabled Whether the item should be enabled or not.
      */
-    public void setEnabled(String id, boolean enabled) {
-        mItemAdapter.setEnabled(id, enabled);
+    public void setEnabled(String key, boolean enabled) {
+        mItemAdapter.setEnabled(key, enabled);
     }
 
     /**