shill: Disconnect a service on DHCP lease failure
Fix a bug where failing to acquire a DHCP lease would result in
permanent device offline state. This happens because marking a service
as disconnected doesn't work through any of the logic to trigger an
actual disconnect from the actually fully associated service. For
instance, in WiFi's case, this leaves the service associated in the
eye's of wifi.cc, but in some state outside of the typical state machine
for connecting services. Thus shill does nothing, leaving users to
wonder.
device.cc should ask the service to disconnect itself rather than
changing its state unbeknownst to the service. However, there does not
seem to be a path to disconnect a service without marking the disconnect
as user initiated. Create such a path. I've gone over the callsites
and looked for places where they might assume that the service won't be
autoconnected again, and not found problems on inspection. At worst,
we'll just be auto connecting some services we didn't used to. In some
places, like cellular, it seems like we Disconnect when we find we are
roaming, which doesn't seem like a permanent/non autoconnectable
offense.
TEST=Unit tests
BUG=chromium-os:32279
Change-Id: I34b60f3c009c276efac5d7e51495dc5681bf4b5f
Reviewed-on: https://gerrit.chromium.org/gerrit/34195
Commit-Ready: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Tested-by: Christopher Wiley <wiley@chromium.org>
diff --git a/service_unittest.cc b/service_unittest.cc
index 29e9645..d3d143e 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -444,7 +444,7 @@
// We should not auto-connect to a Service that a user has
// deliberately disconnected.
Error error;
- service_->Disconnect(&error);
+ service_->UserInitiatedDisconnect(&error);
EXPECT_FALSE(service_->IsAutoConnectable(&reason));
EXPECT_STREQ(Service::kAutoConnExplicitDisconnect, reason);
@@ -456,11 +456,15 @@
EXPECT_TRUE(service_->IsAutoConnectable(&reason));
// A deliberate Connect should also re-enable auto-connect.
- service_->Disconnect(&error);
+ service_->UserInitiatedDisconnect(&error);
EXPECT_FALSE(service_->IsAutoConnectable(&reason));
service_->Connect(&error);
EXPECT_TRUE(service_->IsAutoConnectable(&reason));
+ // A non-user initiated Disconnect doesn't change anything.
+ service_->Disconnect(&error);
+ EXPECT_TRUE(service_->IsAutoConnectable(&reason));
+
// TODO(quiche): After we have resume handling in place, test that
// we re-enable auto-connect on resume. crosbug.com/25213