Implement a bluetooth picker dialog for Android

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

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser;
+
+import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+import android.graphics.Color;
+import android.text.SpannableString;
+import android.text.TextPaint;
+import android.text.TextUtils;
+import android.text.style.ClickableSpan;
+import android.view.View;
+
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.omnibox.OmniboxUrlEmphasizer;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.ui.base.WindowAndroid;
+import org.chromium.ui.text.SpanApplier;
+import org.chromium.ui.text.SpanApplier.SpanInfo;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * A dialog for picking available Bluetooth devices.

[newt (away), 2015/09/01 21:51:23]

Could you add a bit more context here. E.g. when is this dialog shown? / how
does the user or website trigger it?

[Finnur, 2015/09/02 16:01:43]

Done.

+ */
+public class BluetoothChooserDialog implements ItemChooserDialog.ItemSelectedCallback {
+    Context mContext;
+
+    // The dialog to show to let the user pick a device.
+    ItemChooserDialog mItemChooserDialog;
+
+    // The origin for the site wanting to pair with the bluetooth devices.
+    String mOrigin;
+
+    // The security level of the connection to the site wanting to pair with the
+    // bluetooth devices.
+    int mSecurityLevel;

[newt (away), 2015/09/01 21:51:23]

What are the valid values for this variable? Are they defined in an enum
somewhere? Please document.

[Finnur, 2015/09/02 16:01:43]

Done.

+
+    // A pointer back to the native part of the implementation for this dialog.
+    long mNativeBluetoothChooserDialogPtr;
+
+    // The type of link that is shown within the dialog.
+    private enum LinkType {
+        EXPLAIN_BLUETOOTH,
+        EXPLAIN_PARING,
+        ADAPTER_OFF,
+        ADAPTER_OFF_HELP,
+        RESTART_SEARCH,
+    }
+
+    /**
+     * Creates the BluetoothChooserDialog and displays it (and starts waiting for data).
+     *
+     * @param context Context which is used for launching a dialog.
+     */
+    public BluetoothChooserDialog(Context context, String origin,

[newt (away), 2015/09/01 21:51:23]

If this is only called from create(), then make this constructor and the show()
method private.

[Finnur, 2015/09/02 16:01:43]

It didn't use to be, but is now. Fixed.

+            int securityLevel, long nativeBluetoothChooserDialogPtr) {
+        mContext = context;
+        mOrigin = origin;
+        mSecurityLevel = securityLevel;
+        mNativeBluetoothChooserDialogPtr = nativeBluetoothChooserDialogPtr;
+    }
+
+    /**
+     * Show the BluetoothChooserDialog.
+     */
+    public void show() {
+        // Emphasize the origin.
+        Profile profile = Profile.getLastUsedProfile();
+        SpannableString origin = new SpannableString(mOrigin);
+        OmniboxUrlEmphasizer.emphasizeUrl(origin, mContext.getResources(),
+                profile, mSecurityLevel, false, true, true);
+        // Construct a full string and replace the origin text with emphasized version.
+        String message = mContext.getString(R.string.bluetooth_dialog_title, mOrigin);
+        SpannableString title = SpanApplier.applySpans(
+                message, new SpanInfo("<link>", "</link>",
+                        new NoUnderlineClickableSpan(LinkType.EXPLAIN_PARING, mContext)));
+        int start = title.toString().indexOf(mOrigin);

[newt (away), 2015/09/01 21:51:23]

Instead of indexOf(), I'd use title.getSpanStart(theClickableSpan).

[Finnur, 2015/09/02 16:01:43]

I'n not sure how to use getSpanStart to accomplish this. Let me take a step back
to make sure we're on the same page...

I'm starting with something like:
   "http://foo wants to <link-span>pair with</link-span>:"
... and hoping to convert that into:
   "<color-span>http</color-span>://foo wants to <link-span>pair
with</link-span>:"

getSpanStart would work if I were looking for the 'pair with' link-span, but I'm
not. I'm trying to take a static text and converting it into a color-span (so to
speak), while avoiding splitting the string up into two, which will lead to
translation headaches.

[newt (away), 2015/09/02 17:48:42]

Ah, I thought the span was wrapped around the origin. I didn't realize it was
actually wrapped around "Pair with".

I still don't love using indexOf() because it's sensitive to the value of the
origin (which we don't really control) and the translation (which we also don't
really control). It's quite unlikely that the origin text would be contained
elsewhere inside the translation, but it's possible. (This would be a much more
realistic problem if we were just showing hostnames since a hostname like "b" or
"go" could easily appear in the translation.)

