Settings: Network: Merge Tether networks into Mobile subpage

chrome/browser/resources/settings/internet_page/network_summary_item.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 Polymer element for displaying the network state for a specific
- * type and a list of networks for that type.
+ * type and a list of networks for that type. NOTE: It both Cellular and Tether
+ * technologies are available, they are combined into a single 'Mobile data'
+ * section. See crbug.com/726380.
  */
 
 Polymer({
   is: 'network-summary-item',
 
   behaviors: [CrPolicyNetworkBehavior, I18nBehavior],
 
   properties: {
     /**
      * Device state for the network type. This might briefly be undefined if a
      * device becomes unavailable.
      * @type {!CrOnc.DeviceStateProperties|undefined}
      */
     deviceState: Object,
 
     /**
+     * If both Cellular and Tether technologies exist, we combine the sections
+     * and set this to the device state for Tether.
+     * @type {!CrOnc.DeviceStateProperties|undefined}
+     */
+    tetherDeviceState: Object,
+
+    /**
      * Network state for the active network.
      * @type {!CrOnc.NetworkStateProperties|undefined}
      */
     activeNetworkState: Object,
 
     /**
      * List of all network state data for the network type.
      * @type {!Array<!CrOnc.NetworkStateProperties>}
      */
     networkStateList: {
@@ skipping 121 matching lines @@
     return !!deviceState && deviceState.State == CrOnc.DeviceState.ENABLED;
   },
 
   /**
    * @param {!CrOnc.DeviceStateProperties|undefined} deviceState
    * @return {boolean}
    * @private
    */
   enableToggleIsVisible_: function(deviceState) {
     return !!deviceState && deviceState.Type != CrOnc.Type.ETHERNET &&
-        deviceState.Type != CrOnc.Type.VPN;
+        deviceState.Type != CrOnc.Type.VPN &&
+        deviceState.State != CrOnc.DeviceState.UNINITIALIZED;

[Kyle Horimoto, 2017/06/01 20:15:21]

If Type == CELLULAR and neither Cellular or Tether networks are available,
shouldn't we hide the toggle?

[stevenjb, 2017/06/01 21:55:03]

Enable/Disable is separate for networks existing, at least for Cellular.

   },
 
   /**
    * @param {!CrOnc.DeviceStateProperties|undefined} deviceState
    * @return {boolean}
    * @private
    */
   enableToggleIsEnabled_: function(deviceState) {
-    return !!deviceState && deviceState.State != CrOnc.DeviceState.PROHIBITED;
+    return this.enableToggleIsVisible_(deviceState) &&
+        deviceState.State != CrOnc.DeviceState.PROHIBITED;
   },
 
   /**
    * @param {!CrOnc.DeviceStateProperties|undefined} deviceState
    * @return {string}
    * @private
    */
   getToggleA11yString_: function(deviceState) {
     if (!this.enableToggleIsVisible_(deviceState))
       return '';
     switch (deviceState.Type) {
       case CrOnc.Type.TETHER:
-        return this.i18n('internetToggleTetherA11yLabel');
       case CrOnc.Type.CELLULAR:
         return this.i18n('internetToggleMobileA11yLabel');
       case CrOnc.Type.WI_FI:
         return this.i18n('internetToggleWiFiA11yLabel');
       case CrOnc.Type.WI_MAX:
         return this.i18n('internetToggleWiMAXA11yLabel');
     }
     assertNotReached();
     return '';
   },
@@ skipping 14 matching lines @@
   /**
    * @param {!CrOnc.DeviceStateProperties|undefined} deviceState
    * @param {!Array<!CrOnc.NetworkStateProperties>} networkStateList
    * @return {boolean}
    * @private
    */
   shouldShowSubpage_: function(deviceState, networkStateList) {
     if (!deviceState)
       return false;
     var type = deviceState.Type;
-    var minlen = (deviceState.Type == CrOnc.Type.WI_FI ||
-                  deviceState.Type == CrOnc.Type.VPN ||
-                  deviceState.Type == CrOnc.Type.TETHER) ?
+    if (type == CrOnc.Type.CELLULAR && this.tetherDeviceState)

[Kyle Horimoto, 2017/06/01 20:15:21]

nit: Shouldn't this be !!this.tetherDeviceState?

[stevenjb, 2017/06/01 21:55:03]

That's only necessary when you are returning a boolean type; undefined or null
objects are falsy in JS (and vice versa).

+      return true;
+    var minlen = (type == CrOnc.Type.WI_FI || type == CrOnc.Type.VPN ||
+                  type == CrOnc.Type.TETHER) ?

[Kyle Horimoto, 2017/06/01 20:15:21]

Isn't it impossible for type to be TETHER now that you eliminated the TETHER
type from being passed here in NetworkStateSummary?

[stevenjb, 2017/06/01 21:55:03]

We combine Tether with Cellular *IF* Cellular exists (i.e. is available), but if
Cellular is unavailable, type can be Tether.

         1 :
         2;
     return networkStateList.length >= minlen;
   },
 
   /**
    * @param {!Event} event The enable button event.
    * @private
    */
   onShowDetailsTap_: function(event) {
     if (!this.deviceIsEnabled_(this.deviceState)) {
-      this.fire(
-          'device-enabled-toggled',
-          {enabled: true, type: this.deviceState.Type});
+      if (this.enableToggleIsEnabled_(this.deviceState)) {
+        this.fire(
+            'device-enabled-toggled',
+            {enabled: true, type: this.deviceState.Type});
+      }
     } else if (this.shouldShowSubpage_(
                    this.deviceState, this.networkStateList)) {
       this.fire('show-networks', this.deviceState);
     } else if (this.activeNetworkState.GUID) {
       this.fire('show-detail', this.activeNetworkState);
     } else if (this.networkStateList.length > 0) {
       this.fire('show-detail', this.networkStateList[0]);
     }
     event.stopPropagation();
   },
@@ skipping 24 matching lines @@
    */
   onDeviceEnabledTap_: function(event) {
     var deviceIsEnabled = this.deviceIsEnabled_(this.deviceState);
     var type = this.deviceState ? this.deviceState.Type : '';
     this.fire(
         'device-enabled-toggled', {enabled: !deviceIsEnabled, type: type});
     // Make sure this does not propagate to onDetailsTap_.
     event.stopPropagation();
   },
 });