Add functionality to Bluetooth settings UI.

chrome/browser/resources/chromeos/emulator/bluetooth_settings.js

Index: chrome/browser/resources/chromeos/emulator/bluetooth_settings.js
diff --git a/chrome/browser/resources/chromeos/emulator/bluetooth_settings.js b/chrome/browser/resources/chromeos/emulator/bluetooth_settings.js
index e656e41984789b2ead64e450622ce8f413cb0f74..e49a0bbe9142020ab7e823093de05afcdebba4b4 100644
--- a/chrome/browser/resources/chromeos/emulator/bluetooth_settings.js
+++ b/chrome/browser/resources/chromeos/emulator/bluetooth_settings.js
@@ -9,13 +9,22 @@
 var BluetoothDevice = function() {
   this.address = '';
   this.alias = '';
+
+  // The text label of the selected device class.
   this.class = 'Computer';
+
+  // The uint32 value of the selected device class.
   this.classValue = 0;
-  this.isPairable = true;
+
   this.isTrusted = true;
   this.name = '';
   this.path = '';
-  this.pairMethod = 'None';
+
+  // The label of the selected pairing method option.
+  this.pairingMethod = 'None';
+
+  // The text containing a PIN key or passkey for pairing.
+  this.pairingAuthToken = '';
 };
 
 Polymer({
@@ -30,13 +39,6 @@ Polymer({
     },
 
     /**
-     * Indicates whether or not the main bluetooth adapter is turned on.
-     */
-    powerToMainAdapter: {
-      type: Boolean
-    },
-
-    /**
      * A set of bluetooth devices.
      * @type !Array<!BluetoothDevice>
      */
@@ -59,7 +61,7 @@ Polymer({
                 { text: 'Keyboard', value: 0x2540 },
                 { text: 'Audio', value: 0x240408 },
                 { text: 'Phone', value: 0x7a020c },
-                { text: 'Computer', value: 0x100 }];
+                { text: 'Computer', value: 0x104 }];
       }
     },
 
@@ -70,7 +72,15 @@ Polymer({
      */
     deviceAuthenticationMethods: {
       type: Array,
-      value: function() { return ['None', 'Pin Code', 'Pass Key']; }
+      value: function() { return ['None', 'PIN Code', 'PassKey']; }
+    },
+
+    /**
+     * Contains keys for all the device paths which have been discovered.

[stevenjb, 2015/07/30 18:28:51]

@type

[rfrappier, 2015/07/30 22:21:38]

Done.

+     */
+    devicePaths: {
+      type: Object,
+      value: function() { return []; }

[xiyuan, 2015/07/30 17:21:22]

Should we return "{}" for an Object?

[stevenjb, 2015/07/30 18:28:51]

It actually looks like this should be type: Array

[xiyuan, 2015/07/30 18:33:57]

|devicePaths| is used as a dict to look up whether a device path is known. "{}"
is better in this sense since path string is used for look up.

     },
   },
 
@@ -78,9 +88,35 @@ Polymer({
     this.title = 'Bluetooth Settings';
   },
 
+  // Checks whether or not the PIN/passkey input field should be shown.
+  // It should only be shown when the pair method is not 'None' or empty.

[stevenjb, 2015/07/30 18:28:51]

@oaram {string} pairMethod
@return {boolean} Whether the PIN/passkey input field should be shown.

[rfrappier, 2015/07/30 22:21:38]

Done.

+  showAuthToken: function(pairMethod) {
+    return pairMethod != 'None' && pairMethod != undefined;
+  },
+
+  // Called by the WebUI which provides a list of devices which are connected
+  // to the main adapter.

[stevenjb, 2015/07/30 18:28:51]

@param ...

[rfrappier, 2015/07/30 22:21:38]

Done.

+  updateBluetoothInfo: function(devices) {
+    device_list = [];

[xiyuan, 2015/07/30 17:21:21]

var deviceList = [];

[stevenjb, 2015/07/30 18:28:51]

Or actually:
/** @type {!Array<!BluetoothDevice>} */ var device_list = [];

[xiyuan, 2015/07/30 18:33:57]

device_list -< deviceList, jsdoc is nice.

[rfrappier, 2015/07/30 22:21:38]

Done.

[xiyuan, 2015/07/30 23:30:37]

It would be nice to follow stevenjb@'s suggestion to add a jsdoc.
/** @type {!Array<!BluetoothDevice>} */

[rfrappier, 2015/07/31 00:55:59]

Done.

+
+    for (var i = 0; i < devices.length; ++i) {
+      if (this.devicePaths[devices[i]['path']] == undefined) {

[stevenjb, 2015/07/30 18:28:51]

if (this.devicePaths[devices[i]['path']] != undefined)
  continue;

Or maybe { device_list.push(devices[i]); continue; } ?

[rfrappier, 2015/07/30 22:21:38]

Done.

+        // Get the label for the device class which should be selected.
+        var selectedClass =
+            this.getTextForDeviceClass(devices[i]['classValue']);
+        devices[i].class = selectedClass;
+        device_list.push(devices[i]);
+        this.devicePaths[devices[i]['path']] = true;
+      }
+    }
+
+    this.devices = device_list;

[xiyuan, 2015/07/30 17:21:21]

|device_list| contains only new devices at this point. This would cause all know
devices to disappear. Is this intentional?

[rfrappier, 2015/07/30 22:21:38]

This function is only called in the very beginning after the page loads.
Currently, predefined devices do not exist, so |this.devices| is empty before
this line.

[xiyuan, 2015/07/30 23:30:37]

If we are sure this is only intended to be called at the beginning of the page
load, why bother doing the devicePaths check. It should be empty at the page
load too, right?

I would suggest either remove the devicePaths check or change the function name
to better describe what it does.  

[rfrappier, 2015/07/31 00:55:59]

Done.

+  },
+
   pairDevice: function(e) {
     var device = this.devices[e.path[2].dataIndex];
     device.classValue = this.getValueForDeviceClass(device.class);
+    this.devicePaths[device.path] = true;
 
     // Send device info to the WebUI.
     chrome.send('requestBluetoothPair', [device]);
@@ -89,18 +125,41 @@ Polymer({
   discoverDevice: function(e) {
     var device = this.devices[e.path[2].dataIndex];
     device.classValue = this.getValueForDeviceClass(device.class);
+    this.devicePaths[device.path] = true;
 
     // Send device info to WebUI.
     chrome.send('requestBluetoothDiscover', [device]);
   },
 
+  // Adds a new device with default settings to the list of devices.
   appendNewDevice: function() {
     this.push('devices', new BluetoothDevice());
   },
 
-  getValueForDeviceClass: function(classValue) {
+  // This is called when a new device is discovered by the main adapter.
+  // The device is only added to the view's list if it is not already in
+  // the list (i.e. its path has not yet been recorded in |devicePaths|).

[stevenjb, 2015/07/30 18:28:51]

@param

[rfrappier, 2015/07/30 22:21:38]

Done.

+  addBluetoothDevice: function(device) {
+    if (this.devicePaths[device['path']] == undefined) {

[stevenjb, 2015/07/30 18:28:51]

if (this.devicePaths[device['path']] != undefined)
  return;

[rfrappier, 2015/07/30 22:21:38]

Done.

+      var selectedClass = this.getTextForDeviceClass(device['classValue']);
+      device.class = selectedClass;
+      this.push('devices', device);
+      this.devicePaths[device['path']] = true;
+    }
+  },
+
+  // Returns the text for the label that corresponds to |classValue|.

[stevenjb, 2015/07/30 18:28:51]

@param

[rfrappier, 2015/07/30 22:21:38]

Done.

+  getTextForDeviceClass: function(classValue) {

[xiyuan, 2015/07/30 17:21:22]

We probably should consider move this.deviceClassOptions out of the object and
make it a global const and maybe make it a direct map between label and value
for easier look up.

Make getTextForDeviceClass and getValueForDeviceClass to global helper functions
too.

e.g. Make them part of BluetoothDevice.

BluetoothDevice.DEVICE_CLASS_OPTIONS = {
  'Unknown' : 0,
  'Mouse': 0x2580,
  ...
};

BluetoothDevice.getTextForDeviceClass = function(classValue) {
  for (var className in BluetoothDevice.DEVICE_CLASS_OPTIONS) {
    if (BluetoothDevice.DEVICE_CLASS_OPTIONS[className] == classValue)
      return className;
  }
  return '';  // What should we return here? 'Invalid'?
};

BluetoothDevice.getValueForDeviceClass = function(className) {
  return BluetoothDevice.DEVICE_CLASS_OPTIONS[className];
};

[rfrappier, 2015/07/30 22:21:38]

This may be possible, and I agree that this makes sense from an OO perspective.
However, it may make working with Polymer's binding difficult. As far as I know,
Polymer's template repeat cannot iterate through keys in an object--it can only
do arrays. Also, I cannot find any information saying that Polyemr supports
accessing global variables from its binding syntax (and local experimentation
seems to suggest that it does not). This means I would have to have a property
in the Polymer object which contains key/value pairs for DEVICE_CLASS_OPTIONS
anyway.

I'm no expert on Polymer, so stevenjb@ or xiyuan@ please correct me if I'm wrong
here.

[xiyuan, 2015/07/30 23:30:37]

I don't understand why it would not work with Polymer. We just define a couple
global function. BluetoothDevice.getXXX is just a nice name so that we don't
pollute the global namespace. You can call them without an instance of
BluetoothDevice. Why it is a problem for Polymer?

[rfrappier, 2015/07/31 00:55:59]

The problem is that Polymer's data binding cannot access the global namespace.
If we put things in the global namespace, then we'll have to create a property
in the Polymer object that references that value. For example, having

BluetoothDevice.DEVICE_CLASS_OPTIONS = { ... }

would require a new property or method in the Polymer object for data binding to
work. So something like 

Polymer({
  properties: {
    ...
    deviceClassOptions: {
      type: Object,
      value: function() { return BluetoothDevice.DEVICE_CLASS_OPTIONS; }
    }
  }
});

The problem with this approach is that deviceClassOptions is an object, which is
not iterable in Polymer's data binding. This means that you'd have to create
another method to return the keys of the object like so

Polymer({
  ...
  deviceClassOptionKeys: function() { 
    return Object.keys(this.deviceClassOptions); 
  }
  ...
});

and then the data binding in the HTML would look like this:

<template is="dom-repeat" items="{{ key in deviceClassOptionKeys() }}">
  <!-- For printing the label. -->
  ... {{ key }} ...
  <!-- For printing the value -->
  ... {{ deviceClassOptions[key] }} ...
</template>

This approach works, but I think it's quite a bit of extra code for something so
simple. Would you prefer that I implement it this way?

[rfrappier, 2015/07/31 01:09:53]

Also, I think having this property in the Polyemr object makes more sense when
you think about what it is. |deviceClassOptions| stores data that is meant to be
displayed in the view (label and value of <paper-radio-button> elements)--this
is a part of the view, and therefore should be a part of the Polymer object
which is used to construct the view (think of this from a Model-View-ViewModel
perspective). Moving this elsewhere seems counter-intuitive to me (as per my
other comment).

[xiyuan, 2015/07/31 02:00:00]

Why this would not work? Have you tried it? "new BluetoothDeivce()' can be used
in the polymer element. Why calling a global function and accessing global var
not working?

I made a little test here: http://plnkr.co/edit/SLhZzGtvlNyOFeFCxDKD?p=preview

Click on the "Update Global" button would invoke a handler in the
"proto-element", which would create a message using a global variable and call a
global function to set a div's content in global DOM tree.

+    for (var i = 0; i < this.deviceClassOptions.length; ++i) {
+      if (this.deviceClassOptions[i].value == classValue)
+        return this.deviceClassOptions[i].text;
+    }
+  },
+
+  // Returns the integer value which corresponds with the label |classText|.

[stevenjb, 2015/07/30 18:28:51]

@param
@return

[rfrappier, 2015/07/30 22:21:38]

Done.

+  getValueForDeviceClass: function(classText) {
     for (var i = 0; i < this.deviceClassOptions.length; ++i) {
-      if (this.deviceClassOptions[i].text == classValue)
+      if (this.deviceClassOptions[i].text == classText)
         return this.deviceClassOptions[i].value;
     }
     return 0;