MD Settings: Bluetooth: Fix adapter state and discovery

chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js

Index: chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js
diff --git a/chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js b/chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js
index e23ed90c9e17b244d0fc70244be821926f643269..034bfcabae0df8408486cd9b6ac692f8f7798eb5 100644
--- a/chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js
+++ b/chrome/browser/resources/settings/bluetooth_page/bluetooth_page.js
@@ -32,11 +32,26 @@ Polymer({
       notify: true,
     },
 
-    /** @private */
-    bluetoothEnabled_: {
+    /**
+     * Reflects the current state of the toggle buttons. This will be set when

[michaelpg, 2017/04/07 21:47:03]

"button" singular?

[stevenjb, 2017/04/07 23:36:05]

There are two, one in this page and one in the subpage. I will clarify that.

+     * the adapter state change or when the user changes the toggle.
+     * @private
+     */
+    bluetoothToggleState_: {

[michaelpg, 2017/04/07 21:47:03]

"state" is generally an unfortunate word, but hard to get around sometimes. In
this case though, also having |adapterState_| of a completely different type
makes me think it's worth finding a better name here.

[stevenjb, 2017/04/07 23:36:05]

I agree it isn't great, but there are not a lot of good options. 'Enabled' is
worse, ambiguity wise. This represents the state of the toggle button, so I
think 'toggle state' is reasonably OK. I am open to suggestions :)

       type: Boolean,
-      observer: 'bluetoothEnabledChanged_',
-      notify: true,
+      observer: 'bluetoothToggleStateChanged_',
+    },
+
+    /**
+     * Set to true before the adapter state is received, when the adapter is
+     * unavailable, and while an adapter state change is requested. This
+     * prevents user changes while a change is in progress or when the adapter
+     * is not available.
+     * @private
+     */
+    bluetoothToggleDisabled_: {
+      type: Boolean,
+      value: true,
     },
 
     /**
@@ -111,7 +126,7 @@ Polymer({
    * @private
    */
   getIcon_: function() {
-    if (!this.bluetoothEnabled_)
+    if (!this.bluetoothToggleState_)
       return 'settings:bluetooth-disabled';
     return 'settings:bluetooth';
   },
@@ -134,15 +149,16 @@ Polymer({
    */
   onBluetoothAdapterStateChanged_: function(state) {
     this.adapterState_ = state;
-    this.bluetoothEnabled_ = state.powered;
+    this.bluetoothToggleState_ = state.powered;
+    this.bluetoothToggleDisabled_ = !state.available;
   },
 
   /** @private */
   onTap_: function() {
     if (this.adapterState_.available === false)
       return;
-    if (!this.bluetoothEnabled_)
-      this.bluetoothEnabled_ = true;
+    if (!this.bluetoothToggleState_)
+      this.bluetoothToggleState_ = true;
     else
       this.openSubpage_();
   },
@@ -165,9 +181,14 @@ Polymer({
   },
 
   /** @private */
-  bluetoothEnabledChanged_: function() {
+  bluetoothToggleStateChanged_: function() {
+    if (!this.adapterState_ || this.bluetoothToggleDisabled_ ||
+        this.bluetoothToggleState_ == this.adapterState_.powered) {
+      return;
+    }
+    this.bluetoothToggleDisabled_ = true;

[michaelpg, 2017/04/07 21:47:03]

I understand disabling at startup, but is disabling here necessary, and
definitely not noticeable?

[stevenjb, 2017/04/07 23:36:05]

We do the disable specifically for times when it is noticeable. If the user
changes the state while a pending change is in progress it will be ignored. i.e.
if the user clicks the button twice quickly they would see the following:

on -> off
off -> on (device change is in progress, so this is ignored)
on -> off (when the device state changes to off)

With the toggleDisabled state, the second click is ignored, so the user never
sees the toggle flicker on.

     this.bluetoothPrivate.setAdapterState(
-        {powered: this.bluetoothEnabled_}, function() {
+        {powered: this.bluetoothToggleState_}, function() {
           if (chrome.runtime.lastError) {
             console.error(
                 'Error enabling bluetooth: ' +