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..138eff859ce5635a477be70e095faf53c680bd28 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,51 @@ cr.define('options.internet', function() {
     },
 
     /**
-     * Sends the IP Config info to chrome.
+     * Gets the IPConfig ONC Object.
      * @param {string} nameServerType The selected name server type:
      *   'automatic', 'google', or 'user'.
+     * @return {Object} The IPConfig ONC object.
      * @private
      */
-    sendIpConfig_: function(nameServerType) {
-      var userNameServerString = '';
-      if (nameServerType == 'user') {
+    getIpConfig_: function(nameServerType) {
+      var ipConfig = {};
+      // If 'ip-address' is empty, automatic configuration will be used.
+      if (!$('ip-automatic-configuration-checkbox').checked &&
+          $('ip-address').model.value) {
+        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 || '';
+      }
+
+      // Note: If no nameserver fields are set, automatic configuration will be
+      // used. TODO(stevenjb): Validate input fields.
+      if (nameServerType != 'automatic') {
         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;

[pneubeck (no reviews), 2014/12/05 09:47:46]

just be aware that this will not create a copy and any modification of
'userNameServers' will be a modification of GoogleNameServers.
E.g. as does the sort() below.

[stevenjb, 2015/01/08 00:44:39]

Ugh. I hate implicit reference languages. Switched to using slice() (it
shouldn't matter, since GoogleNameServers should be sorted, but seems better
practice).

+        } 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(',');
+        if (userNameServers.length)
+          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,10 +1160,16 @@ cr.define('options.internet', function() {
         break;
       }
     }
-    detailsPage.sendIpConfig_(nameServerType);
+    var ipConfig = detailsPage.getIpConfig_(nameServerType);
+    if (Object.keys(ipConfig).length > 0)
+      oncData.setProperty('IPConfigType', 'Static');
+    else
+      oncData.setProperty('IPConfigType', 'DHCP');
+    oncData.setProperty('StaticIPConfig', ipConfig);
 
     var data = oncData.getData();
     if (Object.keys(data).length > 0) {
+      // TODO(stevenjb): Only set changed properties.
       // TODO(stevenjb): chrome.networkingPrivate.setProperties
       chrome.send('setProperties', [servicePath, data]);
     }
@@ -1270,7 +1328,8 @@ cr.define('options.internet', function() {
     var savedIpAddress = onc.getActiveValue('SavedIPConfig.IPAddress');
     if (savedIpAddress != undefined) {
       inetAddress.automatic = savedIpAddress;
-      inetAddress.value = savedIpAddress;
+      if (!inetAddress.value)
+        inetAddress.value = savedIpAddress;
     }
     var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
     if (savedPrefix != undefined) {
@@ -1278,43 +1337,49 @@ cr.define('options.internet', function() {
       var savedNetmask = prefixLengthToNetmask(
           /** @type {number} */(savedPrefix));
       inetNetmask.automatic = savedNetmask;
-      inetNetmask.value = savedNetmask;
+      if (!inetNetmask.value)
+        inetNetmask.value = savedNetmask;
     }
     var savedGateway = onc.getActiveValue('SavedIPConfig.Gateway');
     if (savedGateway != undefined) {
       inetGateway.automatic = savedGateway;
-      inetGateway.value = savedGateway;
+      if (!inetGateway.value)
+        inetGateway.value = savedGateway;
     }
+
     var savedNameServers = onc.getActiveValue('SavedIPConfig.NameServers');
     if (savedNameServers) {
       savedNameServers = savedNameServers.sort();
       savedNameServersString = savedNameServers.join(',');
     }
 
-    var ipAutoConfig = 'automatic';
-
-    var staticNameServersString;
-    var staticIpAddress = onc.getActiveValue('StaticIPConfig.IPAddress');
-    if (staticIpAddress != undefined) {
+    var ipConfigType = onc.getActiveValue('IPConfigType');
+    var ipAutoConfig;
+    if (ipConfigType == 'Static' && onc.hasValue('StaticIPConfig.IPAddress')) {
       ipAutoConfig = 'user';
+      var staticIpAddress = onc.getActiveValue('StaticIPConfig.IPAddress');
       inetAddress.user = staticIpAddress;
       inetAddress.value = staticIpAddress;
-    }
-    var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
-    if (staticPrefix != undefined) {
-      assert(typeof staticPrefix == 'number');
+
+      var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
+      if (typeof staticPrefix != 'number')
+        staticPrefix = 0;
       var staticNetmask = prefixLengthToNetmask(
-          /** @type {number} */(staticPrefix));
+          /** @type {number} */ (staticPrefix));
       inetNetmask.user = staticNetmask;
       inetNetmask.value = staticNetmask;
-    }
-    var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
-    if (staticGateway != undefined) {
+
+      var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
       inetGateway.user = staticGateway;
       inetGateway.value = staticGateway;
+    } else {
+      ipAutoConfig = 'automatic';
     }
-    var staticNameServers = onc.getActiveValue('StaticIPConfig.NameServers');
-    if (staticNameServers) {
+
+    var staticNameServersString;
+    if (ipConfigType == 'Static' &&
+        onc.hasValue('StaticIPConfig.NameServers')) {

[pneubeck (no reviews), 2014/12/05 09:47:46]

these hasValue checks make me wonder whether you would have preferred to have
ipaddress and nameservers being more separated? so that each can be checked with
a type value?

[stevenjb, 2015/01/08 00:44:39]

Yes, but I'd also rather not mess with it at this point. Maybe later.

+      var staticNameServers = onc.getActiveValue('StaticIPConfig.NameServers');
       staticNameServers = staticNameServers.sort();
       staticNameServersString = staticNameServers.join(',');
     }
@@ -1340,7 +1405,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 +1414,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) {