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

chrome/browser/resources/settings/internet_page/network_summary.js

Index: chrome/browser/resources/settings/internet_page/network_summary.js
diff --git a/chrome/browser/resources/settings/internet_page/network_summary.js b/chrome/browser/resources/settings/internet_page/network_summary.js
index fe02289950fd050e354ab4f369a457502009b713..9682cb7a5e67c7253a3590be9850afc1f0fd584b 100644
--- a/chrome/browser/resources/settings/internet_page/network_summary.js
+++ b/chrome/browser/resources/settings/internet_page/network_summary.js
@@ -23,17 +23,6 @@ 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),
@@ -43,17 +32,6 @@ var NetworkStateObject;
  */
 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',
 
@@ -65,7 +43,7 @@ Polymer({
     defaultNetwork: {
       type: Object,
       value: null,
-      notify: true
+      notify: true,
     },
 
     /**
@@ -78,12 +56,12 @@ Polymer({
     },
 
     /**
-     * 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 []; },
     },
 
     /**
@@ -101,7 +79,7 @@ Polymer({
      */
     networkingPrivate: {
       type: Object,
-    }
+    },
   },
 
   /**
@@ -127,16 +105,14 @@ Polymer({
   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_ =
@@ -167,16 +143,17 @@ Polymer({
   },
 
   /**
-   * 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();
   },
 
   /**
@@ -225,10 +202,12 @@ Polymer({
    * @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);
   },
@@ -247,12 +226,12 @@ Polymer({
   },
 
   /**
-   * 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') {
@@ -263,13 +242,21 @@ Polymer({
       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);
   },
 
   /**
@@ -293,22 +280,22 @@ Polymer({
 
   /**
    * 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
@@ -336,82 +323,62 @@ Polymer({
   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
       });
     }
 
-    this.deviceStates = newDeviceStates;
-  },
+    // 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);
+    }
 
-  /**
-   * 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.deviceStates = newDeviceStates;
+    this.networkStateLists = newNetworkStateLists;
+    // Set activeNetworkStates last to rebuild the dom-repeat.
+    this.activeNetworkStates = newActiveNetworkStates;
   },
 });
-})();