Fix VPN connected logic in internet settings

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 3086221ba1ba8f7239d1cb38afd932fe3edc68d3..ea10c04fa7b208f01366e170616cf92e491a6e9f 100644
--- a/chrome/browser/resources/settings/internet_page/network_summary.js
+++ b/chrome/browser/resources/settings/internet_page/network_summary.js
@@ -59,6 +59,16 @@ Polymer({
 
   properties: {
     /**
+     * Highest priority connected network or null.
+     * @type {?CrOnc.NetworkStateProperties}
+     */
+    defaultNetwork: {
+      type: Object,
+      value: null,
+      notify: true
+    },
+
+    /**
      * The device state for each network device type.
      * @type {DeviceStateObject}
      */
@@ -83,7 +93,7 @@ Polymer({
     networkStateLists: {
       type: Object,
       value: function() { return {}; },
-    }
+    },
   },
 
   /**
@@ -154,7 +164,8 @@ Polymer({
    * @private
    */
   onWiFiExpanded_: function(event) {
-    this.getNetworkStates_();  // Get the latest network states (only).
+    // Get the latest network states (only).
+    this.getNetworkStates_(null);
     if (event.detail.expanded)
       chrome.networkingPrivate.requestNetworkScan();
   },
@@ -166,7 +177,7 @@ Polymer({
    */
   onSelected_: function(event) {
     var state = event.detail;
-    if (state.ConnectionState == CrOnc.ConnectionState.NOT_CONNECTED) {
+    if (this.canConnect_(state)) {
       this.connectToNetwork_(state);
       return;
     }
@@ -232,6 +243,19 @@ Polymer({
   },
 
   /**
+   * 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;
+  },
+
+  /**
    * Handles UI requests to connect to a network.
    * TODO(stevenjb): Handle Cellular activation, etc.
    * @param {!CrOnc.NetworkStateProperties} state The network state.
@@ -240,9 +264,9 @@ Polymer({
   connectToNetwork_: function(state) {
     chrome.networkingPrivate.startConnect(state.GUID, function() {
       if (chrome.runtime.lastError &&
-          chrome.runtime.lastError != 'connecting') {
+          chrome.runtime.lastError.message != 'connecting') {
         console.error('Unexpected networkingPrivate.startConnect error:',
-                      chrome.runtime.lastError);
+                      chrome.runtime.lastError, 'For:', state.GUID);
       }
     });
   },
@@ -256,47 +280,50 @@ Polymer({
   getNetworkLists_: function() {
     // First get the device states.
     chrome.networkingPrivate.getDeviceStates(
-      function(states) {
-        this.getDeviceStatesCallback_(states);
+      function(deviceStates) {
         // Second get the network states.
-        this.getNetworkStates_();
+        this.getNetworkStates_(deviceStates);
       }.bind(this));
   },
 
   /**
    * Requests the list of network states from Chrome. Updates networkStates and
    * networkStateLists once the results are returned from Chrome.
+   * @param {?Array<!DeviceStateProperties>} deviceStates The state properties
+   *     for all available devices.
    * @private
    */
-  getNetworkStates_: function() {
+  getNetworkStates_: function(deviceStates) {

[michaelpg, 2015/10/26 20:41:42]

Maybe make this opt_deviceStates (@param {Array<!DSP>|undefined}), so it can be
called as getNetworkStates_()?

[stevenjb, 2015/10/27 16:49:50]

Done.

     var filter = {
       networkType: chrome.networkingPrivate.NetworkType.ALL,
       visible: true,
       configured: false
     };
-    chrome.networkingPrivate.getNetworks(
-        filter, this.getNetworksCallback_.bind(this));
-  },
-
-  /**
-   * networkingPrivate.getDeviceStates callback.
-   * @param {!Array<!DeviceStateProperties>} states The state properties for all
-   *     available devices.
-   * @private
-   */
-  getDeviceStatesCallback_: function(states) {
-    var newStates = /** @type {!DeviceStateObject} */({});
-    states.forEach(function(state) { newStates[state.Type] = state; });
-    this.deviceStates = newStates;
+    var self = this;

[michaelpg, 2015/10/26 20:41:41]

we mostly/only use .bind(this) instead of var foo = this...

[stevenjb, 2015/10/27 16:49:50]

Done.

+    chrome.networkingPrivate.getNetworks(filter, function(networkStates) {
+      self.getNetworksCallback_(deviceStates, networkStates);
+    });
   },
 
   /**
    * networkingPrivate.getNetworksState callback.
-   * @param {!Array<!CrOnc.NetworkStateProperties>} states The state properties
-   *     for all visible networks.
+   * @param {?Array<!DeviceStateProperties>} deviceStates The state properties
+   *     for all available devices.
+   * @param {!Array<!CrOnc.NetworkStateProperties>} networkStates The state
+   *     properties for all visible networks.
    * @private
    */
-  getNetworksCallback_: function(states) {
+  getNetworksCallback_: function(deviceStates, networkStates) {
+    var newDeviceStates;
+    if (deviceStates) {
+      newDeviceStates = /** @type {!DeviceStateObject} */({});
+      deviceStates.forEach(function(state) {
+        newDeviceStates[state.Type] = state;
+      });
+    } else {
+      newDeviceStates = this.deviceStates;
+    }
+
     // Clear any current networks.
     this.networkIds_ = {};
 
@@ -312,21 +339,28 @@ Polymer({
       VPN: []
     };
 
-    states.forEach(function(state) {
+    var firstConnectedNetwork = null;
+    networkStates.forEach(function(state) {
       var type = state.Type;
       if (!foundTypes[type]) {
         foundTypes[type] = true;
         this.updateNetworkState_(type, state);
+        if (!firstConnectedNetwork && state.Type != CrOnc.Type.VPN &&
+            state.ConnectionState == CrOnc.ConnectionState.CONNECTED) {
+          firstConnectedNetwork = state;
+        }
       }
       networkStateLists[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 (this.deviceStates[type])
+        if (newDeviceStates[type])
           defaultState = {GUID: '', Type: type};
         this.updateNetworkState_(type, defaultState);
       }
@@ -336,9 +370,13 @@ Polymer({
 
     // Create a VPN entry in deviceStates if there are any VPN networks.
     if (networkStateLists.VPN && networkStateLists.VPN.length > 0) {
-      var vpn = {Type: CrOnc.Type.VPN, State: 'Enabled'};
-      this.set('deviceStates.VPN', vpn);
+      newDeviceStates.VPN = /** @type {DeviceStateProperties} */ ({
+        Type: CrOnc.Type.VPN,
+        State: chrome.networkingPrivate.DeviceStateType.ENABLED
+      });
     }
+
+    this.deviceStates = newDeviceStates;
   },
 
   /**