commit 79349f031df30747ae7a1271f80724036ba3e961
author Darin Petkov <petkov@chromium.org> Thu Jan 24 16:18:26 2013 +0100
committer ChromeBot <chrome-bot@google.com> Fri Jan 25 08:43:19 2013 -0800
tree 1b0037a610c2cfef5171dd287277df4d0791ade8
parent 9abd6749e69712e9ec4ed4a54fbba243f16ae7f3

shill: vpn: Don't auto-connect to a VPN service that has never connected.

This improves the chances that the VPN service is connectable.

BUG=chromium-os:38193
TEST=unit test

Change-Id: I5d505604aaa3fd30cb89c2a87de398544d5a1c27
Reviewed-on: https://gerrit.chromium.org/gerrit/41927
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Philipp Neubeck <pneubeck@chromium.org>
Tested-by: Philipp Neubeck <pneubeck@chromium.org>
Commit-Queue: Darin Petkov <petkov@chromium.org>

vpn_service.cc

diff --git a/vpn_service.cc b/vpn_service.cc
index 39abd94..a7991e3 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -23,6 +23,8 @@
 
 namespace shill {
 
+const char VPNService::kAutoConnNeverConnected[] = "never connected";
+
 VPNService::VPNService(ControlInterface *control,
                        EventDispatcher *dispatcher,
                        Metrics *metrics,
@@ -141,4 +143,17 @@
   Service::SetConnection(connection);
 }
 
+bool VPNService::IsAutoConnectable(const char **reason) const {
+  if (!Service::IsAutoConnectable(reason)) {
+    return false;
+  }
+  // Don't auto-connect VPN services that have never connected. This improves
+  // the chances that the VPN service is connectable and avoids dialog popups.
+  if (!has_ever_connected()) {
+    *reason = kAutoConnNeverConnected;
+    return false;
+  }
+  return true;
+}
+
 }  // namespace shill

vpn_service.h

diff --git a/vpn_service.h b/vpn_service.h
index 1162e57..4a6bc9b 100644
--- a/vpn_service.h
+++ b/vpn_service.h
@@ -43,10 +43,17 @@
                                              Error *error);
   void set_storage_id(const std::string &id) { storage_id_ = id; }
 
+ protected:
+  // Inherited from Service.
+  virtual bool IsAutoConnectable(const char **reason) const;
+
  private:
   FRIEND_TEST(VPNServiceTest, GetDeviceRpcId);
+  FRIEND_TEST(VPNServiceTest, IsAutoConnectable);
   FRIEND_TEST(VPNServiceTest, SetConnection);
 
+  static const char kAutoConnNeverConnected[];
+
   virtual std::string GetDeviceRpcId(Error *error);
 
   std::string storage_id_;

vpn_service_unittest.cc

diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index 645f67d..a7076d6 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -59,6 +59,14 @@
     service_->state_ = state;
   }
 
+  void SetHasEverConnected(bool connected) {
+    service_->has_ever_connected_ = connected;
+  }
+
+  void SetConnectable(bool connectable) {
+    service_->connectable_ = connectable;
+  }
+
   std::string interface_name_;
   std::string ipconfig_rpc_identifier_;
   MockVPNDriver *driver_;  // Owned by |service_|.
@@ -211,4 +219,21 @@
   connection_->OnLowerDisconnect();
 }
 
+TEST_F(VPNServiceTest, IsAutoConnectable) {
+  EXPECT_TRUE(service_->connectable());
+  EXPECT_FALSE(service_->has_ever_connected());
+
+  const char *reason = NULL;
+  EXPECT_FALSE(service_->IsAutoConnectable(&reason));
+  EXPECT_STREQ(VPNService::kAutoConnNeverConnected, reason);
+
+  SetHasEverConnected(true);
+  reason = NULL;
+  EXPECT_TRUE(service_->IsAutoConnectable(&reason));
+  EXPECT_FALSE(reason);
+
+  SetConnectable(false);
+  EXPECT_FALSE(service_->IsAutoConnectable(&reason));
+}
+
 }  // namespace shill