This updates the StaticIP configuration UI to match new mocks.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/options2/chromeos/internet_options_handler.h"
 
 #include <ctype.h>
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "base/base64.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #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"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #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"
 #include "chrome/browser/chromeos/mobile_config.h"
 #include "chrome/browser/chromeos/options/network_config_view.h"
 #include "chrome/browser/chromeos/proxy_config_service_impl.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/sim_dialog_delegate.h"
 #include "chrome/browser/chromeos/status/network_menu_icon.h"
 #include "chrome/browser/net/pref_proxy_config_tracker.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/time_format.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/chromium_strings.h"
 #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"
 #include "ui/gfx/display.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
 static const char kOtherNetworksFakePath[] = "?";
 
 // Keys for the network description dictionary passed to the web ui. Make sure
 // to keep the strings in sync with what the Javascript side uses.
 const char kNetworkInfoKeyActivationState[] = "activation_state";
 const char kNetworkInfoKeyConnectable[] = "connectable";
 const char kNetworkInfoKeyConnected[] = "connected";
 const char kNetworkInfoKeyConnecting[] = "connecting";
 const char kNetworkInfoKeyIconURL[] = "iconURL";
 const char kNetworkInfoKeyNeedsNewPlan[] = "needs_new_plan";
 const char kNetworkInfoKeyNetworkName[] = "networkName";
 const char kNetworkInfoKeyNetworkStatus[] = "networkStatus";
 const char kNetworkInfoKeyNetworkType[] = "networkType";
 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 {
  public:
   // Initializes the dictionary with default values.
   explicit NetworkInfoDictionary(float icon_scale);
 
   // Copies in service path, connect{ing|ed|able} flags and connection type from
   // the provided network object. Also chooses an appropriate icon based on the
   // network type.
@@ skipping 159 matching lines @@
   network_info->SetString(kNetworkInfoKeyNetworkName, name_);
   network_info->SetString(kNetworkInfoKeyNetworkStatus, status);
   network_info->SetInteger(kNetworkInfoKeyNetworkType,
                            static_cast<int>(connection_type_));
   network_info->SetBoolean(kNetworkInfoKeyRemembered, remembered_);
   network_info->SetString(kNetworkInfoKeyServicePath, service_path_);
   network_info->SetBoolean(kNetworkInfoKeyPolicyManaged, policy_managed_);
   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 {
 
 InternetOptionsHandler::InternetOptionsHandler()
   : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   registrar_.Add(this, chrome::NOTIFICATION_REQUIRE_PIN_SETTING_CHANGE_ENDED,
       content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_ENTER_PIN_ENDED,
       content::NotificationService::AllSources());
@@ skipping 58 matching lines @@
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CHANGE_PROXY_BUTTON },
     { "managedNetwork", IDS_OPTIONS_SETTINGS_MANAGED_NETWORK },
     { "wifiNetworkTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_CONNECTION },
     { "vpnTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_VPN },
     { "cellularPlanTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_PLAN },
     { "cellularConnTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_CONNECTION },
     { "cellularDeviceTabLabel", IDS_OPTIONS_SETTINGS_INTERNET_TAB_DEVICE },
     { "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 },
     { "activateButton", IDS_OPTIONS_SETTINGS_ACTIVATE },
     { "buyplanButton", IDS_OPTIONS_SETTINGS_BUY_PLAN },
     { "connectButton", IDS_OPTIONS_SETTINGS_CONNECT },
     { "disconnectButton", IDS_OPTIONS_SETTINGS_DISCONNECT },
     { "viewAccountButton", IDS_STATUSBAR_NETWORK_VIEW_ACCOUNT },
 
     // TODO(zelidrag): Change details title to Wimax once we get strings.
@@ skipping 446 matching lines @@
   if (!network)
     return;
 
   bool auto_connect = auto_connect_str == "true";
   if (auto_connect != network->auto_connect())
     network->SetAutoConnect(auto_connect);
 }
 
 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());
   }
 
   const chromeos::NetworkUIData& ui_data = network->ui_data();
   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,
       chromeos::NetworkLibrary::FORMAT_COLON_SEPARATED_HEX);
   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);
   dictionary.SetString("servicePath", network->service_path());
   dictionary.SetBoolean("connecting", network->connecting());
   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();
   dictionary.SetBoolean("showProxy", network_profile != chromeos::PROFILE_NONE);
 
   // Enable static ip config for ethernet. For wifi, enable if flag is set.
   bool staticIPConfig = type == chromeos::TYPE_ETHERNET ||
       (type == chromeos::TYPE_WIFI &&
        CommandLine::ForCurrentProcess()->HasSwitch(
            switches::kEnableStaticIPConfig));
@@ skipping 562 matching lines @@
   if (default_value)
     value_dict->Set("default", default_value->DeepCopy());
   if (ui_data.managed())
     value_dict->SetString("controlledBy", "policy");
   else if (ui_data.recommended())
     value_dict->SetString("controlledBy", "recommended");
   settings->Set(key, value_dict);
 }
 
 }  // namespace options2