commit 3f9131c8dc99939004392a6fc5b7b5a70aee793e
author Darin Petkov <petkov@chromium.org> Tue Mar 20 11:37:32 2012 +0100
committer Gerrit <chrome-bot@google.com> Tue Mar 20 08:22:14 2012 -0700
tree e3291cf36e019554c5721923157c9aec98f2d610
parent 536820ddd33f63e1d95e6e04e1757bd0248bbe38

shill: vpn: Cleanup VPN service disconnect and destruction process.

BUG=chromium-os:28078
TEST=tested on device

Change-Id: Iafccdd28a6b032d088f7b1e4930f3a7bf700626f
Reviewed-on: https://gerrit.chromium.org/gerrit/18528
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>

openvpn_driver.cc

diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index 990c74a..af9ace2 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -61,10 +61,10 @@
       child_watch_tag_(0) {}
 
 OpenVPNDriver::~OpenVPNDriver() {
-  Cleanup();
+  Cleanup(Service::kStateIdle);
 }
 
-void OpenVPNDriver::Cleanup() {
+void OpenVPNDriver::Cleanup(Service::ConnectState state) {
   if (child_watch_tag_) {
     glib_->SourceRemove(child_watch_tag_);
     child_watch_tag_ = 0;
@@ -84,7 +84,7 @@
   }
   tunnel_interface_.clear();
   if (service_) {
-    service_->SetState(Service::kStateFailure);
+    service_->SetState(state);
     service_ = NULL;
   }
 }
@@ -138,7 +138,7 @@
   OpenVPNDriver *me = reinterpret_cast<OpenVPNDriver *>(data);
   me->child_watch_tag_ = 0;
   CHECK_EQ(pid, me->pid_);
-  me->Cleanup();
+  me->Cleanup(Service::kStateFailure);
   // TODO(petkov): Figure if we need to restart the connection.
 }
 
@@ -157,7 +157,7 @@
   device_->SelectService(service_);
   rpc_task_.reset(new RPCTask(control_, this));
   if (!SpawnOpenVPN()) {
-    Cleanup();
+    Cleanup(Service::kStateFailure);
   }
   return true;
 }
@@ -307,7 +307,7 @@
   if (!device_info_->CreateTunnelInterface(&tunnel_interface_)) {
     Error::PopulateAndLog(
         error, Error::kInternalError, "Could not create tunnel interface.");
-    Cleanup();
+    Cleanup(Service::kStateFailure);
   }
   // Wait for the ClaimInterface callback to continue the connection process.
 }
@@ -474,13 +474,7 @@
 }
 
 void OpenVPNDriver::Disconnect() {
-  if (device_) {
-    device_->OnDisconnected();
-    device_->SelectService(NULL);
-  }
-  service_->SetState(Service::kStateIdle);
-  service_ = NULL;
-  Cleanup();
+  Cleanup(Service::kStateIdle);
 }
 
 }  // namespace shill

openvpn_driver.h

diff --git a/openvpn_driver.h b/openvpn_driver.h
index e6598c4..86b1899 100644
--- a/openvpn_driver.h
+++ b/openvpn_driver.h
@@ -17,6 +17,7 @@
 #include "shill/key_value_store.h"
 #include "shill/refptr_types.h"
 #include "shill/rpc_task.h"
+#include "shill/service.h"
 #include "shill/vpn_driver.h"
 
 namespace shill {
@@ -110,7 +111,7 @@
   bool PinHostRoute(const IPConfig::Properties &properties);
 
   bool SpawnOpenVPN();
-  void Cleanup();
+  void Cleanup(Service::ConnectState state);
 
   // Called when the openpvn process exits.
   static void OnOpenVPNDied(GPid pid, gint status, gpointer data);

openvpn_driver_unittest.cc

diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index 6e39ea6..77ebeaf 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -401,7 +401,7 @@
   EXPECT_CALL(*device_, Stop());
   EXPECT_CALL(device_info_, DeleteInterface(kInterfaceIndex));
   EXPECT_CALL(*service_, SetState(Service::kStateFailure));
-  driver_->Cleanup();
+  driver_->Cleanup(Service::kStateFailure);
   EXPECT_EQ(0, driver_->child_watch_tag_);
   EXPECT_EQ(0, driver_->pid_);
   EXPECT_FALSE(driver_->rpc_task_.get());
@@ -445,7 +445,6 @@
 TEST_F(OpenVPNDriverTest, Disconnect) {
   driver_->device_ = device_;
   driver_->service_ = service_;
-  EXPECT_CALL(*device_, OnDisconnected());
   EXPECT_CALL(*device_, Stop());
   EXPECT_CALL(device_info_, DeleteInterface(kInterfaceIndex));
   EXPECT_CALL(*service_, SetState(Service::kStateIdle));