I'll think about some improvements to SpanApplier to make situations like this
more easily handled. For how, it's probably OK to leave as is.

+        TextUtils.copySpansFrom(origin, 0, origin.length(), Object.class, title, start);
+
+        message = mContext.getString(R.string.bluetooth_not_found);
+        SpannableString noneFound = SpanApplier.applySpans(
+                message, new SpanInfo("<link>", "</link>",
+                        new NoUnderlineClickableSpan(LinkType.RESTART_SEARCH, mContext)));
+
+        String searching = mContext.getString(R.string.bluetooth_searching);
+        String positiveButton = mContext.getString(R.string.bluetooth_confirm_button);
+
+        SpannableString status = SpanApplier.applySpans(
+                mContext.getString(R.string.bluetooth_not_seeing_it),
+                new SpanInfo("<link1>", "</link1>",
+                        new NoUnderlineClickableSpan(LinkType.RESTART_SEARCH, mContext)),
+                new SpanInfo("<link2>", "</link2>",
+                        new NoUnderlineClickableSpan(LinkType.EXPLAIN_BLUETOOTH, mContext)));
+
+        SpannableString errorMessage = SpanApplier.applySpans(
+                mContext.getString(R.string.bluetooth_adapter_off),
+                new SpanInfo("<link>", "</link>",
+                        new NoUnderlineClickableSpan(LinkType.ADAPTER_OFF, mContext)));
+        SpannableString errorStatus = SpanApplier.applySpans(
+                mContext.getString(R.string.bluetooth_adapter_off_help),
+                new SpanInfo("<link>", "</link>",
+                        new NoUnderlineClickableSpan(LinkType.ADAPTER_OFF_HELP, mContext)));
+
+        ItemChooserDialog.ItemChooserLabels labels =
+                new ItemChooserDialog.ItemChooserLabels(title, searching, noneFound, status,
+                        errorMessage, errorStatus, positiveButton);
+        mItemChooserDialog = new ItemChooserDialog(mContext, this, labels);
+    }
+
+    @Override
+    public void onItemSelected(String id) {
+        if (mNativeBluetoothChooserDialogPtr != 0) {
+            nativeReportDeviceSelected(mNativeBluetoothChooserDialogPtr, id);
+        }
+    }
+
+    /**
+     * A helper class to show a clickable link with underlines turned off.
+     */
+    private class NoUnderlineClickableSpan extends ClickableSpan {
+        // The type of link this span represents.
+        private LinkType mLinkType;
+
+        // TODO(finnur): Remove this variable when toasts have been eliminated.
+        private Context mContext;
+
+        NoUnderlineClickableSpan(LinkType linkType, Context context) {
+            mLinkType = linkType;
+            mContext = context;
+        }
+
+        @Override
+        public void onClick(View view) {
+            if (mNativeBluetoothChooserDialogPtr == 0) {
+                return;
+            }
+
+            switch (mLinkType) {
+                case EXPLAIN_BLUETOOTH: {
+                    if (mNativeBluetoothChooserDialogPtr != 0) {

[newt (away), 2015/09/01 21:51:23]

This if check is redundant, since you already checked
mNativeBluetoothChooserDialogPtr's nullness at the beginning of the method. Same
for the if checks below.

[Finnur, 2015/09/02 16:01:43]

Ah. Forgot about this. Fixed.

+                        nativeShowBluetoothOverviewLink(mNativeBluetoothChooserDialogPtr);
+                        closeDialog();
+                    }
+                    break;
+                }
+                case EXPLAIN_PARING: {
+                    if (mNativeBluetoothChooserDialogPtr != 0) {
+                        nativeShowBluetoothPairingLink(mNativeBluetoothChooserDialogPtr);
+                        closeDialog();
+                    }
+                    break;
+                }
+                case ADAPTER_OFF: {
+                    Intent intent = new Intent();
+                    intent.setAction(android.provider.Settings.ACTION_BLUETOOTH_SETTINGS);
+                    mContext.startActivity(intent);
+                    break;
+                }
+                case ADAPTER_OFF_HELP: {
+                    if (mNativeBluetoothChooserDialogPtr != 0) {
+                        nativeShowBluetoothAdapterOffLink(mNativeBluetoothChooserDialogPtr);
+                        closeDialog();
+                    }
+                    break;
+                }
+                case RESTART_SEARCH: {
+                    mItemChooserDialog.clear();
+                    if (mNativeBluetoothChooserDialogPtr != 0) {
+                        nativeReportRestartSearch(mNativeBluetoothChooserDialogPtr);
+                    }
+                    break;
+                }
+                default:
+                    assert false;
+            }
+
+            // Get rid of the highlight background on selection.
+            view.invalidate();
+        }
+
+        @Override
+        public void updateDrawState(TextPaint textPaint) {
+            super.updateDrawState(textPaint);
+            textPaint.bgColor = Color.TRANSPARENT;
+            textPaint.setUnderlineText(false);
+        }
+    }
+
+    @CalledByNative
+    private static BluetoothChooserDialog create(
+            WindowAndroid windowAndroid, String origin, int securityLevel,
+            long nativeBluetoothChooserDialogPtr) {
+        Activity activity = windowAndroid.getActivity().get();

[newt (away), 2015/09/01 21:51:23]

windowAndroid.getActivity().get() can be null

[Finnur, 2015/09/02 16:01:43]

Yes, I thought about that. Can we expect it to be null under normal
circumstances and if so, how?

My thinking was that we'd probably want to know when/if it occurs, rather than
silently fail. But, if this occurs normally, e.g. at shutdown, or something,
then we need to silently fail.

[Ted C, 2015/09/02 17:27:47]

There are a couple cases where it can be null.

1.) WebView where it is not created using an activity context, but this is a
    chrome feature, so that should not apply.

