MD Settings: Internet: Reduce use of hidden for complex sections.

chrome/browser/resources/settings/internet_page/network_summary.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 a summary of network states
  * by type: Ethernet, WiFi, Cellular, WiMAX, and VPN.
  */
 
 /** @typedef {chrome.networkingPrivate.DeviceStateProperties} */
 var DeviceStateProperties;
 
 /**
  * @typedef {{
  *   Ethernet: (DeviceStateProperties|undefined),
  *   WiFi: (DeviceStateProperties|undefined),
  *   Cellular: (DeviceStateProperties|undefined),
  *   WiMAX: (DeviceStateProperties|undefined),
  *   VPN: (DeviceStateProperties|undefined)
  * }}
  */
 var DeviceStateObject;
 
 /**
  * @typedef {{
- *   Ethernet: (!CrOnc.NetworkStateProperties|undefined),
- *   WiFi: (!CrOnc.NetworkStateProperties|undefined),
- *   Cellular: (!CrOnc.NetworkStateProperties|undefined),
- *   WiMAX: (!CrOnc.NetworkStateProperties|undefined),
- *   VPN: (!CrOnc.NetworkStateProperties|undefined)
- * }}
- */
-var NetworkStateObject;
-
-/**
- * @typedef {{
  *   Ethernet: (Array<CrOnc.NetworkStateProperties>|undefined),
  *   WiFi: (Array<CrOnc.NetworkStateProperties>|undefined),
  *   Cellular: (Array<CrOnc.NetworkStateProperties>|undefined),
  *   WiMAX: (Array<CrOnc.NetworkStateProperties>|undefined),
  *   VPN: (Array<CrOnc.NetworkStateProperties>|undefined)
  * }}
  */
 var NetworkStateListObject;
 
