Fix VPN dropdown menu in Settings

chrome/browser/resources/options/chromeos/network_list.js

 // Copyright (c) 2012 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.
 
 /**
  * @typedef {{
  *   ConnectionState: string,
  *   iconURL: string,
  *   policyManaged: boolean,
  *   servicePath: string
@@ skipping 493 matching lines @@
           var self = this;
           entry.command = function() {
             options.Preferences.setBooleanPref(
                 'cros.signed.data_roaming_enabled',
                 !enableDataRoaming_, true);
             // Force revalidation of the menu the next time it is displayed.
             self.menu_ = null;
           };
         }
         addendum.push(entry);
+      } else if (this.data_.key == 'VPN') {
+        addendum.push({
+          label: loadTimeData.getString('joinOtherNetwork'),
+          command: createAddConnectionCallback_('VPN'),
+          data: {}
+        });
       }
+
       var list = this.data.rememberedNetworks;
       if (list && list.length > 0) {
         var callback = function(list) {
           $('remembered-network-list').clear();
           var dialog = options.PreferredNetworks.getInstance();
           PageManager.showPageByName('preferredNetworksPage', false);
           dialog.update(list);
           sendChromeMetricsAction('Options_NetworkShowPreferred');
         };
         addendum.push({label: loadTimeData.getString('preferredNetworks'),
                        command: callback,
                        data: list});
       }
 
       var networkGroup = this.ownerDocument.createElement('div');
       networkGroup.className = 'network-menu-group';
       list = this.data.networkList;
       var empty = !list || list.length == 0;
       if (list) {
+        var connectedVpnServicePath = '';
         for (var i = 0; i < list.length; i++) {
           var data = list[i];
           this.createNetworkOptionsCallback_(networkGroup, data);
-          if (data.ConnectionState == 'Connected') {
-            if (data.Type == 'VPN') {
-              var disconnectCallback = function() {
-                sendChromeMetricsAction('Options_NetworkDisconnectVPN');
-                // TODO(stevenjb): chrome.networkingPrivate.startDisconnect
-                chrome.send('startDisconnect', [data.servicePath]);
-              };
-              // Add separator
-              addendum.push({});
-              addendum.push({label: loadTimeData.getString('disconnectNetwork'),
-                             command: disconnectCallback,
-                             data: data});
-            }
+          // For VPN only we want to append a 'Disconnect' item to the dropdown
+          // since there should only ever be one connected or connecting VPN.

[tbarzic, 2014/10/22 17:06:57]

nit: Maybe it would be better to say that it should be OK to show 'Disconnect'
rather than we want to show it because there is at most one connected VPN.

[stevenjb, 2014/10/22 17:31:33]

Actually, I guess that's true for any network type, we just decided to add it to
VPN for convenience. I'll simplify the comment.

+          if (!connectedVpnServicePath &&
+              (data.Type == 'VPN') &&

[tbarzic, 2014/10/22 17:06:57]

nit: I'd drop () here

[stevenjb, 2014/10/22 17:31:33]

Done.

+              (data.ConnectionState == 'Connected' ||
+               data.ConnectionState == 'Connecting')) {
+            connectedVpnServicePath = data.servicePath;
           }
         }
+        if (connectedVpnServicePath) {
+          var disconnectCallback = function() {
+            sendChromeMetricsAction('Options_NetworkDisconnectVPN');
+            // TODO(stevenjb): chrome.networkingPrivate.startDisconnect
+            chrome.send('startDisconnect', [connectedVpnServicePath]);
+          };
+          // Add separator
+          addendum.push({});
+          addendum.push({label: loadTimeData.getString('disconnectNetwork'),
+                         command: disconnectCallback,
+                         data: data});
+        }
       }
       if (this.data_.key == 'WiFi' || this.data_.key == 'WiMAX' ||
           this.data_.key == 'Cellular') {
         addendum.push({});
         if (this.data_.key == 'WiFi') {
           addendum.push({
             label: loadTimeData.getString('turnOffWifi'),
             command: function() {
               sendChromeMetricsAction('Options_NetworkWifiToggle');
               // TODO(stevenjb): chrome.networkingPrivate.disableNetworkType
@@ skipping 667 matching lines @@
   /**
    * Whether the Network list is disabled. Only used for display purpose.
    */
   cr.defineProperty(NetworkList, 'disabled', cr.PropertyKind.BOOL_ATTR);
 
   // Export
   return {
     NetworkList: NetworkList
   };
 });