shill: VPNService: Report Tethering state
Services should store a copy of their tethering state in the
connection. VPNService should use this stored state to report
the underlying connection's tethering state in response to
queries of the VPNService's own Tethering property.
BUG=chromium:323010
TEST=Unit tests
Change-Id: Idf0a8a6265c9cf78af00da00d638cda37b708fcc
Reviewed-on: https://chromium-review.googlesource.com/178720
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/connection.h b/connection.h
index 2ad529e..8d0e51c 100644
--- a/connection.h
+++ b/connection.h
@@ -103,6 +103,8 @@
virtual const IPAddress &local() const { return local_; }
virtual const IPAddress &gateway() const { return gateway_; }
virtual Technology::Identifier technology() const { return technology_; }
+ virtual const std::string &tethering() const { return tethering_; }
+ void set_tethering(const std::string &tethering) { tethering_ = tethering; }
// Return the lowest connection on which this connection depends. In case of
// error, a NULL is returned.
@@ -167,6 +169,11 @@
IPAddress local_;
IPAddress gateway_;
+ // Track the tethering status of the Service associated with this connection.
+ // This property is set by a service as it takes ownership of a connection,
+ // and is read by services that are bound through this connection.
+ std::string tethering_;
+
// A binder to a lower Connection that this Connection depends on, if any.
Binder lower_binder_;
diff --git a/doc/service-api.txt b/doc/service-api.txt
index e63782b..7937872 100644
--- a/doc/service-api.txt
+++ b/doc/service-api.txt
@@ -1332,7 +1332,11 @@
This property is only visible in service types
which can support tethering. Currently only
- Ethernet and WiFi services support this property.
+ Ethernet and WiFi services support this property
+ directly. VPN services make this property visible
+ if the service they're using for connectivity does
+ (i.e., if VPN connectivity is gained via Ethernet
+ or WiFi).
string Type [readonly]
diff --git a/mock_connection.h b/mock_connection.h
index ffbd0e0..9e04b47 100644
--- a/mock_connection.h
+++ b/mock_connection.h
@@ -32,6 +32,7 @@
MOCK_CONST_METHOD0(technology, Technology::Identifier());
MOCK_METHOD0(CreateGatewayRoute, bool());
MOCK_METHOD0(GetCarrierConnection, ConnectionRefPtr());
+ MOCK_CONST_METHOD0(tethering, std::string &());
private:
DISALLOW_COPY_AND_ASSIGN(MockConnection);
diff --git a/service.cc b/service.cc
index 9e2fd18..c844e92 100644
--- a/service.cc
+++ b/service.cc
@@ -680,6 +680,8 @@
// http://crbug.com/216664
http_proxy_.reset(new HTTPProxy(connection));
http_proxy_->Start(dispatcher_, sockets_.get());
+ Error unused_error;
+ connection->set_tethering(GetTethering(&unused_error));
} else {
http_proxy_.reset();
static_ip_parameters_.ClearSavedParameters();
diff --git a/vpn_service.cc b/vpn_service.cc
index f97d7d1..e1541f1 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -174,6 +174,30 @@
return true;
}
+string VPNService::GetTethering(Error *error) const {
+ ConnectionRefPtr conn = connection();
+ if (conn)
+ conn = conn->GetCarrierConnection();
+
+ string tethering;
+ if (conn) {
+ tethering = conn->tethering();
+ if (!tethering.empty()) {
+ return tethering;
+ }
+ // The underlying service may not have a Tethering property. This is
+ // not strictly an error, so we don't print an error message. Populating
+ // an error here just serves to propagate the lack of a property in
+ // GetProperties().
+ error->Populate(Error::kNotSupported);
+ } else {
+ Error::PopulateAndLog(error,
+ Error::kOperationFailed,
+ Error::GetDefaultMessage(Error::kOperationFailed));
+ }
+ return "";
+}
+
bool VPNService::SetNameProperty(const string &name, Error *error) {
if (name == friendly_name()) {
return false;
diff --git a/vpn_service.h b/vpn_service.h
index e7a08de..13e8d9c 100644
--- a/vpn_service.h
+++ b/vpn_service.h
@@ -47,6 +47,7 @@
protected:
// Inherited from Service.
virtual bool IsAutoConnectable(const char **reason) const;
+ virtual std::string GetTethering(Error *error) const override;
private:
friend class VPNServiceTest;
@@ -54,6 +55,7 @@
FRIEND_TEST(VPNServiceTest, SetConnection);
FRIEND_TEST(VPNServiceTest, GetPhysicalTechologyPropertyFailsIfNoCarrier);
FRIEND_TEST(VPNServiceTest, GetPhysicalTechologyPropertyOverWifi);
+ FRIEND_TEST(VPNServiceTest, GetTethering);
static const char kAutoConnNeverConnected[];
static const char kAutoConnVPNAlreadyActive[];
diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index beec2fb..dfb98f5 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -28,6 +28,7 @@
using testing::NiceMock;
using testing::Return;
using testing::ReturnRef;
+using testing::ReturnRefOfCopy;
namespace shill {
@@ -391,4 +392,52 @@
EXPECT_CALL(device_info_, FlushAddresses(0));
}
+TEST_F(VPNServiceTest, GetTethering) {
+ scoped_refptr<Connection> null_connection;
+
+ EXPECT_CALL(*sockets_, Socket(_, _, _)).WillOnce(Return(-1));
+ service_->SetConnection(connection_);
+ EXPECT_EQ(connection_.get(), service_->connection().get());
+
+ // Simulate an error in the GetCarrierConnection() returning a NULL reference.
+ EXPECT_CALL(*connection_, GetCarrierConnection())
+ .WillOnce(Return(null_connection));
+
+ {
+ Error error;
+ EXPECT_EQ("", service_->GetTethering(&error));
+ EXPECT_EQ(Error::kOperationFailed, error.type());
+ }
+
+ scoped_refptr<NiceMock<MockConnection>> lower_connection_ =
+ new NiceMock<MockConnection>(&device_info_);
+
+ EXPECT_CALL(*connection_, tethering()).Times(0);
+ EXPECT_CALL(*connection_, GetCarrierConnection())
+ .WillRepeatedly(Return(lower_connection_));
+
+ const char kTethering[] = "moon unit";
+ EXPECT_CALL(*lower_connection_, tethering())
+ .WillOnce(ReturnRefOfCopy(string(kTethering)))
+ .WillOnce(ReturnRefOfCopy(string()));
+
+ {
+ Error error;
+ EXPECT_EQ(kTethering, service_->GetTethering(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ }
+ {
+ Error error;
+ EXPECT_EQ("", service_->GetTethering(&error));
+ EXPECT_EQ(Error::kNotSupported, error.type());
+ }
+
+ // Clear expectations now, so the Return(lower_connection_) action releases
+ // the reference to |lower_connection_| allowing it to be destroyed now.
+ Mock::VerifyAndClearExpectations(connection_);
+ // Destroying the |lower_connection_| at function exit will also call an extra
+ // FlushAddresses on the |device_info_| object.
+ EXPECT_CALL(device_info_, FlushAddresses(0));
+}
+
} // namespace shill
diff --git a/wifi_service.cc b/wifi_service.cc
index 18205ce..2a55600 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -264,7 +264,7 @@
// Only perform BSSID tests if there is exactly one matching endpoint,
// so we ignore campuses that may use locally administered BSSIDs.
- if (GetEndpointCount() == 1 &&
+ if (endpoints_.size() == 1 &&
(*endpoints_.begin())->has_tethering_signature()) {
return kTetheringSuspectedState;
}