shill: Destroy cellular service after the modem has stopped.
BUG=chromium-os:30889
TEST=network_3GModemControl
Change-Id: I83146b646e54deb5fee85653283b372e589c56c7
Reviewed-on: https://gerrit.chromium.org/gerrit/22755
Commit-Ready: Thieu Le <thieule@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 55675dc..1f8bfb9 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -197,19 +197,14 @@
return;
}
capability_->StartModem(error,
- Bind(&Cellular::OnModemStarted, this, callback));
+ Bind(&Cellular::StartModemCallback, this, callback));
}
void Cellular::Stop(Error *error,
const EnabledStateChangedCallback &callback) {
SLOG(Cellular, 2) << __func__ << ": " << GetStateString(state_);
- if (service_) {
- // TODO(ers): See whether we can/should do DestroyService() here.
- manager()->DeregisterService(service_);
- service_ = NULL;
- }
capability_->StopModem(error,
- Bind(&Cellular::OnModemStopped, this, callback));
+ Bind(&Cellular::StopModemCallback, this, callback));
}
bool Cellular::IsUnderlyingDeviceEnabled() const {
@@ -237,8 +232,8 @@
return false;
}
-void Cellular::OnModemStarted(const EnabledStateChangedCallback &callback,
- const Error &error) {
+void Cellular::StartModemCallback(const EnabledStateChangedCallback &callback,
+ const Error &error) {
SLOG(Cellular, 2) << __func__ << ": " << GetStateString(state_);
if (error.IsSuccess() && (state_ == kStateDisabled)) {
SetState(kStateEnabled);
@@ -249,9 +244,13 @@
callback.Run(error);
}
-void Cellular::OnModemStopped(const EnabledStateChangedCallback &callback,
- const Error &error) {
+void Cellular::StopModemCallback(const EnabledStateChangedCallback &callback,
+ const Error &error) {
SLOG(Cellular, 2) << __func__ << ": " << GetStateString(state_);
+ // Destroy the cellular service regardless of any errors that occur during
+ // the stop process since we do not know the state of the modem at this
+ // point.
+ DestroyService();
if (state_ != kStateDisabled)
SetState(kStateDisabled);
callback.Run(error);
diff --git a/cellular.h b/cellular.h
index 57ad7ca..14a6584 100644
--- a/cellular.h
+++ b/cellular.h
@@ -177,10 +177,10 @@
const std::string &new_pin,
Error *error, const ResultCallback &callback);
- void OnModemStarted(const EnabledStateChangedCallback &callback,
- const Error &error);
- void OnModemStopped(const EnabledStateChangedCallback &callback,
- const Error &error);
+ void StartModemCallback(const EnabledStateChangedCallback &callback,
+ const Error &error);
+ void StopModemCallback(const EnabledStateChangedCallback &callback,
+ const Error &error);
void OnConnecting();
void OnConnected();
void OnConnectFailed(const Error &error);
@@ -213,8 +213,10 @@
FRIEND_TEST(CellularTest, Disconnect);
FRIEND_TEST(CellularTest, ModemStateChangeDisable);
FRIEND_TEST(CellularTest, ModemStateChangeEnable);
- FRIEND_TEST(CellularTest, OnModemStarted);
- FRIEND_TEST(CellularTest, OnModemStartedFail);
+ FRIEND_TEST(CellularTest, StartModemCallback);
+ FRIEND_TEST(CellularTest, StartModemCallbackFail);
+ FRIEND_TEST(CellularTest, StopModemCallback);
+ FRIEND_TEST(CellularTest, StopModemCallbackFail);
FRIEND_TEST(CellularTest, StartConnected);
FRIEND_TEST(CellularTest, StartCDMARegister);
FRIEND_TEST(CellularTest, StartGSMRegister);
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index a81b1e8..6d3cc0c 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -19,6 +19,7 @@
#include "shill/cellular_service.h"
#include "shill/error.h"
#include "shill/event_dispatcher.h"
+#include "shill/mock_cellular_service.h"
#include "shill/mock_device_info.h"
#include "shill/mock_dhcp_config.h"
#include "shill/mock_dhcp_provider.h"
@@ -656,20 +657,50 @@
EXPECT_FALSE(device_->enabled());
}
-TEST_F(CellularTest, OnModemStarted){
+TEST_F(CellularTest, StartModemCallback) {
EXPECT_CALL(*this, TestCallback(IsSuccess()));
EXPECT_EQ(device_->state_, Cellular::kStateDisabled);
- device_->OnModemStarted(Bind(&CellularTest::TestCallback, Unretained(this)),
- Error(Error::kSuccess));
+ device_->StartModemCallback(Bind(&CellularTest::TestCallback,
+ Unretained(this)),
+ Error(Error::kSuccess));
EXPECT_EQ(device_->state_, Cellular::kStateEnabled);
}
-TEST_F(CellularTest, OnModemStartedFail){
+TEST_F(CellularTest, StartModemCallbackFail) {
EXPECT_CALL(*this, TestCallback(IsFailure()));
EXPECT_EQ(device_->state_, Cellular::kStateDisabled);
- device_->OnModemStarted(Bind(&CellularTest::TestCallback, Unretained(this)),
- Error(Error::kOperationFailed));
+ device_->StartModemCallback(Bind(&CellularTest::TestCallback,
+ Unretained(this)),
+ Error(Error::kOperationFailed));
EXPECT_EQ(device_->state_, Cellular::kStateDisabled);
}
+TEST_F(CellularTest, StopModemCallback) {
+ EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ device_->service_ = new MockCellularService(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ device_);
+ device_->StopModemCallback(Bind(&CellularTest::TestCallback,
+ Unretained(this)),
+ Error(Error::kSuccess));
+ EXPECT_EQ(device_->state_, Cellular::kStateDisabled);
+ EXPECT_FALSE(device_->service_.get());
+}
+
+TEST_F(CellularTest, StopModemCallbackFail) {
+ EXPECT_CALL(*this, TestCallback(IsFailure()));
+ device_->service_ = new MockCellularService(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ device_);
+ device_->StopModemCallback(Bind(&CellularTest::TestCallback,
+ Unretained(this)),
+ Error(Error::kOperationFailed));
+ EXPECT_EQ(device_->state_, Cellular::kStateDisabled);
+ EXPECT_FALSE(device_->service_.get());
+}
+
} // namespace shill