Fix always-in-roaming reset loop for cellular connections on Chromium OS.

Fix always-in-roaming reset loop for cellular connections on Chromium OS.

Chrome would not incorrectly switch the roaming setting back and forth
when the cellular connection is marked as always roaming and the roaming
device settings was off.

BUG=chromium-os:33439
TEST=Manual, see bug.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151320

[Mattias Nissler (ping if slow), 2012-08-13 09:28:06]

Friendly review request.

[petkov, 2012-08-13 09:55:13]

http://codereview.chromium.org/10854112/diff/1/chrome/browser/chromeos/cros/n...
File chrome/browser/chromeos/cros/network_library_impl_cros.cc (right):

http://codereview.chromium.org/10854112/diff/1/chrome/browser/chromeos/cros/n...
chrome/browser/chromeos/cros/network_library_impl_cros.cc:1181: if
(!device->data_roaming_allowed() && IsCellularAlwaysInRoaming()) {
Should we change this logic too?

[Mattias Nissler (ping if slow), 2012-08-13 11:05:00]

http://codereview.chromium.org/10854112/diff/1/chrome/browser/chromeos/cros/n...
File chrome/browser/chromeos/cros/network_library_impl_cros.cc (right):

http://codereview.chromium.org/10854112/diff/1/chrome/browser/chromeos/cros/n...
chrome/browser/chromeos/cros/network_library_impl_cros.cc:1181: if
(!device->data_roaming_allowed() && IsCellularAlwaysInRoaming()) {
On 2012/08/13 09:55:13, petkov wrote:
> Should we change this logic too?

Yes, good catch. I think in this case it actually wouldn't have caused a loop,
but it's incorrect here as well. There's really no good reason to have the code
twice though, so I've removed this instance entirely.

[petkov, 2012-08-13 12:33:55]

LGTM FWIW

[stevenjb, 2012-08-13 16:36:38]

LGTM

[commit-bot: I haz the power, 2012-08-13 16:41:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10854112/9001

[commit-bot: I haz the power, 2012-08-13 18:46:38]

Change committed as 151320