Use setProperties for IP Config.

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 31e5d5f0d7d6a4e997e96357e063672035dc6129..fce237399828092b280e40f8d38ff6d1f15db0aa 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -11,22 +11,13 @@
 //   networkingPrivate API. See network_config.js.
 // See crbug.com/279351 for more info.
 
-/** @typedef {{address: (string|undefined),
- *             gateway: (string|undefined),
- *             nameServers: (string|undefined),
- *             netmask: (string|undefined),
- *             prefixLength: (number|undefined)}}
- * @see chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
- */
-var IPInfo;
-
 cr.define('options.internet', function() {
   var OncData = cr.onc.OncData;
   var Page = cr.ui.pageManager.Page;
   var PageManager = cr.ui.pageManager.PageManager;
   /** @const */ var IPAddressField = options.internet.IPAddressField;
 
-  /** @const */ var GoogleNameServersString = '8.8.4.4,8.8.8.8';
+  /** @const */ var GoogleNameServers = ['8.8.4.4', '8.8.8.8'];
   /** @const */ var CarrierGenericUMTS = 'Generic UMTS';
 
   /**
@@ -136,6 +127,49 @@ cr.define('options.internet', function() {
     return netmask;
   }
 
+  /**
+   * Returns the prefix length from the netmask string.
+   * @param {string} netmask The netmask string, e.g. 255.255.255.0.
+   * @return {number} The corresponding netmask or -1 if invalid.
+   */
+  function netmaskToPrefixLength(netmask) {
+    var prefixLength = 0;
+    var tokens = netmask.split('.');
+    if (tokens.length != 4)
+      return -1;
+    for (var i = 0; i < tokens.length; ++i) {
+      var token = tokens[i];
+      // If we already found the last mask and the current one is not
+      // '0' then the netmask is invalid. For example, 255.224.255.0
+      if (prefixLength / 8 != i) {
+        if (token != '0')
+          return -1;
+      } else if (token == '255') {
+        prefixLength += 8;
+      } else if (token == '254') {
+        prefixLength += 7;
+      } else if (token == '252') {
+        prefixLength += 6;
+      } else if (token == '248') {
+        prefixLength += 5;
+      } else if (token == '240') {
+        prefixLength += 4;
+      } else if (token == '224') {
+        prefixLength += 3;
+      } else if (token == '192') {
+        prefixLength += 2;
+      } else if (token == '128') {
+        prefixLength += 1;
+      } else if (token == '0') {
+        prefixLength += 0;
+      } else {
+        // mask is not a valid number.
+        return -1;
+      }
+    }
+    return prefixLength;
+  }
+
   /////////////////////////////////////////////////////////////////////////////
   // DetailsInternetPage class:
 
@@ -354,33 +388,55 @@ cr.define('options.internet', function() {
     },
 
     /**
-     * Sends the IP Config info to chrome.
+     * Gets the IP Config ONC Object.
      * @param {string} nameServerType The selected name server type:
      *   'automatic', 'google', or 'user'.
+     * @return {Object} The IP Config ONC object.

[pneubeck (no reviews), 2014/11/25 21:13:34]

nit: IP Config -> IPConfig

[stevenjb, 2014/11/25 21:59:47]

Done.

      * @private
      */
-    sendIpConfig_: function(nameServerType) {
-      var userNameServerString = '';
-      if (nameServerType == 'user') {
+    getIpConfig_: function(nameServerType) {
+      var ipConfig = {};
+      if ($('ip-automatic-configuration-checkbox').checked) {
+        // Empty IPAddress property indicates automatic IP configuration.
+        ipConfig['IPAddress'] = '';
+      } else {
+        // Note: If the ip-address field is left blank, this will revert to
+        // 'automatic' configuration.
+        ipConfig['IPAddress'] = $('ip-address').model.value || '';
+        var netmask = $('ip-netmask').model.value;
+        var routingPrefix = 0;
+        if (netmask) {
+          routingPrefix = netmaskToPrefixLength(netmask);
+          if (routingPrefix == -1) {
+            console.error('Invalid netmask: ' + netmask);
+            routingPrefix = 0;
+          }
+        }
+        ipConfig['RoutingPrefix'] = routingPrefix;
+        ipConfig['Gateway'] = $('ip-gateway').model.value || '';
+      }
+      if (nameServerType == 'automatic') {
+        // Empty NameServers list indicates automatic nameservers.
+        ipConfig['NameServers'] = [];
+      } else {
+        // Note: If no nameserver fields are set, behavior will revert to
+        // 'automatic'. TODO(stevenjb): Validate input fields.
         var userNameServers = [];
-        for (var i = 1; i <= 4; ++i) {
-          var nameServerField = $('ipconfig-dns' + i);
-          // Skip empty values.
-          if (nameServerField && nameServerField.model &&
-              nameServerField.model.value) {
-            userNameServers.push(nameServerField.model.value);
+        if (nameServerType == 'google') {
+          userNameServers = GoogleNameServers;
+        } else if (nameServerType == 'user') {
+          for (var i = 1; i <= 4; ++i) {
+            var nameServerField = $('ipconfig-dns' + i);
+            // Skip empty values.
+            if (nameServerField && nameServerField.model &&
+                nameServerField.model.value) {
+              userNameServers.push(nameServerField.model.value);
+            }
           }
         }
-        userNameServerString = userNameServers.sort().join(',');
+        ipConfig['NameServers'] = userNameServers.sort();
       }
-      chrome.send('setIPConfig',
-                  [this.servicePath_,
-                   Boolean($('ip-automatic-configuration-checkbox').checked),
-                   $('ip-address').model.value || '',
-                   $('ip-netmask').model.value || '',
-                   $('ip-gateway').model.value || '',
-                   nameServerType,
-                   userNameServerString]);
+      return ipConfig;
     },
 
     /**
@@ -1108,7 +1164,8 @@ cr.define('options.internet', function() {
         break;
       }
     }
-    detailsPage.sendIpConfig_(nameServerType);
+    oncData.setProperty('StaticIPConfig',
+                        detailsPage.getIpConfig_(nameServerType));
 
     var data = oncData.getData();
     if (Object.keys(data).length > 0) {
@@ -1268,22 +1325,23 @@ cr.define('options.internet', function() {
     // if they are set.
     var savedNameServersString;
     var savedIpAddress = onc.getActiveValue('SavedIPConfig.IPAddress');
-    if (savedIpAddress != undefined) {
+    if (savedIpAddress) {

[pneubeck (no reviews), 2014/11/25 21:13:34]

I'm not sure why this block is changing? I didn't see a change to SavedIPConfig.

[stevenjb, 2014/11/25 21:59:47]

I wanted to be consistent with the changes for Static. In practice this should
never happen, but it seems to me that it also doesn't make sense to keep track
of saved gateway/netmask if there is no saved  IP Address. Updated toe comment
to match.

       inetAddress.automatic = savedIpAddress;
       inetAddress.value = savedIpAddress;
-    }
-    var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
-    if (savedPrefix != undefined) {
-      assert(typeof savedPrefix == 'number');
-      var savedNetmask = prefixLengthToNetmask(
-          /** @type {number} */(savedPrefix));
-      inetNetmask.automatic = savedNetmask;
-      inetNetmask.value = savedNetmask;
-    }
-    var savedGateway = onc.getActiveValue('SavedIPConfig.Gateway');
-    if (savedGateway != undefined) {
-      inetGateway.automatic = savedGateway;
-      inetGateway.value = savedGateway;
+
+      var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
+      if (savedPrefix != undefined) {
+        assert(typeof savedPrefix == 'number');
+        var savedNetmask = prefixLengthToNetmask(
+            /** @type {number} */(savedPrefix));
+        inetNetmask.automatic = savedNetmask;
+        inetNetmask.value = savedNetmask;
+      }
+      var savedGateway = onc.getActiveValue('SavedIPConfig.Gateway');
+      if (savedGateway != undefined) {
+        inetGateway.automatic = savedGateway;
+        inetGateway.value = savedGateway;
+      }
     }
     var savedNameServers = onc.getActiveValue('SavedIPConfig.NameServers');
     if (savedNameServers) {
@@ -1293,25 +1351,27 @@ cr.define('options.internet', function() {
 
     var ipAutoConfig = 'automatic';
 
+    // A non-empty static IP Address value indicates a custom IP Config.
     var staticNameServersString;
     var staticIpAddress = onc.getActiveValue('StaticIPConfig.IPAddress');
-    if (staticIpAddress != undefined) {
+    if (staticIpAddress) {
       ipAutoConfig = 'user';
       inetAddress.user = staticIpAddress;
       inetAddress.value = staticIpAddress;
-    }
-    var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
-    if (staticPrefix != undefined) {
-      assert(typeof staticPrefix == 'number');
-      var staticNetmask = prefixLengthToNetmask(
-          /** @type {number} */(staticPrefix));
-      inetNetmask.user = staticNetmask;
-      inetNetmask.value = staticNetmask;
-    }
-    var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
-    if (staticGateway != undefined) {
-      inetGateway.user = staticGateway;
-      inetGateway.value = staticGateway;
+
+      var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
+      if (staticPrefix != undefined) {
+        assert(typeof staticPrefix == 'number');
+        var staticNetmask = prefixLengthToNetmask(
+            /** @type {number} */(staticPrefix));
+        inetNetmask.user = staticNetmask;
+        inetNetmask.value = staticNetmask;
+      }
+      var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
+      if (staticGateway != undefined) {
+        inetGateway.user = staticGateway;
+        inetGateway.value = staticGateway;
+      }
     }
     var staticNameServers = onc.getActiveValue('StaticIPConfig.NameServers');
     if (staticNameServers) {
@@ -1340,7 +1400,7 @@ cr.define('options.internet', function() {
     if (staticNameServersString) {
       // If static nameservers are defined and match the google name servers,
       // show that in the UI, otherwise show the custom static nameservers.
-      if (staticNameServersString == GoogleNameServersString)
+      if (staticNameServersString == GoogleNameServers.join(','))
         nameServerType = 'google';
       else if (staticNameServersString == inetNameServersString)
         nameServerType = 'user';
@@ -1349,7 +1409,7 @@ cr.define('options.internet', function() {
       $('automatic-dns-display').textContent = inetNameServersString;
     else
       $('automatic-dns-display').textContent = savedNameServersString;
-    $('google-dns-display').textContent = GoogleNameServersString;
+    $('google-dns-display').textContent = GoogleNameServers.join(',');
 
     var nameServersUser = [];
     if (staticNameServers) {