shill: WiFiProvider: Track Endpoint -> Service mapping
Create a map for tracking the relationship between endpoints and
their associated service. This not only allows a faster lookup,
but it also allows us to flag situations where an endpoint has
changed in a way that now violates this relationship. In these
situations, we must now find a new service to map the endpoint to.
BUG=chromium:206299
TEST=Unit tests, autotest (network_WiFiSecMat/120ChangeBSSSecurity)
Change-Id: I7cda9d08c807acb878f7fa98c8cdaa255ba3c7f6
Reviewed-on: https://gerrit.chromium.org/gerrit/46530
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/mock_wifi_provider.h b/mock_wifi_provider.h
index c9457a8..01edcea 100644
--- a/mock_wifi_provider.h
+++ b/mock_wifi_provider.h
@@ -32,6 +32,8 @@
MOCK_METHOD1(OnEndpointAdded, void(const WiFiEndpointConstRefPtr &endpoint));
MOCK_METHOD1(OnEndpointRemoved,
WiFiServiceRefPtr(const WiFiEndpointConstRefPtr &endpoint));
+ MOCK_METHOD1(OnEndpointUpdated,
+ void(const WiFiEndpointConstRefPtr &endpoint));
MOCK_METHOD1(OnServiceUnloaded, bool(const WiFiServiceRefPtr &service));
MOCK_METHOD0(GetHiddenSSIDList, ByteArrays());
MOCK_METHOD2(FixupServiceEntries, void(StoreInterface *storage,
diff --git a/wifi.cc b/wifi.cc
index bc523b2..2136fee 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -434,11 +434,7 @@
}
void WiFi::NotifyEndpointChanged(const WiFiEndpointConstRefPtr &endpoint) {
- WiFiService *service = provider_->FindServiceForEndpoint(endpoint);
- DCHECK(service);
- if (service) {
- service->NotifyEndpointUpdated(endpoint);
- }
+ provider_->OnEndpointUpdated(endpoint);
}
void WiFi::AppendBgscan(WiFiService *service,
diff --git a/wifi_provider.cc b/wifi_provider.cc
index 787a04a..9192747 100644
--- a/wifi_provider.cc
+++ b/wifi_provider.cc
@@ -68,6 +68,7 @@
<< service->unique_name();
manager_->DeregisterService(service);
}
+ service_by_endpoint_.clear();
running_ = false;
}
@@ -205,9 +206,11 @@
WiFiServiceRefPtr WiFiProvider::FindServiceForEndpoint(
const WiFiEndpointConstRefPtr &endpoint) {
- return FindService(endpoint->ssid(),
- endpoint->network_mode(),
- endpoint->security_mode());
+ EndpointServiceMap::iterator service_it =
+ service_by_endpoint_.find(endpoint);
+ if (service_it == service_by_endpoint_.end())
+ return NULL;
+ return service_it->second;
}
void WiFiProvider::OnEndpointAdded(const WiFiEndpointConstRefPtr &endpoint) {
@@ -215,7 +218,9 @@
return;
}
- WiFiServiceRefPtr service = FindServiceForEndpoint(endpoint);
+ WiFiServiceRefPtr service = FindService(endpoint->ssid(),
+ endpoint->network_mode(),
+ endpoint->security_mode());
if (!service) {
const bool hidden_ssid = false;
service = AddService(
@@ -226,6 +231,7 @@
}
service->AddEndpoint(endpoint);
+ service_by_endpoint_[endpoint] = service;
SLOG(WiFi, 1) << "Assigned endpoint " << endpoint->bssid_string()
<< " to service " << service->unique_name() << ".";
@@ -246,6 +252,7 @@
SLOG(WiFi, 1) << "Removing endpoint " << endpoint->bssid_string()
<< " from Service " << service->unique_name();
service->RemoveEndpoint(endpoint);
+ service_by_endpoint_.erase(endpoint);
if (service->HasEndpoints() || service->IsRemembered()) {
// Keep services around if they are in a profile or have remaining
@@ -260,6 +267,26 @@
return service;
}
+void WiFiProvider::OnEndpointUpdated(const WiFiEndpointConstRefPtr &endpoint) {
+ WiFiService *service = FindServiceForEndpoint(endpoint);
+ CHECK(service);
+
+ // If the service still matches the endpoint in its new configuration,
+ // we need only to update the service.
+ if (service->ssid() == endpoint->ssid() &&
+ service->mode() == endpoint->network_mode() &&
+ service->IsSecurityMatch(endpoint->security_mode())) {
+ service->NotifyEndpointUpdated(endpoint);
+ return;
+ }
+
+ // The endpoint no longer matches the associated service. Remove the
+ // endpoint, so current references to the endpoint are reset, then add
+ // it again so it can be associated with a new service.
+ OnEndpointRemoved(endpoint);
+ OnEndpointAdded(endpoint);
+}
+
bool WiFiProvider::OnServiceUnloaded(const WiFiServiceRefPtr &service) {
// If the service still has endpoints, it should remain in the service list.
if (service->HasEndpoints()) {
diff --git a/wifi_provider.h b/wifi_provider.h
index 2fa38c0..0c4ffd0 100644
--- a/wifi_provider.h
+++ b/wifi_provider.h
@@ -66,6 +66,13 @@
virtual WiFiServiceRefPtr OnEndpointRemoved(
const WiFiEndpointConstRefPtr &endpoint);
+ // Called by a Device when it receives notification that an Endpoint
+ // has changed. Ensure the updated endpoint still matches its
+ // associated service. If necessary re-assign the endpoint to a new
+ // service, otherwise notify the associated service of the update to
+ // the endpoint.
+ virtual void OnEndpointUpdated(const WiFiEndpointConstRefPtr &endpoint);
+
// Called by a WiFiService when it is unloaded and no longer visible.
virtual bool OnServiceUnloaded(const WiFiServiceRefPtr &service);
@@ -80,6 +87,8 @@
private:
friend class WiFiProviderTest;
+ typedef std::map<const WiFiEndpoint *, WiFiServiceRefPtr> EndpointServiceMap;
+
static const char kManagerErrorSSIDTooLong[];
static const char kManagerErrorSSIDTooShort[];
static const char kManagerErrorSSIDRequired[];
@@ -120,6 +129,8 @@
Manager *manager_;
std::vector<WiFiServiceRefPtr> services_;
+ EndpointServiceMap service_by_endpoint_;
+
bool running_;
DISALLOW_COPY_AND_ASSIGN(WiFiProvider);
diff --git a/wifi_provider_unittest.cc b/wifi_provider_unittest.cc
index 6494e80..8cdf497 100644
--- a/wifi_provider_unittest.cc
+++ b/wifi_provider_unittest.cc
@@ -70,6 +70,10 @@
return provider_.services_;
}
+ const WiFiProvider::EndpointServiceMap &GetServiceByEndpoint() {
+ return provider_.service_by_endpoint_;
+ }
+
bool GetRunning() {
return provider_.running_;
}
@@ -187,6 +191,10 @@
provider_.services_.push_back(service);
return service;
}
+ void AddEndpointToService(WiFiServiceRefPtr service,
+ const WiFiEndpointConstRefPtr &endpoint) {
+ provider_.service_by_endpoint_[endpoint] = service;
+ }
NiceMockControl control_;
MockEventDispatcher dispatcher_;
MockMetrics metrics_;
@@ -217,6 +225,7 @@
provider_.Start();
EXPECT_TRUE(GetServices().empty());
EXPECT_TRUE(GetRunning());
+ EXPECT_TRUE(GetServiceByEndpoint().empty());
}
TEST_F(WiFiProviderTest, Stop) {
@@ -228,7 +237,11 @@
flimflam::kModeManaged,
flimflam::kSecurityNone,
false);
+ WiFiEndpointRefPtr endpoint = MakeEndpoint("", "00:00:00:00:00:00", 0, 0);
+ AddEndpointToService(service0, endpoint);
+
EXPECT_EQ(2, GetServices().size());
+ EXPECT_FALSE(GetServiceByEndpoint().empty());
EXPECT_CALL(*service0, ResetWiFi()).Times(1);
EXPECT_CALL(*service1, ResetWiFi()).Times(1);
EXPECT_CALL(manager_, DeregisterService(RefPtrMatch(service0))).Times(1);
@@ -240,6 +253,7 @@
Mock::VerifyAndClearExpectations(service1);
Mock::VerifyAndClearExpectations(&manager_);
EXPECT_TRUE(GetServices().empty());
+ EXPECT_TRUE(GetServiceByEndpoint().empty());
}
TEST_F(WiFiProviderTest, CreateServicesFromProfileWithNoGroups) {
@@ -648,7 +662,10 @@
WiFiEndpointRefPtr endpoint = MakeEndpoint(kSSID, "00:00:00:00:00:00", 0, 0);
WiFiServiceRefPtr endpoint_service =
provider_.FindServiceForEndpoint(endpoint);
- EXPECT_EQ(service.get(), endpoint_service.get());
+ // Just because a matching service exists, we shouldn't necessarily have
+ // it returned. We will test that this function returns the correct
+ // service if the endpoint is added below.
+ EXPECT_EQ(NULL, endpoint_service.get());
}
TEST_F(WiFiProviderTest, OnEndpointAdded) {
@@ -667,6 +684,10 @@
flimflam::kSecurityNone));
EXPECT_TRUE(service0);
EXPECT_TRUE(service0->HasEndpoints());
+ EXPECT_EQ(1, GetServiceByEndpoint().size());
+ WiFiServiceRefPtr endpoint_service =
+ provider_.FindServiceForEndpoint(endpoint0);
+ EXPECT_EQ(service0.get(), endpoint_service.get());
WiFiEndpointRefPtr endpoint1 = MakeEndpoint(ssid0, "00:00:00:00:00:01", 0, 0);
EXPECT_CALL(manager_, RegisterService(_)).Times(0);
@@ -818,6 +839,9 @@
// Remove the last endpoint of a non-remembered service.
WiFiEndpointRefPtr endpoint0 = MakeEndpoint(ssid0, "00:00:00:00:00:00", 0, 0);
+ AddEndpointToService(service0, endpoint0);
+ EXPECT_EQ(1, GetServiceByEndpoint().size());
+
EXPECT_CALL(*service0, RemoveEndpoint(RefPtrMatch(endpoint0))).Times(1);
EXPECT_CALL(*service1, RemoveEndpoint(_)).Times(0);
EXPECT_CALL(*service0, HasEndpoints()).WillOnce(Return(false));
@@ -833,6 +857,7 @@
Mock::VerifyAndClearExpectations(service1);
EXPECT_EQ(1, GetServices().size());
EXPECT_EQ(service1.get(), GetServices().front().get());
+ EXPECT_TRUE(GetServiceByEndpoint().empty());
}
TEST_F(WiFiProviderTest, OnEndpointRemovedButHasEndpoints) {
@@ -847,6 +872,9 @@
// Remove an endpoint of a non-remembered service.
WiFiEndpointRefPtr endpoint0 = MakeEndpoint(ssid0, "00:00:00:00:00:00", 0, 0);
+ AddEndpointToService(service0, endpoint0);
+ EXPECT_EQ(1, GetServiceByEndpoint().size());
+
EXPECT_CALL(*service0, RemoveEndpoint(RefPtrMatch(endpoint0))).Times(1);
EXPECT_CALL(*service0, HasEndpoints()).WillOnce(Return(true));
EXPECT_CALL(*service0, IsRemembered()).WillRepeatedly(Return(false));
@@ -859,6 +887,7 @@
Mock::VerifyAndClearExpectations(&manager_);
Mock::VerifyAndClearExpectations(service0);
EXPECT_EQ(1, GetServices().size());
+ EXPECT_TRUE(GetServiceByEndpoint().empty());
}
TEST_F(WiFiProviderTest, OnEndpointRemovedButIsRemembered) {
@@ -873,6 +902,9 @@
// Remove the last endpoint of a remembered service.
WiFiEndpointRefPtr endpoint0 = MakeEndpoint(ssid0, "00:00:00:00:00:00", 0, 0);
+ AddEndpointToService(service0, endpoint0);
+ EXPECT_EQ(1, GetServiceByEndpoint().size());
+
EXPECT_CALL(*service0, RemoveEndpoint(RefPtrMatch(endpoint0))).Times(1);
EXPECT_CALL(*service0, HasEndpoints()).WillRepeatedly(Return(false));
EXPECT_CALL(*service0, IsRemembered()).WillOnce(Return(true));
@@ -885,6 +917,7 @@
Mock::VerifyAndClearExpectations(&manager_);
Mock::VerifyAndClearExpectations(service0);
EXPECT_EQ(1, GetServices().size());
+ EXPECT_TRUE(GetServiceByEndpoint().empty());
}
TEST_F(WiFiProviderTest, OnEndpointRemovedWhileStopped) {
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index c8b33b1..7413d6a 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -544,6 +544,9 @@
void ClearCachedCredentials(const WiFiService *service) {
return wifi_->ClearCachedCredentials(service);
}
+ void NotifyEndpointChanged(const WiFiEndpointConstRefPtr &endpoint) {
+ wifi_->NotifyEndpointChanged(endpoint);
+ }
bool RemoveNetwork(const ::DBus::Path &network) {
return wifi_->RemoveNetwork(network);
}
@@ -882,6 +885,13 @@
ClearCachedCredentials(service);
}
+TEST_F(WiFiMainTest, NotifyEndpointChanged) {
+ WiFiEndpointRefPtr endpoint =
+ MakeEndpointWithMode("ssid", "00:00:00:00:00:00", kNetworkModeAdHoc);
+ EXPECT_CALL(*wifi_provider(), OnEndpointUpdated(EndpointMatch(endpoint)));
+ NotifyEndpointChanged(endpoint);
+}
+
TEST_F(WiFiMainTest, RemoveNetwork) {
DBus::Path network = "/test/path";
StartWiFi();