Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Fix nameserver logic, remove reconnect request
Browse files Browse the repository at this point in the history
This CL does the following:
1. Fixes a logic error around correctly detecting when to set
   the radio button to 'Google name servers'.
2. Removes the reconnect request after IPConfig changes.
   This has been unnecessary for some time but was never
   removed from Chrome.

BUG=417387

Review URL: https://codereview.chromium.org/624113002

Cr-Commit-Position: refs/heads/master@{#298899}
  • Loading branch information
stevenjb authored and Commit bot committed Oct 9, 2014
1 parent b73f187 commit 2bcfdba
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 69 deletions.
69 changes: 43 additions & 26 deletions chrome/browser/resources/options/chromeos/internet_detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,36 @@ cr.define('options.internet', function() {
DetailsInternetPage.updateNameServerDisplay(type);
},

/**
* Sends the IP Config info to chrome.
* @param {string} nameServerType The selected name server type:
* 'automatic', 'google', or 'user'.
* @private
*/
sendIpConfig_: function(nameServerType) {
var userNameServerString = '';
if (nameServerType == 'user') {
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);
}
}
userNameServerString = userNameServers.sort().join(',');
}
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]);
},

/**
* Creates an indicator event for controlled properties using
* the same dictionary format as CoreOptionsHandler::CreateValueForPref.
Expand Down Expand Up @@ -1104,26 +1134,8 @@ cr.define('options.internet', function() {
break;
}
}
detailsPage.sendIpConfig_(nameServerType);

// Skip any empty values.
var userNameServers = [];
for (var i = 1; i <= 4; ++i) {
var nameServerField = $('ipconfig-dns' + i);
if (nameServerField && nameServerField.model &&
nameServerField.model.value) {
userNameServers.push(nameServerField.model.value);
}
}

userNameServers = userNameServers.sort();
chrome.send('setIPConfig',
[servicePath,
Boolean($('ip-automatic-configuration-checkbox').checked),
$('ip-address').model.value || '',
$('ip-netmask').model.value || '',
$('ip-gateway').model.value || '',
nameServerType,
userNameServers.join(',')]);
PageManager.closeOverlay();
};

Expand Down Expand Up @@ -1357,19 +1369,24 @@ cr.define('options.internet', function() {

// Set Nameserver fields.
var nameServerType = 'automatic';
if (staticNameServersString &&
staticNameServersString == inetNameServersString) {
nameServerType = 'user';
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)
nameServerType = 'google';
else if (staticNameServersString == inetNameServersString)
nameServerType = 'user';
}
if (inetNameServersString == GoogleNameServersString)
nameServerType = 'google';

$('automatic-dns-display').textContent = inetNameServersString;
$('google-dns-display').textContent = GoogleNameServersString;

var nameServersUser = [];
if (staticNameServers)
if (staticNameServers) {
nameServersUser = staticNameServers;
} else if (savedNameServers) {
// Pre-populate with values provided by DHCP server.
nameServersUser = savedNameServers;
}

var nameServerModels = [];
for (var i = 0; i < 4; ++i)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,6 @@ bool AddIntegerPropertyIfChanged(const std::string& key,
return false;
}

void RequestReconnect(const std::string& service_path) {
NetworkHandler::Get()->network_connection_handler()->DisconnectNetwork(
service_path,
base::Bind(&ash::network_connect::ConnectToNetwork, service_path),
base::Bind(&ShillError, "RequestReconnect"));
}

} // namespace

InternetOptionsHandler::InternetOptionsHandler()
Expand Down Expand Up @@ -766,75 +759,76 @@ void InternetOptionsHandler::SetIPConfigProperties(
NOTREACHED();
return;
}
NET_LOG_USER("SetIPConfigProperties", service_path);
NET_LOG_USER("SetIPConfigProperties: " + name_server_type, service_path);

bool request_reconnect = false;
std::vector<std::string> properties_to_clear;
base::DictionaryValue properties_to_set;

if (dhcp_for_ip) {
request_reconnect |= AppendPropertyKeyIfPresent(
shill::kStaticIPAddressProperty,
shill_properties, &properties_to_clear);
request_reconnect |= AppendPropertyKeyIfPresent(
shill::kStaticIPPrefixlenProperty,
shill_properties, &properties_to_clear);
request_reconnect |= AppendPropertyKeyIfPresent(
shill::kStaticIPGatewayProperty,
shill_properties, &properties_to_clear);
AppendPropertyKeyIfPresent(shill::kStaticIPAddressProperty,
shill_properties,
&properties_to_clear);
AppendPropertyKeyIfPresent(shill::kStaticIPPrefixlenProperty,
shill_properties,
&properties_to_clear);
AppendPropertyKeyIfPresent(shill::kStaticIPGatewayProperty,
shill_properties,
&properties_to_clear);
} else {
request_reconnect |= AddStringPropertyIfChanged(
shill::kStaticIPAddressProperty,
address, shill_properties, &properties_to_set);
AddStringPropertyIfChanged(shill::kStaticIPAddressProperty,
address,
shill_properties,
&properties_to_set);
int prefixlen = network_util::NetmaskToPrefixLength(netmask);
if (prefixlen < 0) {
LOG(ERROR) << "Invalid prefix length for: " << service_path
<< " with netmask " << netmask;
prefixlen = 0;
}
request_reconnect |= AddIntegerPropertyIfChanged(
shill::kStaticIPPrefixlenProperty,
prefixlen, shill_properties, &properties_to_set);
request_reconnect |= AddStringPropertyIfChanged(
shill::kStaticIPGatewayProperty,
gateway, shill_properties, &properties_to_set);
AddIntegerPropertyIfChanged(shill::kStaticIPPrefixlenProperty,
prefixlen,
shill_properties,
&properties_to_set);
AddStringPropertyIfChanged(shill::kStaticIPGatewayProperty,
gateway,
shill_properties,
&properties_to_set);
}

if (name_server_type == kNameServerTypeAutomatic) {
AppendPropertyKeyIfPresent(shill::kStaticIPNameServersProperty,
shill_properties, &properties_to_clear);
shill_properties,
&properties_to_clear);
} else {
if (name_server_type == kNameServerTypeGoogle)
name_servers = kGoogleNameServers;
AddStringPropertyIfChanged(
shill::kStaticIPNameServersProperty,
name_servers, shill_properties, &properties_to_set);
AddStringPropertyIfChanged(shill::kStaticIPNameServersProperty,
name_servers,
shill_properties,
&properties_to_set);
}

if (!properties_to_clear.empty()) {
NetworkHandler::Get()->network_configuration_handler()->ClearProperties(
service_path, properties_to_clear,
base::Bind(&base::DoNothing),
base::Bind(&ShillError, "ClearIPConfigProperties"));
service_path,
properties_to_clear,
base::Bind(&base::DoNothing),
base::Bind(&ShillError, "ClearIPConfigProperties"));
}
if (!properties_to_set.empty()) {
NetworkHandler::Get()->network_configuration_handler()->SetProperties(
service_path, properties_to_set,
service_path,
properties_to_set,
base::Bind(&base::DoNothing),
base::Bind(&ShillError, "SetIPConfigProperties"));
}
std::string device_path;
shill_properties.GetStringWithoutPathExpansion(
shill::kDeviceProperty, &device_path);
shill_properties.GetStringWithoutPathExpansion(shill::kDeviceProperty,
&device_path);
if (!device_path.empty()) {
base::Closure callback = base::Bind(&base::DoNothing);
// If auto config or a static IP property changed, we need to reconnect
// to the network.
if (request_reconnect)
callback = base::Bind(&RequestReconnect, service_path);
NetworkHandler::Get()->network_device_handler()->RequestRefreshIPConfigs(
device_path,
callback,
base::Bind(&base::DoNothing),
base::Bind(&ShillError, "RequestRefreshIPConfigs"));
}
}
Expand Down

0 comments on commit 2bcfdba

Please sign in to comment.