commit a63da1dc556b1ec9a5b3676a75665ad1a5fda0bc
author Bin Jin <binjin@chromium.org> Wed Aug 26 12:28:30 2015 +0200
committer chrome-bot <chrome-bot@chromium.org> Wed Sep 02 08:33:03 2015 -0700
tree 6cedec91dd6d849710413d8152bb020c7e0e2752
parent c3227bbcd9777e34884ea77840263044d7e11a91

shill: fix broken 3rd party vpn connection state change

CL:295388 replaced all "connected" connection state to "online",
but this change is not reflected in ThirdPartyVpnDriver and actually
caused failure on notifyConnectionStateChanged(connected) call.

Fix by ignoring Service::kStateOnline in ThirdPartyVpnDriver.

Also sync changes by the previous CL to cloned class in dbus/.

BUG=chromium:524214
TEST=manually tested

Change-Id: Ic082ea63c47ad449e16fc75fc1c2317c5b042dc3
Reviewed-on: https://chromium-review.googlesource.com/295081
Commit-Ready: Bin Jin <binjin@chromium.org>
Tested-by: Bin Jin <binjin@chromium.org>
Reviewed-by: Bartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: Bin Jin <binjin@chromium.org>

dbus/chromeos_third_party_vpn_dbus_adaptor.cc

diff --git a/dbus/chromeos_third_party_vpn_dbus_adaptor.cc b/dbus/chromeos_third_party_vpn_dbus_adaptor.cc
index 310f8da..8f18573 100644
--- a/dbus/chromeos_third_party_vpn_dbus_adaptor.cc
+++ b/dbus/chromeos_third_party_vpn_dbus_adaptor.cc
@@ -33,7 +33,7 @@
     Service::ConnectState* internal_state) {
   switch (external_state) {
     case ChromeosThirdPartyVpnDBusAdaptor::kStateConnected:
-      *internal_state = Service::kStateConnected;
+      *internal_state = Service::kStateOnline;
       break;
     case ChromeosThirdPartyVpnDBusAdaptor::kStateFailure:
       *internal_state = Service::kStateFailure;

vpn/third_party_vpn_driver.cc

diff --git a/vpn/third_party_vpn_driver.cc b/vpn/third_party_vpn_driver.cc
index 4efa2a1..2ecc7f3 100644
--- a/vpn/third_party_vpn_driver.cc
+++ b/vpn/third_party_vpn_driver.cc
@@ -139,14 +139,14 @@
     error_message->append("Unexpected call");
     return;
   }
-  if (service_ && (connection_state == Service::kStateConnected ||
-                   connection_state == Service::kStateFailure)) {
+  if (service_ && connection_state == Service::kStateFailure) {
     service_->SetState(connection_state);
-    if (Service::kStateFailure == connection_state) {
-      Cleanup(Service::kStateFailure, Service::kFailureUnknown,
-              Service::kErrorDetailsNone);
-    }
-  } else {
+    Cleanup(Service::kStateFailure, Service::kFailureUnknown,
+            Service::kErrorDetailsNone);
+  } else if (!service_ || connection_state != Service::kStateOnline) {
+    // We expect "failure" and "connected" messages from the client, but we
+    // only set state for these "failure" messages. "connected" message (which
+    // is corresponding to kStateOnline here) will simply be ignored.
     error_message->append("Invalid argument");
   }
 }

vpn/third_party_vpn_driver_unittest.cc

diff --git a/vpn/third_party_vpn_driver_unittest.cc b/vpn/third_party_vpn_driver_unittest.cc
index 56eacd0..538df52 100644
--- a/vpn/third_party_vpn_driver_unittest.cc
+++ b/vpn/third_party_vpn_driver_unittest.cc
@@ -159,8 +159,8 @@
 
   error.clear();
   driver_->service_ = service_;
-  EXPECT_CALL(*service_, SetState(Service::kStateConnected)).Times(1);
-  driver_->UpdateConnectionState(Service::kStateConnected, &error);
+  EXPECT_CALL(*service_, SetState(_)).Times(0);
+  driver_->UpdateConnectionState(Service::kStateOnline, &error);
   EXPECT_TRUE(error.empty());
   Mock::VerifyAndClearExpectations(service_.get());