bluetooth: Implement RSSI indicator 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 d93e107d842d9a3df9b1288538dbc1e9eab095a6..23f4ed106e99f2cc524a94392ca9e6a3a274783b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
@@ -10,6 +10,7 @@ import android.content.Context;
 import android.content.DialogInterface;
 import android.graphics.Color;
 import android.graphics.drawable.ColorDrawable;
+import android.graphics.drawable.Drawable;
 import android.text.SpannableString;
 import android.text.method.LinkMovementMethod;
 import android.view.Gravity;
@@ -21,9 +22,11 @@ import android.view.Window;
 import android.widget.AdapterView;
 import android.widget.ArrayAdapter;
 import android.widget.Button;
+import android.widget.ImageView;
 import android.widget.LinearLayout;
 import android.widget.ListView;
 import android.widget.ProgressBar;
+import android.widget.RelativeLayout;
 import android.widget.TextView;
 
 import org.chromium.base.ApiCompatibilityUtils;
@@ -60,15 +63,61 @@ public class ItemChooserDialog {
     }
 
     /**
+     * A class representing an icon to be shown in a chooser row.
+     */
+    public static class ItemChooserRowIcon {

[Ian Wen, 2016/09/26 18:12:48]

I would vote for using level list drawable instead of a custom class. Benefits
of using level list drawable are:

1. More consistent with the framework. If you see the android doc for
LevelListDrawable, it's highlighted example is to display wifi signals, which is
almost identical to the problem here.

2. You no longer need to override equals() and hashcode(). Simply get level and
compare integers.

Like what you did above, you still need to new LevelListDrawable, call mutate on
it, and add level to it manually.

[juncai, 2016/09/26 18:49:01]

Question: if we use LevelListDrawable here, can other icon be shown at the same
place as the signal strength level icon when it is not shown? Because when a
Bluetooth device is connected, we would like to display a Bluetooth connected
icon instead of a signal strength level icon, a screenshot from desktop is at:
https://bugs.chromium.org/p/chromium/issues/detail?id=543466#c63

[Ian Wen, 2016/09/26 21:39:33]

The dialog can store just a drawable, and it can still perform getLevel and
setLevel on a normal drawable. These operations will be no-op if the icon is
"connected".

[ortuno, 2016/09/26 23:34:48]

Some context:

ItemChooserDialog is a class that is used by any client that wants to show a
list with some items. There are two clients UsbChooserDialog and
BluetoothChooserDialog. This CL adds the ability for ItemChooserDialog to show
rows with icons. To do this, clients have to initialize ItemChooserDialog with
isUsingIcon set to true and pass in an icon in addOrUpdateItem().

In the case of Bluetooth that icon is sometimes a signal strength level and
sometimes some other icon. 

I had a first version of this patch using LeveListDrawable. The reason I didn't
end up submitting it was that I thought this was more optional. With this
approach we create the icons once in BluetoothChooserDialog and then reuse them
when calling addOrUpdateItem. Maybe I'm missing something but with the
LevelListDrawable approach I believe we would have to create a new
LevelListDrawable for each new update.
True, but I don't think it quite matches our use case. In the example, there is
an icon on which you can call setLevel when the level changes. In our case the
class that owns that Icon, ItemChooserDialog, doesn't know that the icon
represents a level, it could be a usb icon, a bluetooth icon or a signal level.
Which I think is fine because the Signal Strength level is a Bluetooth-only
concept.

[Ian Wen, 2016/09/27 04:22:41]

Thanks for the explanation!

If you are worried about creating new drawables, it is fine as it should be
fairly cheap. All the actual bitmaps are not copied. See here for detail:
http://android-developers.blogspot.com/2009/05/drawable-mutations.html.


I created a simple CL to demonstrate the approach that I am proposing.
https://chromiumcodereview.appspot.com/2370203002/

[ortuno, 2016/10/04 07:16:18]

Thanks for the article and CL :) I really appreciate it. I tried the approach
you proposed in the CL and bumped into some issues. I left comments in the CL.

