Implement md-settings network lists.

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 34a4b9a30c209d605fc76408bb1a3797f7f84609..0f2983e97620b2b836c66df41dd4e4c1a3e23cdf 100644
--- a/chrome/browser/resources/settings/internet_page/network_summary.js
+++ b/chrome/browser/resources/settings/internet_page/network_summary.js
@@ -6,22 +6,78 @@
  * @fileoverview Polymer element for displaying a summary of network states
  * by type: Ethernet, WiFi, Cellular, WiMAX, and VPN.
  */
+(function() {
+
+/** @typedef {chrome.networkingPrivate.DeviceStateProperties} */
+var DeviceStateProperties;
+
+/** @typedef {chrome.networkingPrivate.NetworkStateProperties} */
+var NetworkStateProperties;
+
+/**
+ * @typedef {{
+ *   Ethernet: (DeviceStateProperties|undefined),
+ *   WiFi: (DeviceStateProperties|undefined),
+ *   Cellular: (DeviceStateProperties|undefined),
+ *   WiMAX: (DeviceStateProperties|undefined),
+ *   VPN: (DeviceStateProperties|undefined)
+ * }}
+ */
+var DeviceStateObject;
+
+/**
+ * @typedef {{
+ *   Ethernet: (CrOncDataElement|undefined),
+ *   WiFi: (CrOncDataElement|undefined),
+ *   Cellular: (CrOncDataElement|undefined),
+ *   WiMAX: (CrOncDataElement|undefined),
+ *   VPN: (CrOncDataElement|undefined)
+ * }}
+ */
+var NetworkStateObject;
+
+/**
+ * @typedef {{
+ *   Ethernet: (Array<CrOncDataElement>|undefined),
+ *   WiFi: (Array<CrOncDataElement>|undefined),
+ *   Cellular: (Array<CrOncDataElement>|undefined),
+ *   WiMAX: (Array<CrOncDataElement>|undefined),
+ *   VPN: (Array<CrOncDataElement>|undefined)
+ * }}
+ */
+var NetworkStateListObject;
+
+/** @const {Array<string>} */ var NETWORK_TYPES =

[Jeremy Klein, 2015/04/23 18:21:15]

nit: I think this would read cleaner like:

/** @const {!Array<string>} */
var NETWORK_TYPES = ['Ethernet', ...

[stevenjb, 2015/04/24 01:25:25]

Done.

+    ['Ethernet', 'WiFi', 'Cellular', 'WiMAX', 'VPN'];
+
 Polymer('cr-network-summary', {
   publish: {
     /**
+     * The device state for each network device type.
+     *
+     * @attribute deviceStates
+     * @type DeviceStateObject
+     * @default null
+     */
+    deviceStates: null,
+
+    /**
      * Network state data for each network type.
      *
      * @attribute networkStates
-     * @type {{
-     *   Ethernet: (CrOncDataElement|undefined),
-     *   WiFi: (CrOncDataElement|undefined),
-     *   Cellular: (CrOncDataElement|undefined),
-     *   WiMAX: (CrOncDataElement|undefined),
-     *   VPN: (CrOncDataElement|undefined)
-     * }}
-     * @default {}
+     * @type {NetworkStateObject}
+     * @default null
      */
     networkStates: null,
+
+    /**
+     * List of network state data for each network type.
+     *
+     * @attribute networkStateLists
+     * @type {NetworkStateListObject}
+     * @default null
+     */
+    networkStateLists: null,
   },
 
   /**
@@ -29,7 +85,15 @@ Polymer('cr-network-summary', {
    * @type {function(!Array<string>)}
    * @private
    */
-  listChangedListener_: null,
+  networkListChangedListener_: null,
+
+  /**
+   * Listener function for chrome.networkingPrivate.onDeviceStateListChanged
+   * event.
+   * @type {function(!Array<string>)}
+   * @private
+   */
+  deviceStateListChangedListener_: null,
 
   /**
    * Listener function for chrome.networkingPrivate.onNetworksChanged event.
@@ -47,16 +111,24 @@ Polymer('cr-network-summary', {
 
   /** @override */
   created: function() {
+    this.deviceStates = {};
     this.networkStates = {};
+    this.networkStateLists = {};
   },
 
   /** @override */
   attached: function() {
     this.getNetworks_();
 
-    this.listChangedListener_ = this.onNetworkListChangedEvent_.bind(this);
+    this.networkListChangedListener_ =
+        this.onNetworkListChangedEvent_.bind(this);
     chrome.networkingPrivate.onNetworkListChanged.addListener(
-        this.listChangedListener_);
+        this.networkListChangedListener_);
+
+    this.deviceStateListChangedListener_ =
+        this.onDeviceStateListChangedEvent_.bind(this);
+    chrome.networkingPrivate.onDeviceStateListChanged.addListener(
+        this.deviceStateListChangedListener_);
 
     this.networksChangedListener_ = this.onNetworksChangedEvent_.bind(this);
     chrome.networkingPrivate.onNetworksChanged.addListener(
@@ -66,20 +138,72 @@ Polymer('cr-network-summary', {
   /** @override */
   detached: function() {
     chrome.networkingPrivate.onNetworkListChanged.removeListener(
-        this.listChangedListener_);
+        this.networkListChangedListener_);
+
+    chrome.networkingPrivate.onDeviceStateListChanged.removeListener(
+        this.deviceStateListChangedListener_);
 
     chrome.networkingPrivate.onNetworksChanged.removeListener(
         this.networksChangedListener_);
   },
 
   /**
+   * @param {string} deviceState The state of the device.
+   * @return {boolean} Whether or not an item for the device type is visible.
    * @private
    */
-  onNetworkListChangedEvent_: function() {
-    this.getNetworks_();
+  itemIsVisible: function(deviceState) { return !!deviceState; },

[Jeremy Klein, 2015/04/23 18:21:14]

Trailing _.

[stevenjb, 2015/04/24 01:25:25]

I haven't use a trailing _ in methods that are used in data binding in the HTML
because it looks odd to me, but I don't feel strongly either way. We should be
consistent either way throughout Settings, so we should pick an approach and
stick with it.

[Jeremy Klein, 2015/04/24 02:55:28]

This is part of the google style guide and I think either the compiler or linter
will complain if you have @private and no trailing underscore. Please add it
everywhere.

[stevenjb, 2015/04/24 19:03:18]

Done.

+
+  /**
+   * Event triggered when a cr-network-summary-item is expanded.
+   * @param {{detail: {expanded: boolean, type: string}}} event
+   * @private
+   */
+  onExpanded: function(event) {

[Jeremy Klein, 2015/04/23 18:21:15]

Trailing _.

[stevenjb, 2015/04/24 01:25:25]

Acknowledged.

+    if (event.detail.type == 'WiFi')

[Jeremy Klein, 2015/04/23 18:21:14]

Why is this needed if this function is only ever called when the wifi item is
expanded? Why not just call it onWifiExpanded?

[stevenjb, 2015/04/24 01:25:25]

In the future we may do things for other types, but we can add new methods for
those, so I'll rename this and remove the if.

+      chrome.networkingPrivate.requestNetworkScan();
   },
 
   /**
+   * Event triggered when a cr-network-summary-item is selected.
+   * @param {{detail: CrOncDataElement}} event

[michaelpg, 2015/04/23 02:16:29]

!CrOncDataElement

[stevenjb, 2015/04/24 01:25:25]

Done.

+   * @private
+   */
+  onSelected: function(event) {

[Jeremy Klein, 2015/04/23 18:21:14]

Trailing _.

[stevenjb, 2015/04/24 01:25:25]

Acknowledged.

+    var onc = event.detail;
+    if (onc.disconnected()) {
+      this.connectToNetwork_(onc);
+      return;
+    }
+    // TODO(stevenjb): Show details for connected or unconfigured networks.
+  },
+
+  /**
+   * Event triggered when the enabled state of a cr-network-summary-item is
+   * toggled.
+   * @param {{detail: {enabled: boolean, type: string}}} event
+   */
+  onToggleEnabled: function(event) {

[Jeremy Klein, 2015/04/23 18:21:14]

@private?

Also nit: maybe just onToggled since this is also called when the toggle is
disabled.

[stevenjb, 2015/04/24 01:25:25]

Done.

Well, it's toggling the "enabled" state (vs. e.g. the visibility of the list),
so I think the current name is maybe slightly less confusing?

[michaelpg, 2015/04/24 19:35:37]

I think it's confusing too -- it implies the event is triggered when "toggle" is
enabled.

It sounds kind of silly but "onEnabledToggled" makes it clear to me what the
event represents and follows the same sort of grammar other events use.

onSelected: [when] this [was] selected
onNetworkListChangedEvent: [when] network list [was] changed
onEnabledToggled: [when] enabled [was] toggled

[stevenjb, 2015/04/24 21:18:07]

OK, I made this as explicit as I felt I reasonably should.

Button -> deviceEnabledButton
Event -> device-enabled-toggled
Function -> onDeviceEnabledToggled_ (here and in network_summary_item)

+    if (event.detail.enabled)
+      chrome.networkingPrivate.enableNetworkType(event.detail.type);
+    else
+      chrome.networkingPrivate.disableNetworkType(event.detail.type);
+  },
+
+  /**
+   * networkingPrivate.onNetworkListChanged event callback.
+   * @private
+   */
+  onNetworkListChangedEvent_: function() { this.getNetworks_(); },

[Jeremy Klein, 2015/04/23 18:21:15]

It doesn't seem like this function and onDeviceStateListChangedEvent are
necessary. Why not just bind directly to this.getNetworks_ in both cases?

[stevenjb, 2015/04/24 01:25:25]

While currently true, I kind of prefer to keep these separate because:
a) I think it makes the asynchronous nature of the callback a little more clear.
b) We may do more in these later (which I realize we could change at that time,
but see 'a').

[Jeremy Klein, 2015/04/24 02:55:28]

I still don't agree with this, but won't block on it.

[stevenjb, 2015/04/24 19:03:18]

Acknowledged.

+
+  /**
+   * networkingPrivate.onDeviceStateListChanged event callback.
+   * @private
+   */
+  onDeviceStateListChangedEvent_: function() { this.getNetworks_(); },
+
+  /**
+   * networkingPrivate.onNetworksChanged event callback.
    * @param {!Array<string>} networkIds The list of changed network GUIDs.
    * @private
    */
@@ -92,48 +216,97 @@ Polymer('cr-network-summary', {
     }, this);
   },
 
-  /** @private */
+  /**
+   * Handles UI requests to connect to a network.
+   * TODO(stevenjb): Handle Cellular activation, etc.
+   * @param {CrOncDataElement} state The network state.

[Jeremy Klein, 2015/04/23 18:21:14]

!CrOncDataElement

[stevenjb, 2015/04/24 01:25:25]

Done.

+   * @private
+   */
+  connectToNetwork_: function(state) {
+    chrome.networkingPrivate.startConnect(state.data.GUID);
+  },
+
+  /**
+   * Requests the list of device states and network states from Chrome.
+   * Updates deviceStates, networkStates, and networkStateLists once the
+   * results are returned from Chrome.
+   * @private
+   */
   getNetworks_: function() {
-    var filter = {
-      networkType: 'All',
-      visible: true,
-      configured: false
-    };
-    chrome.networkingPrivate.getNetworks(filter,
-                                         this.getNetworksCallback_.bind(this));
+    // First get the device states.
+    chrome.networkingPrivate.getDeviceStates(
+      function(states) {
+        this.getDeviceStatesCallback_(states);
+        var filter = {
+          networkType: 'All',
+          visible: true,
+          configured: false
+        };
+        // Second get the network states.
+        chrome.networkingPrivate.getNetworks(
+            filter, this.getNetworksCallback_.bind(this));
+      }.bind(this));
   },
 
   /**
-   * @param {!Array<!chrome.networkingPrivate.NetworkStateProperties>} states
-   *     The state properties for all networks.
+   * networkingPrivate.getDeviceStates callback.
+   * @param {!Array<!DeviceStateProperties>} states The state properties for all
+   *     available devices.
+   * @private
+   */
+  getDeviceStatesCallback_: function(states) {
+    states.forEach(function(state) {
+      this.deviceStates[state.Type] = state;
+    }, this);
+  },
+
+  /**
+   * networkingPrivate.getNetworksState callback.
+   * @param {!Array<!NetworkStateProperties>} states The state properties for
+   *     all visible networks.
    * @private
    */
   getNetworksCallback_: function(states) {
-    // Clear all active networks.
+    // Clear any current networks.
     this.networkIds_ = {};
 
     // Get the first (active) state for each type.
     var foundTypes = {};
+    /** @type {NetworkStateListObject} */ var oncNetworks = {
+      Ethernet: [],
+      WiFi: [],
+      Cellular: [],
+      WiMAX: [],
+      VPN: []
+    };
     states.forEach(function(state) {
       var type = state.Type;
       if (!foundTypes[type]) {
         foundTypes[type] = true;
         this.updateNetworkState_(type, state);
       }
+      oncNetworks[type].push(CrOncDataElement.create(state));
     }, this);
 
-    // Set any types not found to null. TODO(stevenjb): Support types that are
-    // disabled but available with no active network.
-    var types = ['Ethernet', 'WiFi', 'Cellular', 'WiMAX', 'VPN'];
-    types.forEach(function(type) {
+    // Set any types not found to null.
+    NETWORK_TYPES.forEach(function(type) {
       if (!foundTypes[type])
         this.updateNetworkState_(type, null);
     }, this);
+
+    // Set the network list for each type.
+    NETWORK_TYPES.forEach(function(type) {
+      this.networkStateLists[type] = oncNetworks[type];
+    }, this);
+
+    // Create a VPN entry in deviceStates if there are any VPN networks.
+    if (this.networkStateLists.VPN && this.networkStateLists.VPN.length > 0)
+      this.deviceStates.VPN = { Type: 'VPN', State: 'Enabled' };
   },
 
   /**
-   * @param {!chrome.networkingPrivate.NetworkStateProperties} state The state
-   *     properties for the network.
+   * networkingPrivate.getState callback.
+   * @param {!NetworkStateProperties} state The network state properties.
    * @private
    */
   getStateCallback_: function(state) {
@@ -148,9 +321,9 @@ Polymer('cr-network-summary', {
    * Sets 'networkStates[type]' which will update the cr-network-list-item
    * associated with 'type'.
    * @param {string} type The network type.
-   * @param {chrome.networkingPrivate.NetworkStateProperties} state The state
-   *     properties for the network to associate with |type|. May be null if
-   *     there are no networks matching |type|.
+   * @param {NetworkStateProperties} state The state properties for the network
+   *     to associate with |type|. May be null if there are no networks matching
+   *     |type|.
    * @private
    */
   updateNetworkState_: function(type, state) {
@@ -159,3 +332,4 @@ Polymer('cr-network-summary', {
       this.networkIds_[state.GUID] = true;
   },
 });
+})();