MD Settings: Bluetooth: Fix adapter state and discovery

chrome/browser/resources/settings/bluetooth_page/bluetooth_page.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
  * 'Settings page for managing bluetooth properties and devices. This page
  * just provodes a summary and link to the subpage.
  */
 
@@ skipping 14 matching lines @@
 Polymer({
   is: 'settings-bluetooth-page',
 
   properties: {
     /** Preferences state. */
     prefs: {
       type: Object,
       notify: true,
     },
 
-    /** @private */
-    bluetoothEnabled_: {
-      type: Boolean,
-      observer: 'bluetoothEnabledChanged_',
-      notify: true,
-    },
-
     /**
+     * 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: '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,
+    },
+
+    /**
      * The cached bluetooth adapter state.
      * @type {!chrome.bluetooth.AdapterState|undefined}
      * @private
      */
     adapterState_: {
       type: Object,
       notify: true,
     },
 
     /**
@@ skipping 51 matching lines @@
       this.bluetooth.onAdapterStateChanged.removeListener(
           this.bluetoothAdapterStateChangedListener_);
     }
   },
 
   /**
    * @return {string}
    * @private
    */
   getIcon_: function() {
-    if (!this.bluetoothEnabled_)
+    if (!this.bluetoothToggleState_)
       return 'settings:bluetooth-disabled';
     return 'settings:bluetooth';
   },
 
   /**
    * @param {boolean} enabled
    * @param {string} onstr
    * @param {string} offstr
    * @return {string}
    * @private
    */
   getOnOffString_: function(enabled, onstr, offstr) {
     return enabled ? onstr : offstr;
   },
 
   /**
    * Process bluetooth.onAdapterStateChanged events.
    * @param {!chrome.bluetooth.AdapterState} state
    * @private
    */
   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_();
   },
 
   /**
    * @param {Event} e
    * @private
    */
   stopTap_: function(e) {
     e.stopPropagation();
   },
 
   /**
    * @param {Event} e
    * @private
    */
   onSubpageArrowTap_: function(e) {
     this.openSubpage_();
     e.stopPropagation();
   },
 
   /** @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: ' +
                 chrome.runtime.lastError.message);
           }
         });
   },
 
   /** @private */
   openSubpage_: function() {
     settings.navigateTo(settings.Route.BLUETOOTH_DEVICES);
   }
 });