+        private Drawable mNonSelectedIcon;
+        // Icon when the row is selected.
+        private Drawable mSelectedIcon;
+        private String mIconDescription;
+
+        public ItemChooserRowIcon(
+                Drawable nonSelectedIcon, Drawable selectedIcon, String iconDescription) {
+            mNonSelectedIcon = nonSelectedIcon;
+            mSelectedIcon = selectedIcon;
+            mIconDescription = iconDescription;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof ItemChooserRowIcon)) return false;
+            if (this == obj) return true;
+            ItemChooserRowIcon icon = (ItemChooserRowIcon) obj;
+            return mNonSelectedIcon.getConstantState().equals(
+                           icon.mNonSelectedIcon.getConstantState())
+                    && mSelectedIcon.getConstantState().equals(
+                               icon.mSelectedIcon.getConstantState())
+                    && mIconDescription.equals(icon.mIconDescription);
+        }
+
+        @Override
+        public String toString() {
+            return mNonSelectedIcon + ":" + mSelectedIcon + ":" + mIconDescription;
+        }
+
+        @Override
+        public int hashCode() {
+            return mNonSelectedIcon.hashCode() + mSelectedIcon.hashCode()
+                    + mIconDescription.hashCode();
+        }
+    }
+
+    /**
      * A class representing one data row in the picker.
      */
     public static class ItemChooserRow {
         private final String mKey;
         private String mDescription;
+        private ItemChooserRowIcon mIcon;
 
-        public ItemChooserRow(String key, String description) {
+        public ItemChooserRow(String key, String description, ItemChooserRowIcon icon) {
             mKey = key;
             mDescription = description;
+            mIcon = icon;
+        }
+
+        public ItemChooserRow(String key, String description) {
+            this(key, description, null);
         }
 
         @Override
@@ -76,12 +125,24 @@ public class ItemChooserDialog {
             if (!(obj instanceof ItemChooserRow)) return false;
             if (this == obj) return true;
             ItemChooserRow item = (ItemChooserRow) obj;
-            return mKey.equals(item.mKey) && mDescription.equals(item.mDescription);
+
+            if (mIcon == null ^ item.mIcon == null) return false;
+
+            if (mIcon != null && item.mIcon != null && !mIcon.equals(item.mIcon)) return false;
+
+            if (mKey.equals(item.mKey) && mDescription.equals(item.mDescription)) return true;
+
+            return false;
         }
 
         @Override
         public int hashCode() {
-            return mKey.hashCode() + mDescription.hashCode();
+            return mKey.hashCode() + mDescription.hashCode() + mIcon.hashCode();
+        }
+
+        @Override
+        public String toString() {
+            return mKey + ":" + mDescription + ":" + mIcon.toString();
         }
     }
 
@@ -138,6 +199,12 @@ public class ItemChooserDialog {
         // The color of the non-highlighted text.
         private final int mDefaultTextColor;
 
+        // The color of disabled text.
+        private final int mDisabledTextColor;
+
+        // Indicates whether rows will contain an icon.
+        private boolean mUsingIcon;
+
         // 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;
@@ -151,15 +218,19 @@ public class ItemChooserDialog {
         // Map of keys to items so that we can access the items in O(1).
         private Map<String, ItemChooserRow> mKeyToItemMap = new HashMap<>();
 
-        public ItemAdapter(Context context, int resource) {
+        public ItemAdapter(Context context, int resource, boolean usingIcon) {
             super(context, resource);
 
             mInflater = LayoutInflater.from(context);
 
+            mUsingIcon = usingIcon;
+
             mBackgroundHighlightColor = ApiCompatibilityUtils.getColor(getContext().getResources(),
                     R.color.light_active_color);
             mDefaultTextColor = ApiCompatibilityUtils.getColor(getContext().getResources(),
                     R.color.default_text_color);
+            mDisabledTextColor = ApiCompatibilityUtils.getColor(
+                    getContext().getResources(), R.color.primary_text_disabled_material_light);
         }
 
         @Override
@@ -188,6 +259,10 @@ public class ItemChooserDialog {
                     oldItem.mDescription = item.mDescription;
                     addToDescriptionsMap(oldItem.mDescription);
                 }
+                if (item.mIcon != null
+                        && (oldItem.mIcon == null || !oldItem.mIcon.equals(item.mIcon))) {
+                    oldItem.mIcon = item.mIcon;
+                }
                 notifyDataSetChanged();
                 return;
             }
@@ -240,6 +315,16 @@ public class ItemChooserDialog {
                             item.mKey);
         }
 
+        public ItemChooserRowIcon getIcon(int position) {
+            ItemChooserRow item = getItem(position);
+            return item.mIcon;
+        }
+
+        public boolean hasIcon(int position) {
+            ItemChooserRow item = getItem(position);
+            return item.mIcon != null;
+        }
+
         /**
          * Sets whether the itam is enabled. Disabled items are grayed out.
          * @param id The id of the item to affect.
@@ -272,29 +357,44 @@ public class ItemChooserDialog {
 
         @Override
         public View getView(int position, View convertView, ViewGroup parent) {
-            TextView view;
-            if (convertView instanceof TextView) {
-                view = (TextView) convertView;
+            RelativeLayout view;
+            if (convertView instanceof RelativeLayout) {
+                view = (RelativeLayout) convertView;
             } else {
-                view = (TextView) mInflater.inflate(
+                view = (RelativeLayout) mInflater.inflate(
                         R.layout.item_chooser_dialog_row, parent, false);
             }
 
-            // Set highlighting for currently selected item.
-            if (position == mSelectedItem) {
+            boolean selected = position == mSelectedItem;
+            TextView description = (TextView) view.findViewById(R.id.description);
+            description.setText(getDisplayText(position));
+
+            if (selected) {
                 view.setBackgroundColor(mBackgroundHighlightColor);
-                view.setTextColor(Color.WHITE);
+                description.setTextColor(Color.WHITE);
             } else {
                 view.setBackground(null);
-                if (!isEnabled(position)) {
-                    view.setTextColor(ApiCompatibilityUtils.getColor(getContext().getResources(),
-                            R.color.primary_text_disabled_material_light));
+                description.setTextColor(
+                        isEnabled(position) ? mDefaultTextColor : mDisabledTextColor);
+            }
+
+            LinearLayout imageContainer = (LinearLayout) view.findViewById(R.id.imageContainer);
+            if (mUsingIcon) {
+                ImageView icon = (ImageView) view.findViewById(R.id.icon);
+                if (hasIcon(position)) {
+                    imageContainer.setVisibility(View.VISIBLE);
+                    ItemChooserRowIcon rowIcon = getIcon(position);
+                    icon.setContentDescription(rowIcon.mIconDescription);

[Ian Wen, 2016/09/26 18:12:48]

If you choose to use level drawable, you might want to use getQuantityString
here with drawable.getLevel().

+                    icon.setImageDrawable(
+                            selected ? rowIcon.mSelectedIcon : rowIcon.mNonSelectedIcon);
                 } else {
-                    view.setTextColor(mDefaultTextColor);
+                    imageContainer.setVisibility(View.INVISIBLE);
+                    icon.setContentDescription(null);
+                    icon.setImageDrawable(null);
                 }
+            } else {
+                imageContainer.setVisibility(View.GONE);
             }
-
-            view.setText(getDisplayText(position));
             return view;
         }
 
@@ -363,8 +463,8 @@ public class ItemChooserDialog {
      * @param callback The callback used to communicate back what was selected.
      * @param labels The labels to show in the dialog.
      */
-    public ItemChooserDialog(
-            Activity activity, ItemSelectedCallback callback, ItemChooserLabels labels) {
+    public ItemChooserDialog(Activity activity, ItemSelectedCallback callback,
+            ItemChooserLabels labels, boolean usingIcon) {
         mActivity = activity;
         mItemSelectedCallback = callback;
         mLabels = labels;
@@ -398,7 +498,7 @@ public class ItemChooserDialog {
             }
         });
 
-        mItemAdapter = new ItemAdapter(mActivity, R.layout.item_chooser_dialog_row);
+        mItemAdapter = new ItemAdapter(mActivity, R.layout.item_chooser_dialog_row, usingIcon);
         mListView.setAdapter(mItemAdapter);
         mListView.setEmptyView(mEmptyMessage);
         mListView.setOnItemClickListener(mItemAdapter);