MD Settings: Internet: Move network lists to a subpage

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

Index: chrome/browser/resources/settings/internet_page/internet_subpage.js
diff --git a/chrome/browser/resources/settings/internet_page/network_summary_item.js b/chrome/browser/resources/settings/internet_page/internet_subpage.js
similarity index 30%
copy from chrome/browser/resources/settings/internet_page/network_summary_item.js
copy to chrome/browser/resources/settings/internet_page/internet_subpage.js
index a478660a30d6332ffcbc2e1fa4aedf86487afe78..8182014c558785f440b497c22a97b870120e0668 100644
--- a/chrome/browser/resources/settings/internet_page/network_summary_item.js
+++ b/chrome/browser/resources/settings/internet_page/internet_subpage.js
@@ -1,52 +1,46 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2017 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.
+ * @fileoverview Polymer element for displaying informaiton about WiFi,

[michaelpg, 2017/03/01 23:27:59]

information

[stevenjb, 2017/03/02 00:25:17]

Done.

+ * WiMAX, or virtual networks.
  */
 
 /** @typedef {chrome.networkingPrivate.DeviceStateProperties} */
 var DeviceStateProperties;
 
 Polymer({
-  is: 'network-summary-item',
+  is: 'settings-internet-subpage',
 
-  behaviors: [Polymer.IronA11yKeysBehavior, I18nBehavior],
+  behaviors: [
+    CrPolicyNetworkBehavior,
+    settings.RouteObserverBehavior,
+    I18nBehavior,
+  ],
 
   properties: {
     /**
-     * Device state for the network type.
-     * @type {!DeviceStateProperties|undefined}
+     * Highest priority connected network or null. Set by network-summary.

[michaelpg, 2017/03/01 23:27:59]

I think "Set by network-summary." can be misleading. network-summary is where
the value originates, but that's a detail for settings-internet-page, which is
what binds it to this.

[stevenjb, 2017/03/02 00:25:17]

While that is true, I think it is a useful detail for debugging. Data binding is
not super intuitive. I tried to clarify this a bit.

[michaelpg, 2017/03/02 01:25:00]

Great, thanks for making it clearer.

+     * @type {?CrOnc.NetworkStateProperties|undefined}
      */
-    deviceState: {
-      type: Object,
-      observer: 'deviceStateChanged_',
-    },
+    defaultNetwork: Object,
 
     /**
-     * Network state for the active network.
-     * @type {!CrOnc.NetworkStateProperties|undefined}
+     * Device state for the network type.
+     * @type {?DeviceStateProperties|undefined}
      */
-    activeNetworkState: Object,
+    deviceState: Object,
 
-    /**
-     * List of all network state data for the network type.
-     * @type {!Array<!CrOnc.NetworkStateProperties>}
-     */
-    networkStateList: {
-      type: Array,
-      value: function() {
-        return [];
-      },
-    },
+    /** @type {!chrome.networkingPrivate.GlobalPolicy|undefined} */
+    globalPolicy: Object,
 
     /**
-     * Type of networks in networkStateList. Used to initialte scanning.
-     * @type {CrOnc.Type}
+     * List of third party VPN providers.
+     * @type
+     *   {!Array<!chrome.networkingPrivate.ThirdPartyVPNProperties>|undefined}

[michaelpg, 2017/03/01 23:27:59]

nit: 4-space indent?

[stevenjb, 2017/03/02 00:25:17]

Wasn't sure here. Done.

[michaelpg, 2017/03/02 01:25:00]

There are almost no examples in the codebase. I found these two which each do
different things:

https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/ui/table.js?typ...

https://cs.chromium.org/chromium/src/third_party/google_input_tools/third_par...

So... do whatever seems reasonable :-)

      */
-    networkType: String,
+    thirdPartyVpnProviders: Array,
 
     /**
      * Interface for networkingPrivate calls, passed from internet_page.
@@ -54,66 +48,99 @@ Polymer({
      */
     networkingPrivate: Object,
 
-    /**
-     * The expanded state of the list of networks.
-     * @private
-     */
-    expanded_: {
+    showSpinner: {
       type: Boolean,
+      notify: true,
       value: false,
-      observer: 'expandedChanged_',
     },
 
     /**
-     * Whether the list has been expanded. This is used to ensure the
-     * iron-collapse section animates correctly.
-     * @private
+     * List of all network state data for the network type.
+     * @private {!Array<!CrOnc.NetworkStateProperties>}
      */
-    wasExpanded_: {
-      type: Boolean,
-      value: false,
+    networkStateList_: {
+      type: Array,
+      value: function() {
+        return [];
+      },
+    },
+
+    /**
+     * Dictionary of lists of network states for third party VPNs.
+     * @private {!Object<!Array<!CrOnc.NetworkStateProperties>>}
+     */
+    thirdPartyVpns_: {
+      type: Object,
+      value: function() {
+        return {};
+      },
     },
   },
 
-  observers: ['updateScanning_(networkingPrivate, networkType, expanded_)'],
+  observers: ['updateScanning_(networkingPrivate, deviceState)'],
 
   /** @private {number|null} */
   scanIntervalId_: null,
 
-  /** override */
-  detached: function() {
-    if (this.scanIntervalId_ !== null) {
-      window.clearInterval(this.scanIntervalId_);
-      this.scanIntervalId_ = null;
+  /**
+   * Listener function for chrome.networkingPrivate.onNetworkListChanged event.
+   * @type {?function(!Array<string>)}
+   * @private
+   */
+  networkListChangedListener_: null,
+
+  /**
+   * settings.RouteObserverBehavior
+   * @param {!settings.Route} route
+   * @protected
+   */
+  currentRouteChanged: function(route) {

[michaelpg, 2017/03/01 23:27:59]

nit: per the style guide, should be ordered after lifecycle methods

[stevenjb, 2017/03/02 00:25:17]

Done.

+    if (route != settings.Route.INTERNET_NETWORKS) {
+      this.stopScanning_();
+      return;
     }
+    // Clear any stale data.
+    this.networkStateList_ = [];
+    this.thirdPartyVpns_ = {};
+    // Request the list of networks and start scanning of necessary.

[michaelpg, 2017/03/01 23:27:59]

of necessary => if necessary

[stevenjb, 2017/03/02 00:25:17]

Done.

+    this.getNetworkStateList_();
+    this.updateScanning_();
   },
 
-  /** @private */
-  expandedChanged_: function() {
-    var type = this.deviceState ? this.deviceState.Type : '';
-    this.fire('expanded', {expanded: this.expanded_, type: type});
+  /** override */
+  attached: function() {
+    this.scanIntervalId_ = null;
+
+    this.networkListChangedListener_ = this.networkListChangedListener_ ||
+        this.onNetworkListChangedEvent_.bind(this);
+    this.networkingPrivate.onNetworkListChanged.addListener(
+        this.networkListChangedListener_);
   },
 
-  /** @private */
-  deviceStateChanged_: function() {
-    if (this.expanded_ && !this.deviceIsEnabled_(this.deviceState))
-      this.expanded_ = false;
+  /** override */
+  detached: function() {
+    this.stopScanning_();
+    this.networkingPrivate.onNetworkListChanged.removeListener(
+        assert(this.networkListChangedListener_));
   },
 
   /** @private */
   updateScanning_: function() {
-    if (this.scanIntervalId_ != null) {
-      if (!this.expanded_) {
-        window.clearInterval(this.scanIntervalId_);
-        this.scanIntervalId_ = null;
-      }
+    if (!this.deviceState)
       return;
-    }
-    if (!this.expanded_ ||
-        (this.networkType != CrOnc.Type.ALL &&
-         this.networkType != CrOnc.Type.WI_FI)) {
+    if (this.deviceState.Type != CrOnc.Type.WI_FI) {
+      // deviceState probably changed, re-request networks.
+      this.getNetworkStateList_();
       return;
     }
+    this.showSpinner = !!this.deviceState.Scanning;
+    this.startScanning_();
+  },
+
+  /** @private */
+  startScanning_: function() {
+    if (this.scanIntervalId_ != null)
+      return;
     /** @const */ var INTERVAL_MS = 10 * 1000;
     this.networkingPrivate.requestNetworkScan();
     this.scanIntervalId_ = window.setInterval(function() {
@@ -121,58 +148,53 @@ Polymer({
     }.bind(this), INTERVAL_MS);
   },
 
-  /**
-   * @return {boolean} Whether or not the scanning spinner should be visible.
-   * @private
-   */
-  scanningIsVisible_: function() {
-    return this.deviceState.Type == CrOnc.Type.WI_FI;
+  /** @private */
+  stopScanning_: function() {
+    if (this.scanIntervalId_ == null)
+      return;
+    window.clearInterval(this.scanIntervalId_);
+    this.scanIntervalId_ = null;
   },
 
   /**
-   * @return {boolean} Whether or not the scanning spinner should be active.
+   * networkingPrivate.onNetworkListChanged event callback.
    * @private
    */
-  scanningIsActive_: function() {
-    return !!this.expanded_ && !!this.deviceState.Scanning;
+  onNetworkListChangedEvent_: function() {
+    this.getNetworkStateList_();
   },
 
-  /**
-   * Show the <network-siminfo> element if this is a disabled and locked
-   * cellular device.
-   * @return {boolean}
-   * @private
-   */
-  showSimInfo_: function() {
-    var device = this.deviceState;
-    if (device.Type != CrOnc.Type.CELLULAR ||
-        this.deviceIsEnabled_(this.deviceState)) {
-      return false;
-    }
-    return device.SimPresent === false ||
-        device.SimLockType == CrOnc.LockType.PIN ||
-        device.SimLockType == CrOnc.LockType.PUK;
-  },
-
-  /**
-   * Returns a NetworkProperties object for <network-siminfo> built from
-   * the device properties (since there will be no active network).
-   * @param {!DeviceStateProperties} deviceState
-   * @return {!CrOnc.NetworkProperties}
-   * @private
-   */
-  getCellularState_: function(deviceState) {
-    return {
-      GUID: '',
-      Type: CrOnc.Type.CELLULAR,
-      Cellular: {
-        SIMLockStatus: {
-          LockType: deviceState.SimLockType || '',
-          LockEnabled: deviceState.SimLockType != CrOnc.LockType.NONE,
-        },
-        SIMPresent: deviceState.SimPresent,
-      },
+  /** @private */
+  getNetworkStateList_: function() {
+    if (!this.deviceState)
+      return;
+    var filter = {
+      networkType: this.deviceState.Type,
+      visible: true,
+      configured: false
     };
+    this.networkingPrivate.getNetworks(filter, function(networkStates) {
+      if (this.deviceState.Type != CrOnc.Type.VPN) {
+        this.networkStateList_ = networkStates;
+        return;
+      }
+      // For VPNs, separate out third party VPNs.
+      var networkStateList = [];
+      var thirdPartyVpns = {};
+      for (var i = 0; i < networkStates.length; ++i) {
+        var state = networkStates[i];
+        var providerType = state.VPN && state.VPN.ThirdPartyVPN &&
+            state.VPN.ThirdPartyVPN.ProviderName;
+        if (providerType) {
+          thirdPartyVpns[providerType] = thirdPartyVpns[providerType] || [];
+          thirdPartyVpns[providerType].push(state);
+        } else {
+          networkStateList.push(state);
+        }
+      }
+      this.networkStateList_ = networkStateList;
+      this.thirdPartyVpns_ = thirdPartyVpns;
+    }.bind(this));
   },
 
   /**
@@ -186,51 +208,34 @@ Polymer({
   },
 
   /**
-   * @return {boolean} Whether the dom-if for the network list should be true.
-   *   The logic here is designed to allow the enclosed content to be stamped
-   *   before it is expanded.
-   * @private
-   */
-  networksDomIfIsTrue_: function() {
-    if (this.expanded_ == this.wasExpanded_)
-      return this.expanded_;
-    if (this.expanded_) {
-      Polymer.RenderStatus.afterNextRender(this, function() {
-        this.wasExpanded_ = true;
-      }.bind(this));
-      return true;
-    }
-    return this.wasExpanded_;
-  },
-
-  /**
-   * @param {boolean} expanded
-   * @param {boolean} wasExpanded
-   * @return {boolean} Whether the iron-collapse for the network list should
-   *   be opened.
+   * @param {!DeviceStateProperties|undefined} deviceState
+   * @param {string} onstr
+   * @param {string} offstr
+   * @return {string}
    * @private
    */
-  networksIronCollapseIsOpened_: function(expanded, wasExpanded) {
-    return expanded && wasExpanded;
+  getOffOnString_: function(deviceState, onstr, offstr) {
+    return this.deviceIsEnabled_(deviceState) ? onstr : offstr;
   },
 
   /**
-   * @param {!DeviceStateProperties} deviceState
+   * @param {?DeviceStateProperties} deviceState
    * @return {boolean}
    * @private
    */
   enableToggleIsVisible_: function(deviceState) {
-    return deviceState.Type != CrOnc.Type.ETHERNET &&
+    return !!deviceState && deviceState.Type != CrOnc.Type.ETHERNET &&
         deviceState.Type != CrOnc.Type.VPN;
   },
 
   /**
-   * @param {!DeviceStateProperties} deviceState
+   * @param {?DeviceStateProperties} deviceState
    * @return {boolean}
    * @private
    */
   enableToggleIsEnabled_: function(deviceState) {
-    return deviceState.State !=
+    return !!deviceState &&
+        deviceState.State !=
         chrome.networkingPrivate.DeviceStateType.PROHIBITED;
   },
 
@@ -255,66 +260,58 @@ Polymer({
   },
 
   /**
-   * @return {boolean} Whether or not to show the UI to expand the list.
+   * @param {!chrome.networkingPrivate.ThirdPartyVPNProperties} vpnState
+   * @return {string}
    * @private
    */
-  expandIsVisible_: function() {
-    if (!this.deviceIsEnabled_(this.deviceState))
-      return false;
-    var type = this.deviceState.Type;
-    var minLength =
-        (type == CrOnc.Type.WI_FI || type == CrOnc.Type.VPN) ? 1 : 2;
-    return this.networkStateList.length >= minLength;
+  getAddThirdPartyVpnA11yString: function(vpnState) {

[michaelpg, 2017/03/01 23:27:59]

add trailing _ for private method

[stevenjb, 2017/03/02 00:25:17]

Oops, good catch. No headphones here, I haven't tested chromevox, I plan to do
an a networking audit soon.

+    return this.i18n('internetAddThirdPartyVPN', vpnState.ProviderName);
   },
 
   /**
-   * @return {boolean} Whether or not to show the UI to show details.
+   * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
+   * @return {boolean}
    * @private
    */
-  showDetailsIsVisible_: function() {
-    if (this.expandIsVisible_())
-      return false;
-    return this.deviceIsEnabled_(this.deviceState);
+  allowAddConnection_: function(globalPolicy) {
+    return globalPolicy && !globalPolicy.AllowOnlyPolicyNetworksToConnect;
   },
 
   /**
-   * @param {!CrOnc.NetworkStateProperties} activeNetworkState
-   * @return {boolean} True if the known networks button should be shown.
+   * @param {!DeviceStateProperties} deviceState
+   * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
+   * @return {boolean}
    * @private
    */
-  knownNetworksIsVisible_: function(activeNetworkState) {
-    return !!activeNetworkState && activeNetworkState.Type == CrOnc.Type.WI_FI;
+  showAddButton_: function(deviceState, globalPolicy) {
+    if (!deviceState || deviceState.Type != CrOnc.Type.WI_FI)
+      return false;
+    return this.allowAddConnection_(globalPolicy);
+  },
+
+  /** @private */
+  onAddButtonTap_: function() {
+    chrome.send('addNetwork', [this.deviceState.Type]);
   },
 
   /**
-   * Event triggered when the details div is tapped or Enter is pressed.
-   * @param {!Event} event The enable button event.
+   * @param {!{model:
+   *            !{item: !chrome.networkingPrivate.ThirdPartyVPNProperties},

[michaelpg, 2017/03/01 23:27:59]

nit: 4-space indent relative to the nearest opening brace, ie placing "!"
directly below the "l" in model

[stevenjb, 2017/03/02 00:25:17]

Hm. OK, done. (FWIW that pattern makes this sort of wrapping almost useless - it
only saves 2 characters - but in this case and the one above it happens to
work...)

[michaelpg, 2017/03/02 01:25:00]

indenting only 4 characters (relative to @param) would probably be fine, too

or break after the {, like in JS?

+   *        }} event
    * @private
    */
-  onDetailsTap_: function(event) {
-    if ((event.target && event.target.id == 'expandListButton') ||
-        (this.deviceState && !this.deviceIsEnabled_(this.deviceState))) {
-      // Already handled or disabled, do nothing.
-      return;
-    }
-    if (this.expandIsVisible_()) {
-      // Expandable, toggle expand.
-      this.expanded_ = !this.expanded_;
-      return;
-    }
-    // Not expandable, fire 'selected' with |activeNetworkState|.
-    this.fire('selected', this.activeNetworkState);
+  onAddThirdPartyVpnTap_: function(event) {
+    var provider = event.model.item;
+    chrome.send('addNetwork', [CrOnc.Type.VPN, provider.ExtensionID]);
   },
 
   /**
-   * @param {!Event} event The enable button event.
+   * @param {!DeviceStateProperties} deviceState
+   * @return {boolean}
    * @private
    */
-  onShowDetailsTap_: function(event) {
-    if (!this.activeNetworkState.GUID)
-      return;
-    this.fire('show-detail', this.activeNetworkState);
-    event.stopPropagation();
+  knownNetworksIsVisible_: function(deviceState) {
+    return deviceState && deviceState.Type == CrOnc.Type.WI_FI;
   },
 
   /**
@@ -331,6 +328,7 @@ Polymer({
    * @private
    */
   onDeviceEnabledTap_: function(event) {
+    assert(this.deviceState);
     var deviceIsEnabled = this.deviceIsEnabled_(this.deviceState);
     var type = this.deviceState ? this.deviceState.Type : '';

[michaelpg, 2017/03/01 23:27:59]

guaranteed to be true after assert (assuming deviceIsEnabled_() doesn't have
side effects...)

[stevenjb, 2017/03/02 00:25:17]

Done.

     this.fire(
@@ -338,4 +336,91 @@ Polymer({
     // Make sure this does not propagate to onDetailsTap_.
     event.stopPropagation();
   },
+
+  /**
+   * @param {!Object<!Array<!CrOnc.NetworkStateProperties>>} thirdPartyVpns
+   * @param {!chrome.networkingPrivate.ThirdPartyVPNProperties} vpnState
+   * @return {!Array<!CrOnc.NetworkStateProperties>}
+   * @private
+   */
+  getThirdPartyVpnNetworks_: function(thirdPartyVpns, vpnState) {
+    return thirdPartyVpns[vpnState.ProviderName] || [];
+  },
+
+  /**
+   * @param {!Object<!Array<!CrOnc.NetworkStateProperties>>} thirdPartyVpns
+   * @param {!chrome.networkingPrivate.ThirdPartyVPNProperties} vpnState
+   * @return {boolean}
+   * @private
+   */
+  haveThirdPartyVpnNetwork_: function(thirdPartyVpns, vpnState) {
+    var list = this.getThirdPartyVpnNetworks_(thirdPartyVpns, vpnState);
+    return !!list.length;
+  },
+
+  /**
+   * Event triggered when a network list item is selected.
+   * @param {!{target: HTMLElement, detail: !CrOnc.NetworkStateProperties}} e
+   * @private
+   */
+  onNetworkSelected_: function(e) {
+    assert(this.globalPolicy);
+    assert(this.defaultNetwork !== undefined);
+    var state = e.detail;
+    e.target.blur();
+    if (this.canConnect_(state, this.globalPolicy, this.defaultNetwork)) {
+      this.connectToNetwork_(state);
+      return;
+    }
+    this.fire('show-detail', state);
+  },
+
+  /**
+   * Determines whether or not a network state can be connected to.
+   * @param {!CrOnc.NetworkStateProperties} state The network state.
+   * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
+   * @param {?CrOnc.NetworkStateProperties} defaultNetwork
+   * @private
+   */
+  canConnect_: function(state, globalPolicy, defaultNetwork) {
+    if (state.Type == CrOnc.Type.WI_FI && globalPolicy &&
+        globalPolicy.AllowOnlyPolicyNetworksToConnect &&
+        !this.isPolicySource(state.Source)) {
+      return false;
+    }
+    if (state.Type == CrOnc.Type.VPN &&
+        (!defaultNetwork ||
+         defaultNetwork.ConnectionState != CrOnc.ConnectionState.CONNECTED)) {
+      return false;
+    }
+    return state.ConnectionState == CrOnc.ConnectionState.NOT_CONNECTED;

[michaelpg, 2017/03/01 23:27:59]

opt nit: I would make this just another condition, at the top of the function:
    if (state.CS != NOT_CONNECTED)
      return false;
and then return true at the end, so all the conditions have the same format. As
a bonus, I think it does less work in the common case.

[stevenjb, 2017/03/02 00:25:17]

Done.

+  },
+
+  /**
+   * 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);
+        }
+      }
+    });
+  },
+
+  /**
+   * @param {*} lhs
+   * @param {*} rhs
+   * @return {boolean}
+   */
+  isEqual_: function(lhs, rhs) {
+    return lhs === rhs;
+  },
 });