Initate VPN reconnections when the default physical service is Online

am: 8e3ac1b9da

* commit '8e3ac1b9da21aac8c0e97049e0a0c8326f32e3ba':
  Initate VPN reconnections when the default physical service is Online

Change-Id: Ib7f7197c77ec4a58ab9f45704e3d10e933954ee7

7 files changed, 151 insertions, 16 deletions

diff --git a/vpn/third_party_vpn_driver.cc b/vpn/third_party_vpn_driver.cc
index d67369d9..75add13c 100644
--- a/vpn/third_party_vpn_driver.cc
+++ b/vpn/third_party_vpn_driver.cc
@@ -547,6 +547,21 @@ void ThirdPartyVpnDriver::OnConnectionDisconnected() {
   SLOG(this, 2) << __func__;
 }
 
+void ThirdPartyVpnDriver::TriggerReconnect(const ServiceRefPtr& new_service) {
+  StartConnectTimeout(kConnectTimeoutSeconds);
+  SLOG(this, 2) << __func__
+                << " - requesting reconnection via "
+                << new_service->unique_name();
+  if (link_down_) {
+    adaptor_interface_->EmitPlatformMessage(
+        static_cast<uint32_t>(PlatformMessage::kLinkUp));
+    link_down_ = false;
+  } else {
+    adaptor_interface_->EmitPlatformMessage(
+        static_cast<uint32_t>(PlatformMessage::kLinkChanged));
+  }
+}
+
 void ThirdPartyVpnDriver::OnDefaultServiceChanged(
     const ServiceRefPtr& service) {
   if (!service_ || !device_)
@@ -561,31 +576,31 @@ void ThirdPartyVpnDriver::OnDefaultServiceChanged(
   device_->SetServiceState(Service::kStateConfiguring);
   device_->ResetConnection();
 
-  if (!service || !service->IsConnected()) {
-    // No physical connection, so all we can do is wait.
+  if (service && service->state() == Service::kStateOnline) {
+    // The original service is no longer the default, but manager was able
+    // to find another physical service that is already Online.
+    // Ask the vpnProvider to reconnect.
+    TriggerReconnect(service);
+  } else {
+    // The default physical service went away, and nothing else is available
+    // right now.  All we can do is wait.
+    if (link_down_)
+      return;
     StopConnectTimeout();
     SLOG(this, 2) << __func__
                   << " - physical connection lost";
     link_down_ = true;
     adaptor_interface_->EmitPlatformMessage(
         static_cast<uint32_t>(PlatformMessage::kLinkDown));
-  } else {
-    // Ask the vpnProvider to reconnect.
-    StartConnectTimeout(kConnectTimeoutSeconds);
-    SLOG(this, 2) << __func__
-                  << " - requesting reconnection via "
-                  << service->unique_name();
-    if (link_down_) {
-      adaptor_interface_->EmitPlatformMessage(
-          static_cast<uint32_t>(PlatformMessage::kLinkUp));
-      link_down_ = false;
-    } else {
-      adaptor_interface_->EmitPlatformMessage(
-          static_cast<uint32_t>(PlatformMessage::kLinkChanged));
-    }
   }
 }
 
+void ThirdPartyVpnDriver::OnDefaultServiceStateChanged(
+    const ServiceRefPtr& service) {
+  if (link_down_ && service->state() == Service::kStateOnline)
+    TriggerReconnect(service);
+}
+
 void ThirdPartyVpnDriver::OnBeforeSuspend(const ResultCallback& callback) {
   if (service_ && reconnect_supported_) {
     // FIXME: Currently the VPN app receives this message at the same time
diff --git a/vpn/third_party_vpn_driver.h b/vpn/third_party_vpn_driver.h
index 43a48659..d6e9bd82 100644
--- a/vpn/third_party_vpn_driver.h
+++ b/vpn/third_party_vpn_driver.h
@@ -85,6 +85,7 @@ class ThirdPartyVpnDriver : public VPNDriver {
   void Disconnect() override;
   void OnConnectionDisconnected() override;
   void OnDefaultServiceChanged(const ServiceRefPtr& service);
+  void OnDefaultServiceStateChanged(const ServiceRefPtr& service) override;
   bool Load(StoreInterface* storage, const std::string& storage_id) override;
   bool Save(StoreInterface* storage, const std::string& storage_id,
             bool save_credentials) override;
@@ -100,6 +101,8 @@ class ThirdPartyVpnDriver : public VPNDriver {
  private:
   friend class ThirdPartyVpnDriverTest;
   FRIEND_TEST(ThirdPartyVpnDriverTest, ConnectAndDisconnect);
+  FRIEND_TEST(ThirdPartyVpnDriverTest, ReconnectionEvents);
+  FRIEND_TEST(ThirdPartyVpnDriverTest, PowerEvents);
   FRIEND_TEST(ThirdPartyVpnDriverTest, SetParameters);
   FRIEND_TEST(ThirdPartyVpnDriverTest, UpdateConnectionState);
   FRIEND_TEST(ThirdPartyVpnDriverTest, SendPacket);
@@ -193,6 +196,10 @@ class ThirdPartyVpnDriver : public VPNDriver {
   void OnInput(InputData* data);
   void OnInputError(const std::string& error);
 
+  // This function is called when a new default service first comes online,
+  // so the app knows it needs to reconnect to the VPN gateway.
+  void TriggerReconnect(const ServiceRefPtr& service);
+
   static const Property kProperties[];
 
   // This variable keeps track of the active instance. There can be multiple
diff --git a/vpn/third_party_vpn_driver_unittest.cc b/vpn/third_party_vpn_driver_unittest.cc
index 477d13ec..5a529062 100644
--- a/vpn/third_party_vpn_driver_unittest.cc
+++ b/vpn/third_party_vpn_driver_unittest.cc
@@ -16,8 +16,10 @@
 
 #include "shill/vpn/third_party_vpn_driver.h"
 
+#include <base/bind.h>
 #include <gtest/gtest.h>
 
+#include "shill/callbacks.h"
 #include "shill/mock_adaptors.h"
 #include "shill/mock_device_info.h"
 #include "shill/mock_event_dispatcher.h"
@@ -66,6 +68,8 @@ class ThirdPartyVpnDriverTest : public testing::Test {
     driver_->file_io_ = nullptr;
   }
 
+  MOCK_METHOD1(TestCallback, void(const Error& error));
+
  protected:
   static const char kConfigName[];
   static const char kInterfaceName[];
@@ -123,6 +127,105 @@ TEST_F(ThirdPartyVpnDriverTest, ConnectAndDisconnect) {
   EXPECT_EQ(driver_->io_handler_.get(), nullptr);
 }
 
+TEST_F(ThirdPartyVpnDriverTest, ReconnectionEvents) {
+  const std::string interface = kInterfaceName;
+  IOHandler* io_handler = new IOHandler();  // Owned by |driver_|
+  int fd = 1;
+
+  EXPECT_CALL(device_info_, CreateTunnelInterface(_))
+      .WillOnce(DoAll(SetArgumentPointee<0>(interface), Return(true)));
+  Error error;
+  driver_->Connect(service_, &error);
+  EXPECT_TRUE(error.IsSuccess());
+
+  EXPECT_CALL(device_info_, OpenTunnelInterface(interface))
+      .WillOnce(Return(fd));
+  EXPECT_CALL(dispatcher_, CreateInputHandler(fd, _, _))
+      .WillOnce(Return(io_handler));
+  EXPECT_TRUE(driver_->ClaimInterface(interface, kInterfaceIndex));
+
+  driver_->reconnect_supported_ = true;
+
+  // Roam from one Online network to another -> kLinkChanged.
+  scoped_refptr<MockService> mock_service(
+      new NiceMock<MockService>(&control_, &dispatcher_, &metrics_, &manager_));
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kLinkChanged)));
+  EXPECT_CALL(*mock_service, state())
+      .WillOnce(Return(Service::kStateOnline));
+  driver_->OnDefaultServiceChanged(mock_service);
+
+  // Default physical service has no connection -> kLinkDown.
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kLinkDown)));
+  driver_->OnDefaultServiceChanged(nullptr);
+
+  // New default physical service not online yet -> no change.
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(_))
+      .Times(0);
+  EXPECT_CALL(*mock_service, state())
+      .WillOnce(Return(Service::kStateConnected));
+  driver_->OnDefaultServiceChanged(mock_service);
+
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(_))
+      .Times(0);
+  EXPECT_CALL(*mock_service, state())
+      .WillOnce(Return(Service::kStatePortal));
+  driver_->OnDefaultServiceStateChanged(mock_service);
+
+  // Default physical service comes Online -> kLinkUp.
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kLinkUp)));
+  EXPECT_CALL(*mock_service, state())
+      .WillOnce(Return(Service::kStateOnline));
+  driver_->OnDefaultServiceStateChanged(mock_service);
+
+  // Default physical service vanishes, but the app doesn't support
+  // reconnecting -> kDisconnected.
+  driver_->reconnect_supported_ = false;
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kDisconnected)));
+  driver_->OnDefaultServiceChanged(nullptr);
+
+  driver_->Disconnect();
+}
+
+TEST_F(ThirdPartyVpnDriverTest, PowerEvents) {
+  const std::string interface = kInterfaceName;
+  IOHandler* io_handler = new IOHandler();  // Owned by |driver_|
+  int fd = 1;
+
+  EXPECT_CALL(device_info_, CreateTunnelInterface(_))
+      .WillOnce(DoAll(SetArgumentPointee<0>(interface), Return(true)));
+  Error error;
+  driver_->Connect(service_, &error);
+  EXPECT_TRUE(error.IsSuccess());
+
+  EXPECT_CALL(device_info_, OpenTunnelInterface(interface))
+      .WillOnce(Return(fd));
+  EXPECT_CALL(dispatcher_, CreateInputHandler(fd, _, _))
+      .WillOnce(Return(io_handler));
+  EXPECT_TRUE(driver_->ClaimInterface(interface, kInterfaceIndex));
+
+  driver_->reconnect_supported_ = true;
+
+  ResultCallback callback =
+      base::Bind(&ThirdPartyVpnDriverTest::TestCallback,
+                 base::Unretained(this));
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kSuspend)));
+  EXPECT_CALL(*this, TestCallback(_));
+  driver_->OnBeforeSuspend(callback);
+
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kResume)));
+  driver_->OnAfterResume();
+
+  EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
+                                       ThirdPartyVpnDriver::kDisconnected)));
+  driver_->Disconnect();
+}
+
 TEST_F(ThirdPartyVpnDriverTest, SendPacket) {
   int fd = 1;
   std::string error;
diff --git a/vpn/vpn_driver.cc b/vpn/vpn_driver.cc
index e21c3d4d..dfbc5744 100644
--- a/vpn/vpn_driver.cc
+++ b/vpn/vpn_driver.cc
@@ -306,6 +306,9 @@ void VPNDriver::OnBeforeSuspend(const ResultCallback& callback) {
 void VPNDriver::OnAfterResume() {
 }
 
+void VPNDriver::OnDefaultServiceStateChanged(const ServiceRefPtr& service) {
+}
+
 string VPNDriver::GetHost() const {
   return args_.LookupString(kProviderHostProperty, "");
 }
diff --git a/vpn/vpn_driver.h b/vpn/vpn_driver.h
index 82cc8862..ea72b05e 100644
--- a/vpn/vpn_driver.h
+++ b/vpn/vpn_driver.h
@@ -62,6 +62,7 @@ class VPNDriver {
   // Power management events.
   virtual void OnBeforeSuspend(const ResultCallback& callback);
   virtual void OnAfterResume();
+  virtual void OnDefaultServiceStateChanged(const ServiceRefPtr& service);
 
   std::string GetHost() const;
 
diff --git a/vpn/vpn_service.cc b/vpn/vpn_service.cc
index 47d96290..52f0d7fe 100644
--- a/vpn/vpn_service.cc
+++ b/vpn/vpn_service.cc
@@ -267,4 +267,8 @@ void VPNService::OnAfterResume() {
   Service::OnAfterResume();
 }
 
+void VPNService::OnDefaultServiceStateChanged(const ServiceRefPtr& service) {
+  driver_->OnDefaultServiceStateChanged(service);
+}
+
 }  // namespace shill
diff --git a/vpn/vpn_service.h b/vpn/vpn_service.h
index a6ed7789..4db7ba18 100644
--- a/vpn/vpn_service.h
+++ b/vpn/vpn_service.h
@@ -54,6 +54,8 @@ class VPNService : public Service {
   // Power management events.
   virtual void OnBeforeSuspend(const ResultCallback& callback) override;
   virtual void OnAfterResume() override;
+  virtual void OnDefaultServiceStateChanged(
+      const ServiceRefPtr& service) override;
 
   virtual void InitDriverPropertyStore();