Use GetManagedProperties in InternetOptionsHandler

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

Index: chrome/browser/resources/options/chromeos/internet_detail.js
diff --git a/chrome/browser/resources/options/chromeos/internet_detail.js b/chrome/browser/resources/options/chromeos/internet_detail.js
index d00836beb2a44d786bde6103953c9bb4688bf447..ee1c7db9378c661cae9344d8394a4c42cf0902dc 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -15,6 +15,8 @@ cr.define('options.internet', function() {
   /** @const */ var ArrayDataModel = cr.ui.ArrayDataModel;
   /** @const */ var IPAddressField = options.internet.IPAddressField;
 
+  /** @const */ var GoogleNameServers = '8.8.4.4,8.8.8.8';
+
   var GetManagedTypes = {
     ACTIVE: 0,
     TRANSLATED: 1,
@@ -42,7 +44,7 @@ cr.define('options.internet', function() {
       return getRecommendedValue(property);
     if (type == GetManagedTypes.TRANSLATED && 'Translated' in property)
       return property['Translated'];
-    // Otherwise get the Active value (defalt behavior).
+    // Otherwise get the Active value (default behavior).
     if ('Active' in property)
       return property['Active'];
     // If no Active value is defined, return the effective value if present.
@@ -210,6 +212,33 @@ cr.define('options.internet', function() {
     return getManagedValue(data, 'Name');
   }
 
+  /**
+   * Returns the netmask as a string for a given prefix length.
+   * @param {string} prefixLength The ONC routing prefix length.
+   * @return {string} The corresponding netmask.
+   */
+  function PrefixLengthToNetmask(prefixLength) {
+    var netmask = '';
+    // Return the empty string for invalid inputs.
+    if (prefixLength < 0 || prefixLength > 32)
+      return netmask;
+    for (var i = 0; i < 4; ++i) {
+      var remainder = 8;
+      if (prefixLength >= 8) {
+        prefixLength -= 8;
+      } else {
+        remainder = prefixLength;
+        prefixLength = 0;
+      }
+      if (i > 0)
+        netmask += '.';
+      var value =
+          remainder == 0 ? 0 : ((2 << (remainder - 1)) - 1) << (8 - remainder);
+      netmask += value;
+    }
+    return netmask;
+  }
+
   /////////////////////////////////////////////////////////////////////////////
   // DetailsInternetPage class:
 
@@ -1131,56 +1160,94 @@ cr.define('options.internet', function() {
     var restrictedString =
         loadTimeData.getString(restricted ? 'restrictedYes' : 'restrictedNo');
 
-    var ipAutoConfig = data.ipAutoConfig ? 'automatic' : 'user';
-    $('ip-automatic-configuration-checkbox').checked = data.ipAutoConfig;
+    var ipAutoConfig = 'automatic';
+    var staticIpconfig = getManagedValue(data, 'StaticIPConfig');
+    if (staticIpconfig && Object.keys(staticIpconfig).length > 0)

[pneubeck (no reviews), 2014/09/03 15:14:28]

maybe add a comment that explains what the intention of the second check is.

[stevenjb, 2014/09/03 21:28:06]

Done.

+      ipAutoConfig = 'user';
+    $('ip-automatic-configuration-checkbox').checked = ipAutoConfig;

[pneubeck (no reviews), 2014/09/03 15:14:28]

before this was assigned a boolean. Not sure what happens when you assign a
string as you do now.

maybe this should be
 ...checked = ipAutoConfig == 'automatic';
?

[stevenjb, 2014/09/03 21:28:05]

Good catch. Done.

     var inetAddress = {autoConfig: ipAutoConfig};
     var inetNetmask = {autoConfig: ipAutoConfig};
     var inetGateway = {autoConfig: ipAutoConfig};
 
-    if (data.ipconfig.value) {
-      inetAddress.automatic = data.ipconfig.value.address;
-      inetAddress.value = data.ipconfig.value.address;
-      inetNetmask.automatic = data.ipconfig.value.netmask;
-      inetNetmask.value = data.ipconfig.value.netmask;
-      inetGateway.automatic = data.ipconfig.value.gateway;
-      inetGateway.value = data.ipconfig.value.gateway;
-      if (data.ipconfig.value.webProxyAutoDiscoveryUrl) {
-        $('web-proxy-auto-discovery').hidden = false;
-        $('web-proxy-auto-discovery-url').value =
-            data.ipconfig.value.webProxyAutoDiscoveryUrl;
+    var inetNameServers = '';
+    var nameServerType = 'automatic';
+
+    if ('IPConfigs' in data) {
+      var ipconfigList = getManagedValue(data, 'IPConfigs');
+      for (var i = 0; i < ipconfigList.length; ++i) {
+        var ipconfig = ipconfigList[i];
+        var type = ipconfig['Type'];
+        if (type != 'IPv4') {
+          // TODO(stevenjb): Handle IPv6 properties.
+          continue;
+        }
+        inetAddress.automatic = ipconfig['IPAddress'];
+        inetAddress.value = ipconfig['IPAddress'];
+        var netmask = PrefixLengthToNetmask(ipconfig['RoutingPrefix']);
+        inetNetmask.automatic = netmask;
+        inetNetmask.value = netmask;
+        inetGateway.automatic = ipconfig['Gateway'];
+        inetGateway.value = ipconfig['Gateway'];
+        if ('WebProxyAutoDiscoveryUrl' in ipconfig) {
+          $('web-proxy-auto-discovery').hidden = false;
+          $('web-proxy-auto-discovery-url').value =
+              ipconfig['WebProxyAutoDiscoveryUrl'];
+        }
+        if ('NameServers' in ipconfig)
+          inetNameServers = ipconfig['NameServers'];
+        break;  // Use the first IPv4 entry.
       }
     }
 
     // Override the "automatic" values with the real saved DHCP values,
     // if they are set.
-    if (data.savedIP.address) {
-      inetAddress.automatic = data.savedIP.address;
-      inetAddress.value = data.savedIP.address;
-    }
-    if (data.savedIP.netmask) {
-      inetNetmask.automatic = data.savedIP.netmask;
-      inetNetmask.value = data.savedIP.netmask;
-    }
-    if (data.savedIP.gateway) {
-      inetGateway.automatic = data.savedIP.gateway;
-      inetGateway.value = data.savedIP.gateway;
+    var savedNameServers;
+    if ('SavedIPConfig' in data) {
+      var savedIpAddress = getManagedValue(data, 'SavedIPConfig.IPAddress');
+      if (savedIpAddress != undefined) {
+        inetAddress.automatic = savedIpAddress;
+        inetAddress.value = savedIpAddress;
+      }
+      var savedPrefix = getManagedValue(data, 'SavedIPConfig.RoutingPrefix');
+      if (savedPrefix != undefined) {
+        var savedNetmask = PrefixLengthToNetmask(savedPrefix);
+        inetNetmask.automatic = savedNetmask;
+        inetNetmask.value = savedNetmask;
+      }
+      var savedGateway = getManagedValue(data, 'SavedIPConfig.Gateway');
+      if (savedGateway != undefined) {
+        inetGateway.automatic = savedGateway;
+        inetGateway.value = savedGateway;
+      }
+      savedNameServers = getManagedValue(data, 'SavedIPConfig.NameServers');
     }
 
-    if (ipAutoConfig == 'user') {
-      if (data.staticIP.value.address) {
-        inetAddress.value = data.staticIP.value.address;
-        inetAddress.user = data.staticIP.value.address;
+    var staticNameServers;
+    if (ipAutoConfig == 'user' && 'StaticIPConfig' in data) {

[pneubeck (no reviews), 2014/09/03 15:14:28]

optional nit:
second check is redundant. ipAutoConfig is only set to 'user' if staticIpConfig
is available.

[stevenjb, 2014/09/03 21:28:05]

Done.

+      var staticIpAddress = getManagedValue(data, 'StaticIPConfig.IPAddress');
+      if (staticIpAddress != undefined) {
+        inetAddress.user = staticIpAddress;
+        inetAddress.value = staticIpAddress;
       }
-      if (data.staticIP.value.netmask) {
-        inetNetmask.value = data.staticIP.value.netmask;
-        inetNetmask.user = data.staticIP.value.netmask;
+      var staticPrefix = getManagedValue(data, 'StaticIPConfig.RoutingPrefix');
+      if (staticPrefix != undefined) {
+        var staticNetmask = PrefixLengthToNetmask(staticPrefix);
+        inetNetmask.user = staticNetmask;
+        inetNetmask.value = staticNetmask;
       }
-      if (data.staticIP.value.gateway) {
-        inetGateway.value = data.staticIP.value.gateway;
-        inetGateway.user = data.staticIP.value.gateway;
+      var staticGateway = getManagedValue(data, 'StaticIPConfig.Gateway');
+      if (staticGateway != undefined) {
+        inetGateway.user = staticGateway;
+        inetGateway.value = staticGateway;
       }
+      staticNameServers = getManagedValue(data, 'StaticIPConfig.NameServers');
+      if (staticNameServers == inetNameServers)

[pneubeck (no reviews), 2014/09/03 15:14:28]

note that this comparison fails if the order of the nameservers differs, which
might not be the intended behavior.

[stevenjb, 2014/09/03 21:28:06]

Hmm. This should be the same as the existing C++ logic. I don't know enough
about how these are used to know whether or not we should file that as a
separate issue. My (naturally lazy) inclination is to wait until/unless it come
sup - we have no shortage of tasks...

+        nameServerType = 'user';
     }
 
+    if (inetNameServers == GoogleNameServers)
+      nameServerType = 'google';
+
     var configureAddressField = function(field, model) {
       IPAddressField.decorate(field);
       field.model = model;
@@ -1191,33 +1258,28 @@ cr.define('options.internet', function() {
     configureAddressField($('ip-netmask'), inetNetmask);
     configureAddressField($('ip-gateway'), inetGateway);
 
-    var inetNameServers = '';
-    if (data.ipconfig.value && data.ipconfig.value.nameServers) {
-      inetNameServers = data.ipconfig.value.nameServers;
+    if (savedNameServers)
+      $('automatic-dns-display').textContent = savedNameServers;
+    else
       $('automatic-dns-display').textContent = inetNameServers;
-    }
-
-    if (data.savedIP && data.savedIP.nameServers)
-      $('automatic-dns-display').textContent = data.savedIP.nameServers;
 
-    if (data.nameServersGoogle)
-      $('google-dns-display').textContent = data.nameServersGoogle;
+    $('google-dns-display').textContent = GoogleNameServers;
 
     var nameServersUser = [];
-    if (data.staticIP.value.nameServers)
-      nameServersUser = data.staticIP.value.nameServers.split(',');
+    if (staticNameServers != undefined)
+      nameServersUser = staticNameServers.split(',');
 
     var nameServerModels = [];
     for (var i = 0; i < 4; ++i)
       nameServerModels.push({value: nameServersUser[i] || ''});
 
-    $(data.nameServerType + '-dns-radio').checked = true;
+    $(nameServerType + '-dns-radio').checked = true;
     configureAddressField($('ipconfig-dns1'), nameServerModels[0]);
     configureAddressField($('ipconfig-dns2'), nameServerModels[1]);
     configureAddressField($('ipconfig-dns3'), nameServerModels[2]);
     configureAddressField($('ipconfig-dns4'), nameServerModels[3]);
 
-    DetailsInternetPage.updateNameServerDisplay(data.nameServerType);
+    DetailsInternetPage.updateNameServerDisplay(nameServerType);
 
     var macAddress = getManagedValue(data, 'MacAddress');
     if (macAddress) {
@@ -1294,9 +1356,7 @@ cr.define('options.internet', function() {
       $('auto-connect-network-wimax').checked =
           getManagedValue(data, 'AutoConnect');
       $('auto-connect-network-wimax').disabled = !data.remembered;
-      var identity;
-      if (data.Wimax.EAP)
-        identity = getManagedValue(data.Wimax.EAP, 'Identity');
+      var identity = getManagedValue(data, 'Wimax.EAP.Identity');
       setOrHideParent('wimax-eap-identity', identity);
       $('wimax-signal-strength').textContent = strengthString;
     } else if (data.type == 'Cellular') {
@@ -1373,8 +1433,15 @@ cr.define('options.internet', function() {
         var otherOption = apnSelector[0];
         data.selectedApn = -1;
         data.userApnIndex = -1;
-        var activeApn = getManagedValue(data, 'Cellular.APN');
-        var lastGoodApn = getManagedValue(data, 'Cellular.LastGoodAPN');
+        var activeApn = getManagedValue(data, 'Cellular.APN.AccessPointName');
+        var activeUsername = getManagedValue(data, 'Cellular.APN.Username');
+        var activePassword = getManagedValue(data, 'Cellular.APN.Password');
+        var lastGoodApn =
+            getManagedValue(data, 'Cellular.LastGoodAPN.AccessPointName');
+        var lastGoodUsername =
+            getManagedValue(data, 'Cellular.LastGoodAPN.Username');
+        var lastGoodPassword =
+            getManagedValue(data, 'Cellular.LastGoodAPN.Password');
         var apnList = getManagedValue(data, 'Cellular.APNList');
         for (var i = 0; i < apnList.length; i++) {
           var apnDict = apnList[i];
@@ -1387,24 +1454,21 @@ cr.define('options.internet', function() {
           option.value = i;
           // If this matches the active Apn, or LastGoodApn, set it as the
           // selected Apn.
-          if ((activeApn != undefined &&
-               activeApn['AccessPointName'] == accessPointName &&
-               activeApn['Username'] == apnDict['Username'] &&
-               activeApn['Password'] == apnDict['Password']) ||
-              ((activeApn == undefined || !activeApn['AccessPointName']) &&
-                lastGoodApn != undefined &&
-                lastGoodApn['AccessPointName'] == accessPointName &&
-                lastGoodApn['Username'] == apnDict['Username'] &&
-                lastGoodApn['Password'] == apnDict['Password'])) {
+          if ((activeApn == accessPointName &&
+               activeUsername == apnDict['Username'] &&
+               activePassword == apnDict['Password']) ||
+              (!activeApn &&
+               lastGoodApn == accessPointName &&
+               lastGoodUsername == apnDict['Username'] &&
+               lastGoodPassword == apnDict['Password'])) {
             data.selectedApn = i;
           }
           // Insert new option before "other" option.
           apnSelector.add(option, otherOption);
         }
-        if (data.selectedApn == -1 &&
-            activeApn != undefined && activeApn['AccessPointName']) {
+        if (data.selectedApn == -1 && activeApn) {
           var option = document.createElement('option');
-          option.textContent = activeApn['AccessPointName'];
+          option.textContent = activeApn;
           option.value = -1;
           apnSelector.add(option, otherOption);
           data.selectedApn = apnSelector.length - 2;