2.) During shutdown.  The activity is dying and it has been garbage collected
    already by android.  It implies that you are holding onto a stale window
    ref, which more often happens on async messages being handled by the android
    looper.  Here you are driven off a webcontents, which should be deleted when
    an activity is destroyed, so this most likely would be a programming error.
    Granted if you have any messages that are being processed outside of the
    webcontents lifecycle then you start introducing chances for this to happen.

I suspect in your case you can either gracefully fail or assert it is not null.

[Finnur, 2015/09/03 11:51:45]

Thanks for the explanation! I think I'll go with an assert for now and we can
change it to a null check if it becomes problematic.

+        BluetoothChooserDialog dialog = new BluetoothChooserDialog(
+                activity, origin, securityLevel, nativeBluetoothChooserDialogPtr);
+        dialog.show();
+        return dialog;
+    }
+
+    @CalledByNative
+    private void addDevice(String deviceId, String deviceName) {
+        List<ItemChooserDialog.ItemChooserRow> devices =
+                new ArrayList<ItemChooserDialog.ItemChooserRow>();
+        devices.add(new ItemChooserDialog.ItemChooserRow(deviceId, deviceName));
+        mItemChooserDialog.showList(devices);
+    }
+
+    @CalledByNative
+    private void closeDialog() {

[newt (away), 2015/09/01 21:51:23]

If the user closes the dialog via Java (e.g. by pressing back, or even by
Android killing the Activity), will the native side of this object be deleted?
It doesn't appear so currently.

[Finnur, 2015/09/02 16:01:43]

The native side is owned by the RequestDeviceSession object so it is cleaned up
alongside that. However, this reminded me that the dismiss handler for the
dialog is missing, it is supposed to notify that nothing was selected. Once that
is added, the requestDevice session is destroyed on dismiss and it takes the
native side down along with it.

+        mNativeBluetoothChooserDialogPtr = 0;
+        mItemChooserDialog.dismiss();
+    }
+
+    @CalledByNative
+    private void removeDevice(String deviceId) {
+        mItemChooserDialog.setAvailability(deviceId, false);
+    }
+
+    @CalledByNative
+    private void notifyAdapterTurnedOff() {
+        mItemChooserDialog.setErrorState();
+    }
+
+    private native void nativeReportDeviceSelected(
+            long nativeBluetoothChooserAndroid, String deviceId);
+    private native void nativeReportRestartSearch(long nativeBluetoothChooserAndroid);
+    // Help links.
+    private native void nativeShowBluetoothOverviewLink(long nativeBluetoothChooserAndroid);
+    private native void nativeShowBluetoothPairingLink(long nativeBluetoothChooserAndroid);
+    private native void nativeShowBluetoothAdapterOffLink(long nativeBluetoothChooserAndroid);
+}