 Chromium Code Reviews
 Chromium Code Reviews Issue 1017443002:
  Allow users to add third-party VPNs from the settings page  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@f_3_460428_update_details_dialog
    
  
    Issue 1017443002:
  Allow users to add third-party VPNs from the settings page  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@f_3_460428_update_details_dialog| Index: chrome/browser/resources/options/chromeos/network_list.js | 
| diff --git a/chrome/browser/resources/options/chromeos/network_list.js b/chrome/browser/resources/options/chromeos/network_list.js | 
| index 9e6d7298311511db07585f3ea02d5a23455a50cd..6ccb9257b610f55065619ccef962bf3a6f1fe68c 100644 | 
| --- a/chrome/browser/resources/options/chromeos/network_list.js | 
| +++ b/chrome/browser/resources/options/chromeos/network_list.js | 
| @@ -377,11 +377,29 @@ cr.define('options.network', function() { | 
| return null; | 
| }, | 
| + /** | 
| + * Determines if a menu can be updated on the fly. Menus that cannot be | 
| + * updated are fully regenerated using createMenu. The advantage of | 
| + * updating a menu is that it can preserve ordering of networks avoiding | 
| + * entries from jumping around after an update. | 
| + * @return {boolean} Whether the menu can be updated on the fly. | 
| + */ | 
| canUpdateMenu: function() { | 
| return false; | 
| }, | 
| /** | 
| + * Removes the current menu contents, causing it to be regenerated when the | 
| + * menu is shown the next time. If the menu is showing right now, its | 
| + * contents is regenerated immediately and the menu remains visible. | 
| 
michaelpg
2015/03/17 19:28:16
nit: s/is/are
 
bartfab (slow)
2015/03/18 21:27:15
Done.
 | 
| + */ | 
| + refreshMenu: function() { | 
| + this.menu_ = null; | 
| + if (activeMenu_ == this.getMenuName()) | 
| + this.showMenu(); | 
| + }, | 
| + | 
| + /** | 
| * Displays a popup menu. | 
| */ | 
| showMenu: function() { | 
| @@ -395,7 +413,7 @@ cr.define('options.network', function() { | 
| rebuild = true; | 
| var existing = $(this.getMenuName()); | 
| if (existing) { | 
| - if (this.updateMenu()) | 
| + if (this.canUpdateMenu() && this.updateMenu()) | 
| return; | 
| closeMenu_(); | 
| } | 
| @@ -499,14 +517,14 @@ cr.define('options.network', function() { | 
| if (this.data_.key == 'WiFi') { | 
| addendum.push({ | 
| label: loadTimeData.getString('joinOtherNetwork'), | 
| - command: createAddConnectionCallback_('WiFi'), | 
| + command: createAddNonVPNConnectionCallback_('WiFi'), | 
| data: {} | 
| }); | 
| } else if (this.data_.key == 'Cellular') { | 
| if (cellularEnabled_ && cellularSupportsScan_) { | 
| addendum.push({ | 
| label: loadTimeData.getString('otherCellularNetworks'), | 
| - command: createAddConnectionCallback_('Cellular'), | 
| + command: createAddNonVPNConnectionCallback_('Cellular'), | 
| addClass: ['other-cellulars'], | 
| data: {} | 
| }); | 
| @@ -533,11 +551,7 @@ cr.define('options.network', function() { | 
| } | 
| addendum.push(entry); | 
| } else if (this.data_.key == 'VPN') { | 
| - addendum.push({ | 
| - label: loadTimeData.getString('joinOtherNetwork'), | 
| - command: createAddConnectionCallback_('VPN'), | 
| - data: {} | 
| - }); | 
| + addendum = addendum.concat(createAddVPNConnectionEntries_()); | 
| } | 
| var list = this.data.rememberedNetworks; | 
| @@ -636,12 +650,7 @@ cr.define('options.network', function() { | 
| return menu; | 
| }, | 
| - /** | 
| - * Determines if a menu can be updated on the fly. Menus that cannot be | 
| - * updated are fully regenerated using createMenu. The advantage of | 
| - * updating a menu is that it can preserve ordering of networks avoiding | 
| - * entries from jumping around after an update. | 
| - */ | 
| + /** @override */ | 
| canUpdateMenu: function() { | 
| return this.data_.key == 'WiFi' && activeMenu_ == this.getMenuName(); | 
| }, | 
| @@ -652,12 +661,11 @@ cr.define('options.network', function() { | 
| * preferred ordering as determined by the network library. If the | 
| * ordering becomes potentially out of sync, then the updated menu is | 
| * marked for disposal on close. Reopening the menu will force a | 
| - * regeneration, which will in turn fix the ordering. | 
| + * regeneration, which will in turn fix the ordering. This method must only | 
| + * be called if canUpdateMenu() returned |true|. | 
| * @return {boolean} True if successfully updated. | 
| */ | 
| updateMenu: function() { | 
| - if (!this.canUpdateMenu()) | 
| 
michaelpg
2015/03/17 19:28:16
why does this need to be moved out to line 416?
 
bartfab (slow)
2015/03/18 21:27:15
showMenu() needs to be compatible with the base cl
 | 
| - return false; | 
| var oldMenu = $(this.getMenuName()); | 
| var group = oldMenu.getElementsByClassName('network-menu-group')[0]; | 
| if (!group) | 
| @@ -824,6 +832,7 @@ cr.define('options.network', function() { | 
| * A list of controls for manipulating network connectivity. | 
| * @constructor | 
| * @extends {cr.ui.List} | 
| + * @implements {options.VPNProviders.Observer} | 
| */ | 
| var NetworkList = cr.ui.define('list'); | 
| @@ -844,18 +853,7 @@ cr.define('options.network', function() { | 
| // Wi-Fi control is always visible. | 
| this.update({key: 'WiFi', networkList: []}); | 
| - var entryAddWifi = { | 
| - label: loadTimeData.getString('addConnectionWifi'), | 
| - command: createAddConnectionCallback_('WiFi') | 
| - }; | 
| - var entryAddVPN = { | 
| - label: loadTimeData.getString('addConnectionVPN'), | 
| - command: createAddConnectionCallback_('VPN') | 
| - }; | 
| - this.update({key: 'addConnection', | 
| - iconType: 'add-connection', | 
| - menu: [entryAddWifi, entryAddVPN] | 
| - }); | 
| + this.updateAddConnectionMenuEntries_(); | 
| var prefs = options.Preferences.getInstance(); | 
| prefs.addEventListener('cros.signed.data_roaming_enabled', | 
| @@ -863,6 +861,43 @@ cr.define('options.network', function() { | 
| enableDataRoaming_ = event.value.value; | 
| }); | 
| this.endBatchUpdates(); | 
| + | 
| + options.VPNProviders.addObserver(this); | 
| + }, | 
| + | 
| + /** | 
| + * Called when the list of VPN providers changes. Refreshes the contents of | 
| + * menus that list VPN providers. | 
| + * @override | 
| + */ | 
| + onVPNProvidersChanged() { | 
| + // Refresh the contents of the VPN menu. | 
| + var index = this.indexOf('VPN'); | 
| + if (index != undefined) | 
| + this.getListItemByIndex(index).refreshMenu(); | 
| + | 
| + // Refresh the contents of the "add connection" menu. | 
| + this.updateAddConnectionMenuEntries_(); | 
| + index = this.indexOf('addConnection'); | 
| + if (index != undefined) | 
| + this.getListItemByIndex(index).refreshMenu(); | 
| + }, | 
| + | 
| + /** | 
| + * Updates the entries in the "add connection" menu, based on the VPN | 
| + * providers currently enabled in the primary user's profile. | 
| + * @private | 
| + */ | 
| + updateAddConnectionMenuEntries_() { | 
| + var entries = [{ | 
| + label: loadTimeData.getString('addConnectionWifi'), | 
| + command: createAddNonVPNConnectionCallback_('WiFi') | 
| + }]; | 
| + entries = entries.concat(createAddVPNConnectionEntries_()); | 
| + this.update({key: 'addConnection', | 
| + iconType: 'add-connection', | 
| + menu: entries | 
| + }); | 
| }, | 
| /** | 
| @@ -1220,22 +1255,61 @@ cr.define('options.network', function() { | 
| } | 
| /** | 
| - * Create a callback function that adds a new connection of the given type. | 
| + * Creates a callback function that adds a new connection of the given type. | 
| + * This method may be used for all network types except VPN. | 
| * @param {string} type An ONC network type | 
| * @return {function()} The created callback. | 
| * @private | 
| */ | 
| - function createAddConnectionCallback_(type) { | 
| + function createAddNonVPNConnectionCallback_(type) { | 
| return function() { | 
| if (type == 'WiFi') | 
| sendChromeMetricsAction('Options_NetworkJoinOtherWifi'); | 
| - else if (type == 'VPN') | 
| - sendChromeMetricsAction('Options_NetworkJoinOtherVPN'); | 
| - chrome.send('addConnection', [type]); | 
| + chrome.send('addNonVPNConnection', [type]); | 
| }; | 
| } | 
| /** | 
| + * Creates a callback function that asks the VPN provider identified by | 
| + * |providerID| to show its "add network" dialog. | 
| + * @param {string} providerID The VPN provider's ID, as retrieved from | 
| + * options.VPNProviders.getProviders(). | 
| + * @return {function()} The created callback. | 
| + * @private | 
| + */ | 
| + function createAddVPNConnectionCallback_(providerID) { | 
| + return function() { | 
| + sendChromeMetricsAction( | 
| + options.VPNProviders.isThirdPartyProvider(providerID) ? | 
| + 'Options_NetworkAddVPNThirdParty' : | 
| + 'Options_NetworkAddVPNBuiltIn'); | 
| + chrome.send('addVPNConnection', [providerID]); | 
| + }; | 
| + } | 
| + | 
| + /** | 
| + * Generates an "add network" entry for each VPN provider currently enabled in | 
| + * the primary user's profile. | 
| + * @return {!Array<Object<{label: string, | 
| 
michaelpg
2015/03/17 19:28:16
The Object<> notation designates a set. To just de
 
bartfab (slow)
2015/03/18 21:27:15
Done.
 | 
| + * command: function(), | 
| + * data: !Object}>>} The list of entries. | 
| + * @private | 
| + */ | 
| + function createAddVPNConnectionEntries_() { | 
| + var entries = []; | 
| + var providers = options.VPNProviders.getProviders(); | 
| + for (var providerID in providers) { | 
| + entries.push({ | 
| + label: loadTimeData.getStringF('addConnectionVPNTemplate', | 
| + providers[providerID]), | 
| + command: createAddVPNConnectionCallback_(providerID), | 
| + data: {} | 
| + }); | 
| + } | 
| + return entries; | 
| + } | 
| + | 
| + /** | 
| * Whether the Network list is disabled. Only used for display purpose. | 
| */ | 
| cr.defineProperty(NetworkList, 'disabled', cr.PropertyKind.BOOL_ATTR); |