commit 76d1388a4a0435f1586117b55674ad61f5a8e6c6
author mukesh agrawal <quiche@chromium.org> Thu Jan 12 15:23:11 2012 -0800
committer Gerrit <chrome-bot@google.com> Thu Jan 12 17:13:40 2012 -0800
tree e4252e60eaf3c7fa49f48425820f2438b42e82c1
parent 592516dae4f2e37f8a40cf54333f18eb440721fe

shill: have Service check if a service is connected/connecting
before executing an AutoConnect request.

This avoids a spurious warning in the log file, if manager
autoconnects to an already connected Cellular service.

No effect on Ethernet services, because Ethernet's connect is
a no-op. No effect on WiFi services, because they override
Service::AutoConnect.

BUG=None
TEST=new unit test

Change-Id: If9e6c18735a5fb8a0d9baa1602c392ce7ec6e932
Reviewed-on: https://gerrit.chromium.org/gerrit/14097
Reviewed-by: Thieu Le <thieule@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Ready: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>

service.cc

diff --git a/service.cc b/service.cc
index 37fb889..ea7a9cd 100644
--- a/service.cc
+++ b/service.cc
@@ -178,6 +178,8 @@
   if (this->IsAutoConnectable()) {
     Error error;
     Connect(&error);
+  } else {
+    LOG(INFO) << "Suppressed autoconnect to " << friendly_name();
   }
 }
 
@@ -497,6 +499,10 @@
   }
 }
 
+bool Service::IsAutoConnectable() const {
+  return connectable() && !IsConnected() && !IsConnecting();
+}
+
 void Service::HelpRegisterDerivedBool(
     const string &name,
     bool(Service::*get)(Error *),

service.h

diff --git a/service.h b/service.h
index 6404f32..a2a6eba 100644
--- a/service.h
+++ b/service.h
@@ -234,7 +234,7 @@
   // This should include any tests used for computing connectable(),
   // as well as other critera such as whether the device associated with
   // this service is busy with another connection.
-  virtual bool IsAutoConnectable() const { return connectable(); }
+  virtual bool IsAutoConnectable() const;
 
   void HelpRegisterDerivedBool(
       const std::string &name,
@@ -277,6 +277,7 @@
   FRIEND_TEST(DeviceTest, SelectedService);
   FRIEND_TEST(ManagerTest, SortServicesWithConnection);
   FRIEND_TEST(ServiceTest, Constructor);
+  FRIEND_TEST(ServiceTest, IsAutoConnectable);
   FRIEND_TEST(ServiceTest, Save);
   FRIEND_TEST(ServiceTest, SaveString);
   FRIEND_TEST(ServiceTest, SaveStringCrypted);

service_unittest.cc

diff --git a/service_unittest.cc b/service_unittest.cc
index 4f73bb8..634113d 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -276,4 +276,15 @@
   EXPECT_FALSE(service_->auto_connect());
 }
 
+TEST_F(ServiceTest, IsAutoConnectable) {
+  service_->set_connectable(true);
+  EXPECT_TRUE(service_->IsAutoConnectable());
+
+  service_->SetState(Service::kStateConnected);
+  EXPECT_FALSE(service_->IsAutoConnectable());
+
+  service_->SetState(Service::kStateAssociating);
+  EXPECT_FALSE(service_->IsAutoConnectable());
+}
+
 }  // namespace shill

wifi_service.cc

diff --git a/wifi_service.cc b/wifi_service.cc
index a676fa2..2100bc4 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -152,8 +152,8 @@
       // (Needed because hidden Services may remain registered with
       // Manager even without visible Endpoints.)
       && HasEndpoints()
-      // Do not preempt other connections (whether pending, or
-      // connected).
+      // Do not preempt an existing connection (whether pending, or
+      // connected, and whether to this service, or another).
       && wifi_->IsIdle();
 }