commit 3d27539b13590e0b149440ce017fdc401ef6e735
author Thieu Le <thieule@chromium.org> Fri Jul 20 15:32:58 2012 -0700
committer Gerrit <chrome-bot@google.com> Mon Jul 23 12:43:58 2012 -0700
tree 966fdca23c74f7539c8c089df0ef2c0103f64897
parent 3f43f432700a31526a57c413a1872284d766ac2f

shill: Check modem proxy validity before Disconnecting.

BUG=chromium-os:32668
TEST=Unit tests, cellular_Smoke, cellular_ModemControl

Change-Id: I457fe576317a89216efb203c0b90e4279e6f8c6a
Reviewed-on: https://gerrit.chromium.org/gerrit/28109
Reviewed-by: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
Commit-Ready: Thieu Le <thieule@chromium.org>

cellular_capability_classic.cc

diff --git a/cellular_capability_classic.cc b/cellular_capability_classic.cc
index a33133d..08c93c9 100644
--- a/cellular_capability_classic.cc
+++ b/cellular_capability_classic.cc
@@ -226,7 +226,8 @@
 void CellularCapabilityClassic::Disconnect(Error *error,
                                     const ResultCallback &callback) {
   SLOG(Cellular, 2) << __func__;
-  proxy_->Disconnect(error, callback, kTimeoutDefault);
+  if (proxy_.get())
+    proxy_->Disconnect(error, callback, kTimeoutDefault);
 }
 
 void CellularCapabilityClassic::Activate(const string &/*carrier*/,

cellular_capability_classic_unittest.cc

diff --git a/cellular_capability_classic_unittest.cc b/cellular_capability_classic_unittest.cc
index 1b87375..0a12c74 100644
--- a/cellular_capability_classic_unittest.cc
+++ b/cellular_capability_classic_unittest.cc
@@ -106,6 +106,10 @@
     return dynamic_cast<CellularCapabilityGSM *>(cellular_->capability_.get());
   }
 
+  void ReleaseCapabilityProxies() {
+    capability_->ReleaseProxies();
+  }
+
   void InvokeEnable(bool enable, Error *error,
                     const ResultCallback &callback, int timeout) {
     callback.Run(Error());
@@ -432,4 +436,13 @@
   dispatcher_.DispatchPendingEvents();
 }
 
+TEST_F(CellularCapabilityTest, DisconnectNoProxy) {
+  Error error;
+  ResultCallback disconnect_callback;
+  EXPECT_CALL(*proxy_, Disconnect(_, _, CellularCapability::kTimeoutDefault))
+      .Times(0);
+  ReleaseCapabilityProxies();
+  capability_->Disconnect(&error, disconnect_callback);
+}
+
 }  // namespace shill

cellular_capability_universal.cc

diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index eee024d..f3ff509 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -307,7 +307,7 @@
   if (bearer_path_.empty()) {
     LOG(WARNING) << "In " << __func__ << "(): "
                  << "Ignoring attempt to disconnect without bearer";
-  } else {
+  } else if (modem_simple_proxy_.get()) {
     modem_simple_proxy_->Disconnect(bearer_path_,
                                     error,
                                     callback,

cellular_capability_universal.h

diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index b0687b5..f6c97d6 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -117,18 +117,19 @@
   static const char kE362ModelId[];
 
   friend class CellularTest;
-  friend class CellularCapabilityUniversalTest;
   friend class CellularCapabilityTest;
+  friend class CellularCapabilityUniversalTest;
+  FRIEND_TEST(CellularCapabilityUniversalTest, Connect);
+  FRIEND_TEST(CellularCapabilityUniversalTest, ConnectApns);
+  FRIEND_TEST(CellularCapabilityUniversalTest, DisconnectNoProxy);
+  FRIEND_TEST(CellularCapabilityUniversalTest, GetTypeString);
+  FRIEND_TEST(CellularCapabilityUniversalTest, PropertiesChanged);
+  FRIEND_TEST(CellularCapabilityUniversalTest, Scan);
+  FRIEND_TEST(CellularCapabilityUniversalTest, ScanFailure);
+  FRIEND_TEST(CellularCapabilityUniversalTest, SimPropertiesChanged);
   FRIEND_TEST(CellularCapabilityUniversalTest, StartModem);
   FRIEND_TEST(CellularCapabilityUniversalTest, StopModem);
   FRIEND_TEST(CellularCapabilityUniversalTest, StopModemConnected);
-  FRIEND_TEST(CellularCapabilityUniversalTest, PropertiesChanged);
-  FRIEND_TEST(CellularCapabilityUniversalTest, SimPropertiesChanged);
-  FRIEND_TEST(CellularCapabilityUniversalTest, Scan);
-  FRIEND_TEST(CellularCapabilityUniversalTest, ScanFailure);
-  FRIEND_TEST(CellularCapabilityUniversalTest, Connect);
-  FRIEND_TEST(CellularCapabilityUniversalTest, ConnectApns);
-  FRIEND_TEST(CellularCapabilityUniversalTest, GetTypeString);
 
   // Methods used in starting a modem
   void Start_EnableModemCompleted(const ResultCallback &callback,

cellular_capability_universal_unittest.cc

diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index f466b62..5960dad 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -148,6 +148,10 @@
     capability_->modem_simple_proxy_.reset(modem_simple_proxy_.release());
   }
 
+  void ReleaseCapabilityProxies() {
+    capability_->ReleaseProxies();
+  }
+
   MOCK_METHOD1(TestCallback, void(const Error &error));
 
  protected:
@@ -345,13 +349,23 @@
 TEST_F(CellularCapabilityUniversalTest, DisconnectModemNoBearer) {
   Error error;
   ResultCallback disconnect_callback;
-  mm1::MockModemSimpleProxy *modem_simple_proxy = modem_simple_proxy_.get();
-  EXPECT_CALL(*modem_simple_proxy,
+  EXPECT_CALL(*modem_simple_proxy_,
               Disconnect(_, _, _, CellularCapability::kTimeoutDefault))
       .Times(0);
   capability_->Disconnect(&error, disconnect_callback);
 }
 
+TEST_F(CellularCapabilityUniversalTest, DisconnectNoProxy) {
+  Error error;
+  ResultCallback disconnect_callback;
+  capability_->bearer_path_ = "/foo";
+  EXPECT_CALL(*modem_simple_proxy_,
+              Disconnect(_, _, _, CellularCapability::kTimeoutDefault))
+      .Times(0);
+  ReleaseCapabilityProxies();
+  capability_->Disconnect(&error, disconnect_callback);
+}
+
 TEST_F(CellularCapabilityUniversalTest, PropertiesChanged) {
   // Set up mock modem properties
   DBusPropertiesMap modem_properties;