commit d4974cd523440171033048d03d27730b47f50cf2
author Thieu Le <thieule@chromium.org> Thu May 23 10:39:28 2013 -0700
committer ChromeBot <chrome-bot@google.com> Thu May 23 14:42:24 2013 -0700
tree da07d6b364ecfdaf81446267ab4fae8d3b4ec75f
parent 40d992cc0f7356f179b31ae71473538e0e09fd97

shill: cellular: Fix crash if service disappears before connect finishes

BUG=chromium:238360
TEST=Unit tests

Change-Id: Ie52b570d75cf8d17b8152e8ac99fcb2231690e30
Reviewed-on: https://gerrit.chromium.org/gerrit/56420
Reviewed-by: Arman Uguray <armansito@chromium.org>
Commit-Queue: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>

cellular.cc

diff --git a/cellular.cc b/cellular.cc
index 5d1c5c5..ac5724a 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -537,7 +537,10 @@
                           weak_ptr_factory_.GetWeakPtr());
   manager()->AddTerminationAction(FriendlyName(), start_cb);
   SetState(kStateConnected);
-  if (!capability_->AllowRoaming() &&
+  if (!service_) {
+    LOG(INFO) << "Disconnecting due to no cellular service.";
+    Disconnect(NULL);
+  } else if (!capability_->AllowRoaming() &&
       service_->roaming_state() == flimflam::kRoamingStateRoaming) {
     LOG(INFO) << "Disconnecting due to roaming.";
     Disconnect(NULL);

cellular.h

diff --git a/cellular.h b/cellular.h
index d9e94c7..878a85c 100644
--- a/cellular.h
+++ b/cellular.h
@@ -266,6 +266,7 @@
   FRIEND_TEST_ALL_PREFIXES(CellularTest, ConnectAddsTerminationAction);
   FRIEND_TEST(CellularTest, ConnectFailure);
   FRIEND_TEST(CellularTest, ConnectFailureNoService);
+  FRIEND_TEST(CellularTest, ConnectSuccessNoService);
   FRIEND_TEST(CellularTest, CustomSetterNoopChange);
   FRIEND_TEST(CellularTest, DisableModem);
   FRIEND_TEST(CellularTest, Disconnect);

cellular_capability_classic.h

diff --git a/cellular_capability_classic.h b/cellular_capability_classic.h
index 334f7aa..b9ec6f6 100644
--- a/cellular_capability_classic.h
+++ b/cellular_capability_classic.h
@@ -171,6 +171,7 @@
   FRIEND_TEST_ALL_PREFIXES(CellularTest, ConnectAddsTerminationAction);
   FRIEND_TEST(CellularTest, ConnectFailure);
   FRIEND_TEST(CellularTest, ConnectFailureNoService);
+  FRIEND_TEST(CellularTest, ConnectSuccessNoService);
   FRIEND_TEST(CellularTest, Disconnect);
   FRIEND_TEST(CellularTest, DisconnectFailure);
   FRIEND_TEST(CellularTest, DisconnectWithCallback);

cellular_unittest.cc

diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 435a8fe..7289036 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -263,6 +263,12 @@
     device_->service_ = NULL;
     callback.Run(Error(Error::kNotOnHomeNetwork));
   }
+  void InvokeConnectSuccessNoService(DBusPropertiesMap props, Error *error,
+                                     const ResultCallback &callback,
+                                     int timeout) {
+    device_->service_ = NULL;
+    callback.Run(Error());
+  }
   void InvokeDisconnect(Error *error, const ResultCallback &callback,
                         int timeout) {
     if (!callback.is_null())
@@ -696,6 +702,22 @@
   device_->Connect(&error);
 }
 
+TEST_F(CellularTest, ConnectSuccessNoService) {
+  // Make sure we don't crash if the connect succeeds but the service was
+  // destroyed before the connect request completes.
+  SetCellularType(Cellular::kTypeCDMA);
+  device_->state_ = Cellular::kStateRegistered;
+  SetService();
+  EXPECT_CALL(
+      *simple_proxy_,
+      Connect(_, _, _, CellularCapability::kTimeoutConnect))
+      .WillOnce(Invoke(this, &CellularTest::InvokeConnectSuccessNoService));
+  EXPECT_CALL(*modem_info_.mock_manager(), UpdateService(_));
+  GetCapabilityClassic()->simple_proxy_.reset(simple_proxy_.release());
+  Error error;
+  device_->Connect(&error);
+}
+
 TEST_F(CellularTest, LinkEventWontDestroyService) {
   // If the network interface goes down, Cellular::LinkEvent should
   // drop the connection but the service object should persist.