This updates the StaticIP configuration UI to match new mocks.

chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc

Index: chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc
diff --git a/chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc b/chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc
index 09dbe3900dea19e75536e99958eacc228c16238f..7a79f1b0fa66f889d40c46d4381b2e7ef366d99f 100644
--- a/chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc
+++ b/chrome/browser/ui/webui/options2/chromeos/internet_options_handler.cc
@@ -18,6 +18,7 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/i18n/time_formatting.h"
+#include "base/json/json_writer.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
@@ -27,6 +28,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/choose_mobile_network_dialog.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros/cros_network_functions.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/cros/onc_constants.h"
 #include "chrome/browser/chromeos/enrollment_dialog_view.h"
@@ -54,6 +56,7 @@
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -81,6 +84,9 @@ const char kNetworkInfoKeyRemembered[] = "remembered";
 const char kNetworkInfoKeyServicePath[] = "servicePath";
 const char kNetworkInfoKeyPolicyManaged[] = "policyManaged";
 
+// Google public name servers (DNS).
+const char kGoogleNameServers[] = "8.8.4.4,8.8.8.8";
+
 // A helper class for building network information dictionaries to be sent to
 // the webui code.
 class NetworkInfoDictionary {
@@ -260,6 +266,68 @@ DictionaryValue* NetworkInfoDictionary::BuildDictionary() {
   return network_info.release();
 }
 
+// Pulls IP information out of a shill service properties dictionary. If
+// |static_ip| is true, then it fetches "StaticIP.*" properties. If not, then it
+// fetches "SavedIP.*" properties. Caller must take ownership of returned
+// dictionary.  If non-NULL, |ip_parameters_set| returns a count of the number
+// of IP routing parameters that get set.
+DictionaryValue* BuildIPInfoDictionary(const DictionaryValue& shill_properties,
+                                       bool static_ip,
+                                       int* routing_parameters_set) {
+  std::string address_key;
+  std::string prefix_len_key;
+  std::string gateway_key;
+  std::string name_servers_key;
+  if (static_ip) {
+    address_key = shill::kStaticIPAddressProperty;
+    prefix_len_key = shill::kStaticIPPrefixlenProperty;
+    gateway_key = shill::kStaticIPGatewayProperty;
+    name_servers_key = shill::kStaticIPNameServersProperty;
+  } else {
+    address_key = shill::kSavedIPAddressProperty;
+    prefix_len_key = shill::kSavedIPPrefixlenProperty;
+    gateway_key = shill::kSavedIPGatewayProperty;
+    name_servers_key = shill::kSavedIPNameServersProperty;
+  }
+
+  scoped_ptr<DictionaryValue> ip_info_dict(new DictionaryValue);
+  std::string value;

[stevenjb, 2012/08/13 17:43:44]

nit: I think this would be more readable using separate specifically named
variables instead of reusing 'value'

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done.

+  int routing_parameters = 0;
+  if (shill_properties.GetStringWithoutPathExpansion(address_key, &value)) {
+    ip_info_dict->SetString("address", value);

[stevenjb, 2012/08/13 17:43:44]

"address" and other JS (I assume?) keys should be defined as constants at the
top of this file.

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done, but I'd like to point out that there are several hundred other instances
of this problem in this file.  I could fix them all, but I'm not sure it's worth
it.

[stevenjb, 2012/08/14 01:06:38]

Thanks for doing these. Anything that is used as a key between C++ and Web UI
really ought to (especially) be defined as a const. It makes finding the shared
keys easier and they tend to be easier to search for within the file. 

If there are other such occurrences in this file, we don't need to fix them all,
but we should definitely use the new consts anywhere that is appropriate.

[Greg Spencer (Chromium), 2012/08/14 01:19:27]

OK, I do agree that they *should* be.  I'll take a stab at fixing everything in
the file, since if it should be that way, we should be consistent.

+    VLOG(2) << "Found " << address_key << ": " << value;
+    routing_parameters++;
+  }
+  int prefix_len = -1;
+  if (shill_properties.GetIntegerWithoutPathExpansion(
+      prefix_len_key, &prefix_len)) {
+    ip_info_dict->SetInteger("prefixLength", prefix_len);
+    ip_info_dict->SetString("netmask",
+                              chromeos::CrosPrefixLengthToNetmask(prefix_len));
+    VLOG(2) << "Found " << prefix_len_key << ": "
+        <<  prefix_len
+        << " (" << chromeos::CrosPrefixLengthToNetmask(prefix_len) << ")";
+    routing_parameters++;
+  }
+  value.clear();
+  if (shill_properties.GetStringWithoutPathExpansion(gateway_key, &value)) {
+    ip_info_dict->SetString("gateway", value);
+    VLOG(2) << "Found " << gateway_key << ": " << value;
+    routing_parameters++;
+  }
+  if (routing_parameters_set)
+    *routing_parameters_set = routing_parameters;
+
+  value.clear();
+  if (shill_properties.GetStringWithoutPathExpansion(
+      name_servers_key, &value)) {
+    ip_info_dict->SetString("nameServers", value);
+    VLOG(2) << "Found " << name_servers_key << ": " << value;
+  }
+
+  return ip_info_dict.release();
+}
+
 }  // namespace
 
 namespace options2 {
@@ -338,13 +406,21 @@ void InternetOptionsHandler::GetLocalizedValues(
     { "networkTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_NETWORK },
     { "securityTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_SECURITY },
     { "proxyTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_PROXY },
-    { "useDHCP", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_USE_DHCP },
-    { "useStaticIP", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_USE_STATIC_IP },
     { "connectionState", IDS_OPTIONS_SETTINGS_INTERNET_CONNECTION_STATE },
     { "inetAddress", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_ADDRESS },
-    { "inetSubnetAddress", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_SUBNETMASK },
+    { "inetNetmask", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_SUBNETMASK },
     { "inetGateway", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_GATEWAY },
-    { "inetDns", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_DNSSERVER },
+    { "inetNameServers", IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_DNSSERVER },
+    { "ipAutomaticConfiguration",
+        IDS_OPTIONS_SETTINGS_INTERNET_IP_AUTOMATIC_CONFIGURATION },
+    { "automaticNameServers",
+        IDS_OPTIONS_SETTINGS_INTERNET_AUTOMATIC_NAME_SERVERS },
+    { "userNameServer1", IDS_OPTIONS_SETTINGS_INTERNET_USER_NAME_SERVER_1 },
+    { "userNameServer2", IDS_OPTIONS_SETTINGS_INTERNET_USER_NAME_SERVER_2 },
+    { "userNameServer3", IDS_OPTIONS_SETTINGS_INTERNET_USER_NAME_SERVER_3 },
+    { "userNameServer4", IDS_OPTIONS_SETTINGS_INTERNET_USER_NAME_SERVER_4 },
+    { "googleNameServers", IDS_OPTIONS_SETTINGS_INTERNET_GOOGLE_NAME_SERVERS },
+    { "userNameServers", IDS_OPTIONS_SETTINGS_INTERNET_USER_NAME_SERVERS },
     { "hardwareAddress",
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_HARDWARE_ADDRESS },
     { "detailsInternetDismiss", IDS_CLOSE },
@@ -811,37 +887,70 @@ void InternetOptionsHandler::SetAutoConnectCallback(const ListValue* args) {
 
 void InternetOptionsHandler::SetIPConfigCallback(const ListValue* args) {
   std::string service_path;
-  std::string dhcp_str;
+  std::string dhcp_for_ip;
   std::string address;
   std::string netmask;
   std::string gateway;
+  std::string name_server_type;
   std::string name_servers;
 
-  if (args->GetSize() < 6 ||
+  if (args->GetSize() < 7 ||
       !args->GetString(0, &service_path) ||
-      !args->GetString(1, &dhcp_str) ||
+      !args->GetString(1, &dhcp_for_ip) ||
       !args->GetString(2, &address) ||
       !args->GetString(3, &netmask) ||
       !args->GetString(4, &gateway) ||
-      !args->GetString(5, &name_servers)) {
+      !args->GetString(5, &name_server_type) ||
+      !args->GetString(6, &name_servers)) {
     NOTREACHED();
     return;
   }
 
-  chromeos::Network* network = cros_->FindNetworkByPath(service_path);
-  if (!network)
-    return;
+  int dhcp_usage_mask = 0;
+  if (dhcp_for_ip == "true") {
+    dhcp_usage_mask = (chromeos::NetworkLibrary::USE_DHCP_ADDRESS |
+                       chromeos::NetworkLibrary::USE_DHCP_NETMASK |
+                       chromeos::NetworkLibrary::USE_DHCP_GATEWAY);
+  }
+  if (name_server_type == "automatic") {

[stevenjb, 2012/08/13 17:43:44]

nit: kNameServerTypeAutomatic

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done.

+    dhcp_usage_mask |= chromeos::NetworkLibrary::USE_DHCP_NAME_SERVERS;
+    name_servers.clear();
+  } else if (name_server_type == "google") {

[stevenjb, 2012/08/13 17:43:44]

nit: kNameServerTypeGoogle

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done.

+    name_servers = kGoogleNameServers;
+  }
 
-  cros_->SetIPConfig(chromeos::NetworkIPConfig(network->device_path(),
-      dhcp_str == "true" ? chromeos::IPCONFIG_TYPE_DHCP :
-                           chromeos::IPCONFIG_TYPE_IPV4,
-      address, netmask, gateway, name_servers));
+  cros_->SetIPParameters(service_path,
+                         address,
+                         netmask,
+                         gateway,
+                         name_servers,
+                         dhcp_usage_mask);
 }
 
 void InternetOptionsHandler::PopulateDictionaryDetails(
     const chromeos::Network* network) {
   DCHECK(network);
 
+  // Send off an asynchronous request to Shill to get the service properties
+  // before we continue.

[stevenjb, 2012/08/13 17:43:44]

nit: 'before we continue' implies waiting to me; 'and continue in the callback'
might be better.

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done.

+  chromeos::CrosRequestNetworkServiceProperties(
+      network->service_path(),
+      base::Bind(&InternetOptionsHandler::PopulateDictionaryDetailsCallback,
+                 weak_factory_.GetWeakPtr(), network));
+}
+
+void InternetOptionsHandler::PopulateDictionaryDetailsCallback(
+    const chromeos::Network* network,
+    const std::string& service_path,
+    const base::DictionaryValue* shill_properties) {
+  if (VLOG_IS_ON(2)) {
+    std::string properties_json;
+    base::JSONWriter::WriteWithOptions(shill_properties,
+                                       base::JSONWriter::OPTIONS_PRETTY_PRINT,
+                                       &properties_json);
+    VLOG(2) << "Shill Properties: " << std::endl << properties_json;
+  }
+
   if (web_ui()) {
     Profile::FromWebUI(web_ui())->GetProxyConfigTracker()->UISetCurrentNetwork(
         network->service_path());
@@ -851,7 +960,7 @@ void InternetOptionsHandler::PopulateDictionaryDetails(
   const base::DictionaryValue* onc =
       cros_->FindOncForNetwork(network->unique_id());
 
-  DictionaryValue dictionary;
+  base::DictionaryValue dictionary;
   std::string hardware_address;
   chromeos::NetworkIPConfigVector ipconfigs = cros_->GetIPConfigs(
       network->device_path(), &hardware_address,
@@ -859,28 +968,58 @@ void InternetOptionsHandler::PopulateDictionaryDetails(
   if (!hardware_address.empty())
     dictionary.SetString("hardwareAddress", hardware_address);
 
-  scoped_ptr<DictionaryValue> ipconfig_dhcp;
-  scoped_ptr<DictionaryValue> ipconfig_static;
+  // The DHCP IPConfig contains the values that are actually in use at the
+  // moment, even if some are overridden by static IP values.
+  scoped_ptr<DictionaryValue> ipconfig_dhcp(new DictionaryValue);
+  std::string ipconfig_name_servers;
   for (chromeos::NetworkIPConfigVector::const_iterator it = ipconfigs.begin();
        it != ipconfigs.end(); ++it) {
     const chromeos::NetworkIPConfig& ipconfig = *it;
-    scoped_ptr<DictionaryValue> ipconfig_dict(new DictionaryValue());
-    ipconfig_dict->SetString("address", ipconfig.address);
-    ipconfig_dict->SetString("subnetAddress", ipconfig.netmask);
-    ipconfig_dict->SetString("gateway", ipconfig.gateway);
-    ipconfig_dict->SetString("dns", ipconfig.name_servers);
-    if (ipconfig.type == chromeos::IPCONFIG_TYPE_DHCP)
-      ipconfig_dhcp.reset(ipconfig_dict.release());
-    else if (ipconfig.type == chromeos::IPCONFIG_TYPE_IPV4)
-      ipconfig_static.reset(ipconfig_dict.release());
+    if (ipconfig.type == chromeos::IPCONFIG_TYPE_DHCP) {
+      ipconfig_dhcp->SetString("address", ipconfig.address);

[stevenjb, 2012/08/13 17:43:44]

Use consts

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done.

+      VLOG(2) << "Found DHCP Address: " << ipconfig.address;
+      ipconfig_dhcp->SetString("netmask", ipconfig.netmask);
+      VLOG(2) << "Found DHCP Netmask: " << ipconfig.netmask;
+      ipconfig_dhcp->SetString("gateway", ipconfig.gateway);
+      VLOG(2) << "Found DHCP Gateway: " << ipconfig.gateway;
+      ipconfig_dhcp->SetString("nameServers", ipconfig.name_servers);
+      ipconfig_name_servers = ipconfig.name_servers; // save for later
+      VLOG(2) << "Found DHCP Name Servers: " << ipconfig.name_servers;
+      break;
+    }
   }
 
   chromeos::NetworkPropertyUIData ipconfig_dhcp_ui_data(ui_data);
-  SetValueDictionary(&dictionary, "ipconfigDHCP", ipconfig_dhcp.release(),
+
+  chromeos::NetworkPropertyUIData static_ip_ui_data(ui_data);
+  int automatic_ip_config;
+  scoped_ptr<DictionaryValue> static_ip_dict(
+      BuildIPInfoDictionary(*shill_properties, true, &automatic_ip_config));
+  dictionary.SetBoolean("ipAutoConfig", automatic_ip_config == 0);

[stevenjb, 2012/08/13 17:43:44]

Use const here and below

[Greg Spencer (Chromium), 2012/08/13 23:53:44]

Done in my changes at least.  Do you want me to fix the entire file (I can, if
you want, but there's a heck of a lot of one-time-use strings in here)?

+  DCHECK(automatic_ip_config == 3 || automatic_ip_config == 0)
+      << "UI doesn't support automatic specification of individual "
+      << "static ip parameters.";
+  scoped_ptr<DictionaryValue> saved_ip_dict(
+      BuildIPInfoDictionary(*shill_properties, false, NULL));
+  dictionary.Set("savedIP", saved_ip_dict.release());
+
+  // Determine what kind of name server setting we have by comparing the
+  // StaticIP and Google values with the ipconfig values.
+  std::string name_server_type = "automatic";
+  std::string static_ip_nameservers;
+  static_ip_dict->GetString("nameServers", &static_ip_nameservers);
+  if (!static_ip_nameservers.empty() &&
+      static_ip_nameservers == ipconfig_name_servers) {
+    name_server_type = "user";
+  }
+  if (ipconfig_name_servers == kGoogleNameServers) {
+    name_server_type = "google";
+  }
+
+  SetValueDictionary(&dictionary, "ipconfig", ipconfig_dhcp.release(),
                      ipconfig_dhcp_ui_data);
-  chromeos::NetworkPropertyUIData ipconfig_static_ui_data(ui_data);
-  SetValueDictionary(&dictionary, "ipconfigStatic", ipconfig_static.release(),
-                     ipconfig_static_ui_data);
+  SetValueDictionary(&dictionary, "staticIP", static_ip_dict.release(),
+                     static_ip_ui_data);
 
   chromeos::ConnectionType type = network->type();
   dictionary.SetInteger("type", type);
@@ -889,6 +1028,8 @@ void InternetOptionsHandler::PopulateDictionaryDetails(
   dictionary.SetBoolean("connected", network->connected());
   dictionary.SetString("connectionState", network->GetStateString());
   dictionary.SetString("networkName", network->name());
+  dictionary.SetString("nameServerType", name_server_type);
+  dictionary.SetString("nameServersGoogle", kGoogleNameServers);
 
   // Only show proxy for remembered networks.
   chromeos::NetworkProfileType network_profile = network->profile_type();