Add Settings bluetooth page test

chrome/test/data/webui/settings/fake_bluetooth.js

+// 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.
+
+/**
+ * @fileoverview Fake implementations of chrome.bluetooth and for testing.

[dpapad, 2015/12/16 22:07:49]

s/and//

[stevenjb, 2015/12/17 00:13:34]

Done.

+ */
+cr.define('settings', function() {
+  /**
+   * Fake of the chrome.bluetooth API.
+   * @constructor
+   * @implements {Bluetooth}
+   */
+  function FakeBluetooth() {
+    this.enabled = false;

[dpapad, 2015/12/16 22:07:49]

Should we add type annotations for all members? I think it is helpful even if
this file is not being compiled.

[stevenjb, 2015/12/17 00:13:34]

Personally I think that typing everything quickly becomes tedious and loses some
of the benefits of Javascript's flexibility. For exposed (public) members and
functions I agree that it is extremely helpful, but for a local member like this
which is clearly a boolean, I don't see the advantage.

If the consensus differs, I can be more diligent, but I do feel that it would
slow me down.

[dpapad, 2015/12/17 01:12:54]

If you don't add @private and/or rename it to this.enabled_, it is not clear at
all that this is a private member (see
https://google.github.io/styleguide/javascriptguide.xml?showone=Naming#Naming
for trailing underscore guideline). The reader has to search the file everytime
to determine whether this is a private field. At the very least I would suggest
adding the underscore at the end, and we can discuss whether we need to add
types next time we meet (meaning when everyone is back from vacation).

[stevenjb, 2015/12/17 01:28:57]

So, this isn't actually private, we do use it elsewhere, so my own argument
suggests that we should document these. Done.

+    this.devices = [];
+
+    this.setDevicesForTest = function(devices) {

[dpapad, 2015/12/16 22:07:49]

Can we move this on the prototype? Methods have no good reason to be defined in
the constructor.

[stevenjb, 2015/12/17 00:13:34]

Sure. It seemed better to put just the API implementation in the prototype, but
I'll admit I'm still unclear on the advantages of using a prototype at all for a
singleton instance like this. (I started with the FakeSettingsPrivate
implementation).

+      for (var d of this.devices)
+        this.onDeviceRemoved.callListeners(d);
+      this.devices = devices;
+      for (var d of this.devices)
+        this.onDeviceAdded.callListeners(d);
+    };
+  }
+
+  FakeBluetooth.prototype = {
+    // Bluetooth overrides.
+    /**
+     * @param {function(!chrome.bluetooth.AdapterState):void} callback
+     */
+    getAdapterState: function(callback) {

[dpapad, 2015/12/16 22:07:49]

If these methods are overrides, why do we need to replicate type annotations?
/** @override */ should be enough.

[stevenjb, 2015/12/17 00:13:34]

I didn't realize that. Done.

+      setTimeout(function() {
+        callback({
+          address: '00:11:22:33:44:55:66',
+          name: 'Fake Adapter',
+          powered: this.enabled,
+          available: true,
+          discovering: false
+        });
+      }.bind(this));
+    },
+
+    /**
+     * @param {string} deviceAddress Address of device to get.
+     * @param {function(!chrome.bluetooth.Device):void} callback
+     */
+    getDevice: assertNotReached,

[dschuyler, 2015/12/16 22:15:01]

If this were to fire (in error), how would the person debugging this error know
it was an invalid call to getDevice?  Would this be nicer as
getDevice: function(deviceAddress, callback) {
  assertNotReached();
},

[stevenjb, 2015/12/17 00:13:34]

I don't think that would help in a test where we have to rely on console output
which is just a call stack. We would have to do something like:
assertNotReached('Unexpected call, deviceAddress = ' + deviceAddress);

Which would involve a fair bit of extra work for something we don't expect to
happen.

Also, but the time this section is fully implemented and reasonably well tested,
most of this API will be implemented.

+
+    /** @param {function(!Array<!chrome.bluetooth.Device>):void} callback */
+    getDevices: function(callback) {
+      setTimeout(function() {
+        callback(this.devices);
+      }.bind(this));
+    },
+
+    /** @param {function():void=} callback */
+    startDiscovery: assertNotReached,
+
+    /** @param {function():void=} callback */
+    stopDiscovery: assertNotReached,
+
+    /** @type {!FakeChromeEvent} */
+    onAdapterStateChanged: new FakeChromeEvent(),
+
+    /** @type {!FakeChromeEvent} */
+    onDeviceAdded: new FakeChromeEvent(),
+
+    /** @type {!FakeChromeEvent} */
+    onDeviceChanged: new FakeChromeEvent(),
+
+    /** @type {!FakeChromeEvent} */
+    onDeviceRemoved: new FakeChromeEvent(),
+  };
+
+  return {FakeBluetooth: FakeBluetooth};
+});