-(function() {
-
-/** @const {!Array<chrome.networkingPrivate.NetworkType>} */
-var NETWORK_TYPES = [
-  CrOnc.Type.ETHERNET,
-  CrOnc.Type.WI_FI,
-  CrOnc.Type.CELLULAR,
-  CrOnc.Type.WI_MAX,
-  CrOnc.Type.VPN
-];
-
 Polymer({
   is: 'network-summary',
 
   properties: {
     /**
      * Highest priority connected network or null.
      * @type {?CrOnc.NetworkStateProperties}
      */
     defaultNetwork: {
       type: Object,
       value: null,
-      notify: true
+      notify: true,
     },
 
     /**
      * The device state for each network device type.
      * @type {DeviceStateObject}
      */
     deviceStates: {
       type: Object,
       value: function() { return {}; },
     },
 
     /**
-     * Network state data for each network type.
-     * @type {NetworkStateObject}
+     * Array of active network states, one per device type.
+     * @type {!Array<!CrOnc.NetworkStateProperties>}
      */
-    networkStates: {
+    activeNetworkStates: {
       type: Object,

[michaelpg, 2016/07/20 19:36:37]

Array

[stevenjb, 2016/07/20 20:29:16]

Done.

-      value: function() { return {}; },
+      value: function() { return []; },
     },
 
     /**
      * List of network state data for each network type.
      * @type {NetworkStateListObject}
      */
     networkStateLists: {
       type: Object,
       value: function() { return {}; },
     },
 
     /**
      * Interface for networkingPrivate calls, passed from internet_page.
      * @type {NetworkingPrivate}
      */
     networkingPrivate: {
       type: Object,
-    }
+    },
   },
 
   /**
    * Listener function for chrome.networkingPrivate.onNetworkListChanged event.
    * @type {function(!Array<string>)}
    * @private
    */
   networkListChangedListener_: function() {},
 
   /**
    * Listener function for chrome.networkingPrivate.onDeviceStateListChanged
    * event.
    * @type {function(!Array<string>)}
    * @private
    */
   deviceStateListChangedListener_: function() {},
 
   /**
    * Listener function for chrome.networkingPrivate.onNetworksChanged event.
    * @type {function(!Array<string>)}
    * @private
    */
   networksChangedListener_: function() {},
 
   /**
-   * Dictionary of GUIDs identifying primary (active) networks for each type.
-   * @type {?Object}
+   * Set of GUIDs identifying active networks for each type.

[michaelpg, 2016/07/20 19:36:37]

how does a Set of GUIDS identify networks on a per-type basis?

or should the comment just delete "for each type"

[stevenjb, 2016/07/20 20:29:16]

Updated comment.

+   * @type {?Set<string>}
    * @private
    */
-  networkIds_: null,
+  activeNetworkIds_: null,
 
   /** @override */
   attached: function() {
-    this.networkIds_ = {};
-
     this.getNetworkLists_();
 
     this.networkListChangedListener_ =
         this.onNetworkListChangedEvent_.bind(this);
     this.networkingPrivate.onNetworkListChanged.addListener(
         this.networkListChangedListener_);
 
     this.deviceStateListChangedListener_ =
         this.onDeviceStateListChangedEvent_.bind(this);
     this.networkingPrivate.onDeviceStateListChanged.addListener(
@@ skipping 10 matching lines @@
         this.networkListChangedListener_);
 
     this.networkingPrivate.onDeviceStateListChanged.removeListener(
         this.deviceStateListChangedListener_);
 
     this.networkingPrivate.onNetworksChanged.removeListener(
         this.networksChangedListener_);
   },
 
   /**
-   * Event triggered when the WiFi network-summary-item is expanded.
+   * Event triggered when the network-summary-item is expanded.
    * @param {!{detail: {expanded: boolean, type: string}}} event
    * @private
    */
-  onWiFiExpanded_: function(event) {
+  onExpanded_: function(event) {
     if (!event.detail.expanded)
       return;
-    // Get the latest network states (only).
+    // Get the latest network states.
     this.getNetworkStates_();
-    this.networkingPrivate.requestNetworkScan();
+    if (event.detail.type == CrOnc.Type.WI_FI)
+      this.networkingPrivate.requestNetworkScan();
   },
 
   /**
    * Event triggered when a network-summary-item is selected.
    * @param {!{detail: !CrOnc.NetworkStateProperties}} event
    * @private
    */
   onSelected_: function(event) {
     var state = event.detail;
     if (this.canConnect_(state)) {
@@ skipping 28 matching lines @@
    * @private
    */
   onDeviceStateListChangedEvent_: function() { this.getNetworkLists_(); },
 
   /**
    * networkingPrivate.onNetworksChanged event callback.
    * @param {!Array<string>} networkIds The list of changed network GUIDs.
    * @private
    */
   onNetworksChangedEvent_: function(networkIds) {
+    if (!this.activeNetworkIds_)
+      return;  // Initial list of networks not received yet.
     networkIds.forEach(function(id) {
-      if (id in this.networkIds_) {
+      if (this.activeNetworkIds_.has(id)) {
         this.networkingPrivate.getState(
-            id, this.getStateCallback_.bind(this, id));
+            id, this.getActiveStateCallback_.bind(this, id));
       }
     }, this);
   },
 
   /**
    * Determines whether or not a network state can be connected to.
    * @param {!CrOnc.NetworkStateProperties} state The network state.
    * @private
    */
   canConnect_: function(state) {
     if (state.Type == CrOnc.Type.ETHERNET ||
         state.Type == CrOnc.Type.VPN && !this.defaultNetwork) {
       return false;
     }
     return state.ConnectionState == CrOnc.ConnectionState.NOT_CONNECTED;
   },
 
   /**
-   * networkingPrivate.getState event callback.
+   * networkingPrivate.getState event callback for an active state.
    * @param {string} id The id of the requested state.
    * @param {!chrome.networkingPrivate.NetworkStateProperties} state
    * @private
    */
-  getStateCallback_: function(id, state) {
+  getActiveStateCallback_: function(id, state) {
     if (chrome.runtime.lastError) {
       var message = chrome.runtime.lastError.message;
       if (message != 'Error.NetworkUnavailable') {
         console.error(
             'Unexpected networkingPrivate.getState error: ' + message +
             ' For: ' + id);
       }
       return;
     }
     // Async call, ensure id still exists.
-    if (!this.networkIds_[id])
+    if (!this.activeNetworkIds_.has(id))
       return;
     if (!state) {
-      this.networkIds_[id] = undefined;
+      this.activeNetworkIds_.delete(id);
       return;
     }
-    this.updateNetworkState_(state.Type, state);
+    // Find the active state for the type and update it.
+    for (let i = 0; i < this.activeNetworkStates.length; ++i) {
+      if (this.activeNetworkStates[i].type == state.type) {
+        this.activeNetworkStates[i] = state;
+        return;
+      }
+    }
+    // Not found
+    console.error('Active state not found: ' + state.Name);
   },
 
   /**
    * Handles UI requests to connect to a network.
    * TODO(stevenjb): Handle Cellular activation, etc.
    * @param {!CrOnc.NetworkStateProperties} state The network state.
    * @private
    */
   connectToNetwork_: function(state) {
     this.networkingPrivate.startConnect(state.GUID, function() {
       if (chrome.runtime.lastError) {
         var message = chrome.runtime.lastError.message;
         if (message != 'connecting') {
           console.error(
               'Unexpected networkingPrivate.startConnect error: ' + message +
               'For: ' + state.GUID);
         }
       }
     });
   },
 
   /**
    * Requests the list of device states and network states from Chrome.
-   * Updates deviceStates, networkStates, and networkStateLists once the
+   * Updates deviceStates, activeNetworkStates, and networkStateLists once the
    * results are returned from Chrome.
    * @private
    */
   getNetworkLists_: function() {
     // First get the device states.
-    this.networkingPrivate.getDeviceStates(
-      function(deviceStates) {
-        // Second get the network states.
-        this.getNetworkStates_(deviceStates);
-      }.bind(this));
+    this.networkingPrivate.getDeviceStates(function(deviceStates) {
+      // Second get the network states.
+      this.getNetworkStates_(deviceStates);
+    }.bind(this));
   },
 
   /**
-   * Requests the list of network states from Chrome. Updates networkStates and
-   * networkStateLists once the results are returned from Chrome.
+   * Requests the list of network states from Chrome. Updates
+   * activeNetworkStates and networkStateLists once the results are returned
+   * from Chrome.
    * @param {!Array<!DeviceStateProperties>=} opt_deviceStates
    *     Optional list of state properties for all available devices.
    * @private
    */
   getNetworkStates_: function(opt_deviceStates) {
     var filter = {
       networkType: chrome.networkingPrivate.NetworkType.ALL,
       visible: true,
       configured: false
     };
     this.networkingPrivate.getNetworks(filter, function(networkStates) {
       this.updateNetworkStates_(networkStates, opt_deviceStates);
     }.bind(this));
   },
 
   /**
    * Called after network states are received from getNetworks.
    * @param {!Array<!CrOnc.NetworkStateProperties>} networkStates The state
    *     properties for all visible networks.
    * @param {!Array<!DeviceStateProperties>=} opt_deviceStates
    *     Optional list of state properties for all available devices. If not
    *     defined the existing list of device states will be used.
    * @private
    */
   updateNetworkStates_: function(networkStates, opt_deviceStates) {
     var newDeviceStates;
     if (opt_deviceStates) {
-      newDeviceStates = /** @type {!DeviceStateObject} */({});
-      opt_deviceStates.forEach(function(state) {
+      newDeviceStates = /** @type {!DeviceStateObject} */ ({});
+      for (let state of opt_deviceStates)
         newDeviceStates[state.Type] = state;
-      });
     } else {
       newDeviceStates = this.deviceStates;
     }
 
     // Clear any current networks.
-    this.networkIds_ = {};
-
-    // Track the first (active) state for each type.
-    var foundTypes = {};
+    var activeNetworkStatesByType =
+        /** @type {!Map<string, !CrOnc.NetworkStateProperties>} */ (new Map);
 
     // Complete list of states by type.
-    /** @type {!NetworkStateListObject} */ var networkStateLists = {
+    /** @type {!NetworkStateListObject} */ var newNetworkStateLists = {
       Ethernet: [],
       WiFi: [],
       Cellular: [],
       WiMAX: [],
-      VPN: []
+      VPN: [],
     };
 
     var firstConnectedNetwork = null;
     networkStates.forEach(function(state) {
-      var type = state.Type;
-      if (!foundTypes[type]) {
-        foundTypes[type] = true;
-        this.updateNetworkState_(type, state);
+      let type = state.Type;
+      if (!activeNetworkStatesByType.has(type)) {
+        activeNetworkStatesByType.set(type, state);
         if (!firstConnectedNetwork && state.Type != CrOnc.Type.VPN &&
             state.ConnectionState == CrOnc.ConnectionState.CONNECTED) {
           firstConnectedNetwork = state;
         }
       }
-      networkStateLists[type].push(state);
+      newNetworkStateLists[type].push(state);
     }, this);
 
     this.defaultNetwork = firstConnectedNetwork;
 
-    // Set any types with a deviceState and no network to a default state,
-    // and any types not found to undefined.
-    NETWORK_TYPES.forEach(function(type) {
-      if (!foundTypes[type]) {
-        var defaultState = undefined;
-        if (newDeviceStates[type])
-          defaultState = {GUID: '', Type: type};
-        this.updateNetworkState_(type, defaultState);
-      }
-    }, this);
-
-    this.networkStateLists = networkStateLists;
-
     // Create a VPN entry in deviceStates if there are any VPN networks.
-    if (networkStateLists.VPN && networkStateLists.VPN.length > 0) {
+    if (newNetworkStateLists.VPN && newNetworkStateLists.VPN.length > 0) {
       newDeviceStates.VPN = /** @type {DeviceStateProperties} */ ({
         Type: CrOnc.Type.VPN,
         State: chrome.networkingPrivate.DeviceStateType.ENABLED
       });
     }
 
+    // Push the active networks onto newActiveNetworkStates in device order,
+    // creating an empty state for devices with no networks.
+    var newActiveNetworkStates = [];
+    this.activeNetworkIds_ = new Set;
+    for (let type in newDeviceStates) {
+      var state = activeNetworkStatesByType.get(type) || {GUID: '', Type: type};

[michaelpg, 2016/07/20 19:36:37]

opt nit: this is fine, but i would line break after the ||. fn() || {key: val}
is kinda unexpected

[stevenjb, 2016/07/20 20:29:16]

Blame clang.

+      newActiveNetworkStates.push(state);
+      this.activeNetworkIds_.add(state.GUID);
+    }
+
     this.deviceStates = newDeviceStates;
-  },
-
-  /**
-   * Sets 'networkStates[type]' which will update the
-   * cr-network-list-network-item associated with 'type'.
-   * @param {string} type The network type.
-   * @param {!CrOnc.NetworkStateProperties|undefined} state The state properties
-   *     for the network to associate with |type|. May be undefined if there are
-   *     no networks matching |type|.
-   * @private
-   */
-  updateNetworkState_: function(type, state) {
-    this.set('networkStates.' + type, state);
-    if (state)
-      this.networkIds_[state.GUID] = true;
+    this.networkStateLists = newNetworkStateLists;
+    // Set activeNetworkStates last to rebuild the dom-repeat.
+    this.activeNetworkStates = newActiveNetworkStates;
   },
 });
-})();