Move GetWifiAccessPoints() to cros_network_functions.cc

Move GetWifiAccessPoints() from network_library_impl_cros.cc to cros_network_functions.cc
Move struct WifiAccessPoint from network_library.h to cros_network_functions.h
Remove CrosFreeDeviceNetworkList

BUG=chromium-os:16557
TEST=unit_tests --gtest_filter="CrosNetworkFunctions.CrosGetWifiAccessPoints"

Review URL: https://chromiumcodereview.appspot.com/10160005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133371 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 104 insertions, 53 deletions

diff --git a/chrome/browser/chromeos/cros/cros_network_functions.cc b/chrome/browser/chromeos/cros/cros_network_functions.cc
index fd59e25..1a43a3e 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions.cc
+++ b/chrome/browser/chromeos/cros/cros_network_functions.cc
@@ -222,6 +222,12 @@ void OnConfigureService(void* object,
   }
 }
 
+// Safe string constructor since we can't rely on non NULL pointers
+// for string values from libcros.
+std::string SafeString(const char* s) {
+  return s ? std::string(s) : std::string();
+}
+
 // A bool to remember whether we are using Libcros network functions or not.
 bool g_libcros_network_functions_enabled = true;
 
@@ -631,12 +637,27 @@ void CrosFreeIPConfigStatus(IPConfigStatus* status) {
   chromeos::FreeIPConfigStatus(status);
 }
 
-DeviceNetworkList* CrosGetDeviceNetworkList() {
-  return chromeos::GetDeviceNetworkList();
-}
-
-void CrosFreeDeviceNetworkList(DeviceNetworkList* network_list) {
+bool CrosGetWifiAccessPoints(WifiAccessPointVector* result) {
+  DeviceNetworkList* network_list = chromeos::GetDeviceNetworkList();
+  if (network_list == NULL)
+    return false;
+  result->clear();
+  result->reserve(network_list->network_size);
+  const base::Time now = base::Time::Now();
+  for (size_t i = 0; i < network_list->network_size; ++i) {
+    DCHECK(network_list->networks[i].address);
+    DCHECK(network_list->networks[i].name);
+    WifiAccessPoint ap;
+    ap.mac_address = SafeString(network_list->networks[i].address);
+    ap.name = SafeString(network_list->networks[i].name);
+    ap.timestamp = now -
+        base::TimeDelta::FromSeconds(network_list->networks[i].age_seconds);
+    ap.signal_strength = network_list->networks[i].strength;
+    ap.channel = network_list->networks[i].channel;
+    result->push_back(ap);
+  }
   chromeos::FreeDeviceNetworkList(network_list);
+  return true;
 }
 
 void CrosConfigureService(const base::DictionaryValue& properties) {
diff --git a/chrome/browser/chromeos/cros/cros_network_functions.h b/chrome/browser/chromeos/cros/cros_network_functions.h
index 20c4cab..ab10387 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions.h
+++ b/chrome/browser/chromeos/cros/cros_network_functions.h
@@ -10,6 +10,8 @@
 // All calls to functions in chromeos_network.h should be made through
 // functions provided by this header.
 
+#include <vector>
+
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "third_party/cros/chromeos_network.h"
@@ -43,6 +45,19 @@ class CrosNetworkWatcher {
   CrosNetworkWatcher() {}
 };
 
+struct WifiAccessPoint {
+  WifiAccessPoint();
+
+  std::string mac_address;  // The mac address of the WiFi node.
+  std::string name;         // The SSID of the WiFi node.
+  base::Time timestamp;     // Timestamp when this AP was detected.
+  int signal_strength;      // Radio signal strength measured in dBm.
+  int signal_to_noise;      // Current signal to noise ratio measured in dB.
+  int channel;              // Wifi channel number.
+};
+
+typedef std::vector<WifiAccessPoint> WifiAccessPointVector;
+
 // Enables/Disables Libcros network functions.
 void SetLibcrosNetworkFunctionsEnabled(bool enabled);
 
@@ -234,16 +249,13 @@ bool CrosRemoveIPConfig(IPConfig* config);
 // Frees out a full status
 void CrosFreeIPConfigStatus(IPConfigStatus* status);
 
-// Retrieves the list of visible network objects. This structure is not cached
-// within the DLL, and is fetched via d-bus on each call.
-// Ownership of the DeviceNetworkList is returned to the caller; use
-// FreeDeviceNetworkList to release it.
-DeviceNetworkList* CrosGetDeviceNetworkList();
-
-// Deletes a DeviceNetworkList type that was allocated in the ChromeOS dll. We
-// need to do this to safely pass data over the dll boundary between our .so
-// and Chrome.
-void CrosFreeDeviceNetworkList(DeviceNetworkList* network_list);
+// Reads out the results of the last wifi scan. These results are not
+// pre-cached in the library, so the call may block whilst the results are
+// read over IPC.
+// Returns false if an error occurred in reading the results. Note that
+// a true return code only indicates the result set was successfully read,
+// it does not imply a scan has successfully completed yet.
+bool CrosGetWifiAccessPoints(WifiAccessPointVector* result);
 
 // Configures the network service specified by |properties|.
 void CrosConfigureService(const base::DictionaryValue& properties);
diff --git a/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc b/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
index 362fee7..30f05c1 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
+++ b/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
@@ -611,6 +611,45 @@ TEST_F(CrosNetworkFunctionsLibcrosTest, CrosRemoveIPConfig) {
   CrosRemoveIPConfig(&config);
 }
 
+TEST_F(CrosNetworkFunctionsLibcrosTest, CrosGetWifiAccessPoints) {
+  // Preapare data.
+  std::vector<DeviceNetworkInfo> networks(1);
+  networks[0].device_path = "/device/path";
+  networks[0].network_path = "/network/path";
+  networks[0].address = "address";
+  networks[0].name = "name";
+  networks[0].strength = 42;
+  networks[0].channel = 24;
+  networks[0].connected = true;
+  networks[0].age_seconds = 12345;
+
+  DeviceNetworkList network_list;
+  network_list.network_size = networks.size();
+  network_list.networks = &networks[0];
+
+  const base::Time expected_timestamp =
+      base::Time::Now() - base::TimeDelta::FromSeconds(networks[0].age_seconds);
+  const base::TimeDelta acceptable_timestamp_range =
+      base::TimeDelta::FromSeconds(1);
+
+  // Set expectations.
+  EXPECT_CALL(*MockChromeOSNetwork::Get(),
+              GetDeviceNetworkList()).WillOnce(Return(&network_list));
+  EXPECT_CALL(*MockChromeOSNetwork::Get(),
+              FreeDeviceNetworkList(&network_list)).Times(1);
+
+  // Call function.
+  WifiAccessPointVector aps;
+  ASSERT_TRUE(CrosGetWifiAccessPoints(&aps));
+  ASSERT_EQ(1U, aps.size());
+  EXPECT_EQ(networks[0].address, aps[0].mac_address);
+  EXPECT_EQ(networks[0].name, aps[0].name);
+  EXPECT_LE(expected_timestamp - acceptable_timestamp_range, aps[0].timestamp);
+  EXPECT_GE(expected_timestamp + acceptable_timestamp_range, aps[0].timestamp);
+  EXPECT_EQ(networks[0].strength, aps[0].signal_strength);
+  EXPECT_EQ(networks[0].channel, aps[0].channel);
+}
+
 TEST_F(CrosNetworkFunctionsLibcrosTest, CrosConfigureService) {
   EXPECT_CALL(*MockChromeOSNetwork::Get(),
               ConfigureService(_, _, _, _))
diff --git a/chrome/browser/chromeos/cros/mock_chromeos_network.cc b/chrome/browser/chromeos/cros/mock_chromeos_network.cc
index 0a01b33..2062e0c 100644
--- a/chrome/browser/chromeos/cros/mock_chromeos_network.cc
+++ b/chrome/browser/chromeos/cros/mock_chromeos_network.cc
@@ -203,6 +203,16 @@ bool CallMockRemoveIPConfig(IPConfig* config) {
   return g_mock_chromeos_network->RemoveIPConfig(config);
 }
 
+// Calls mock GetDeviceNetworkList.
+DeviceNetworkList* CallMockGetDeviceNetworkList() {
+  return g_mock_chromeos_network->GetDeviceNetworkList();
+}
+
+// Calls mock FreeDeviceNetworkList.
+void CallMockFreeDeviceNetworkList(DeviceNetworkList* network_list) {
+  g_mock_chromeos_network->FreeDeviceNetworkList(network_list);
+}
+
 // Calls mock ConfigureService.
 void CallMockConfigureService(const char* identifier,
                               const GHashTable* properties,
@@ -274,6 +284,8 @@ void MockChromeOSNetwork::Initialize() {
     chromeos::RequestNetworkDeviceEnable = &CallMockRequestNetworkDeviceEnable;
     chromeos::AddIPConfig = &CallMockAddIPConfig;
     chromeos::RemoveIPConfig = &CallMockRemoveIPConfig;
+    chromeos::GetDeviceNetworkList = &CallMockGetDeviceNetworkList;
+    chromeos::FreeDeviceNetworkList = &CallMockFreeDeviceNetworkList;
     chromeos::ConfigureService = &CallMockConfigureService;
   } else {
     LOG(ERROR) << "CrosLibrary is initialized.";
@@ -310,6 +322,8 @@ void MockChromeOSNetwork::Shutdown() {
     chromeos::RequestNetworkDeviceEnable = NULL;
     chromeos::AddIPConfig = NULL;
     chromeos::RemoveIPConfig = NULL;
+    chromeos::GetDeviceNetworkList = NULL;
+    chromeos::FreeDeviceNetworkList = NULL;
     chromeos::ConfigureService = NULL;
   } else {
     LOG(ERROR) << "CrosLibrary is initialized.";
diff --git a/chrome/browser/chromeos/cros/mock_chromeos_network.h b/chrome/browser/chromeos/cros/mock_chromeos_network.h
index 8d6d549..37dab1e 100644
--- a/chrome/browser/chromeos/cros/mock_chromeos_network.h
+++ b/chrome/browser/chromeos/cros/mock_chromeos_network.h
@@ -100,6 +100,8 @@ class MockChromeOSNetwork {
                                                 bool enable));
   MOCK_METHOD2(AddIPConfig, bool(const char* device_path, IPConfigType type));
   MOCK_METHOD1(RemoveIPConfig, bool(IPConfig* config));
+  MOCK_METHOD0(GetDeviceNetworkList, DeviceNetworkList*());
+  MOCK_METHOD1(FreeDeviceNetworkList, void(DeviceNetworkList* network_list));
   MOCK_METHOD4(ConfigureService,
                void(const char* identifier,
                     const GHashTable* properties,
diff --git a/chrome/browser/chromeos/cros/network_library.h b/chrome/browser/chromeos/cros/network_library.h
index 1980f05..ce4a36b 100644
--- a/chrome/browser/chromeos/cros/network_library.h
+++ b/chrome/browser/chromeos/cros/network_library.h
@@ -1180,19 +1180,7 @@ struct CellTower {
                          // Each unit is roughly 550 meters.
 };
 
-struct WifiAccessPoint {
-  WifiAccessPoint();
-
-  std::string mac_address;  // The mac address of the WiFi node.
-  std::string name;         // The SSID of the WiFi node.
-  base::Time timestamp;     // Timestamp when this AP was detected.
-  int signal_strength;      // Radio signal strength measured in dBm.
-  int signal_to_noise;      // Current signal to noise ratio measured in dB.
-  int channel;              // Wifi channel number.
-};
-
 typedef std::vector<CellTower> CellTowerVector;
-typedef std::vector<WifiAccessPoint> WifiAccessPointVector;
 
 // IP Configuration.
 struct NetworkIPConfig {
diff --git a/chrome/browser/chromeos/cros/network_library_impl_cros.cc b/chrome/browser/chromeos/cros/network_library_impl_cros.cc
index 6074a34..5062297 100644
--- a/chrome/browser/chromeos/cros/network_library_impl_cros.cc
+++ b/chrome/browser/chromeos/cros/network_library_impl_cros.cc
@@ -31,12 +31,6 @@ const char* kAlwaysInRoamingOperators[] = {
 // List of interfaces that have portal check enabled by default.
 const char kDefaultCheckPortalList[] = "ethernet,wifi,cellular";
 
-// Safe string constructor since we can't rely on non NULL pointers
-// for string values from libcros.
-std::string SafeString(const char* s) {
-  return s ? std::string(s) : std::string();
-}
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////
@@ -488,26 +482,7 @@ void NetworkLibraryImplCros::RequestNetworkScan() {
 bool NetworkLibraryImplCros::GetWifiAccessPoints(
     WifiAccessPointVector* result) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DeviceNetworkList* network_list = CrosGetDeviceNetworkList();
-  if (network_list == NULL)
-    return false;
-  result->clear();
-  result->reserve(network_list->network_size);
-  const base::Time now = base::Time::Now();
-  for (size_t i = 0; i < network_list->network_size; ++i) {
-    DCHECK(network_list->networks[i].address);
-    DCHECK(network_list->networks[i].name);
-    WifiAccessPoint ap;
-    ap.mac_address = SafeString(network_list->networks[i].address);
-    ap.name = SafeString(network_list->networks[i].name);
-    ap.timestamp = now -
-        base::TimeDelta::FromSeconds(network_list->networks[i].age_seconds);
-    ap.signal_strength = network_list->networks[i].strength;
-    ap.channel = network_list->networks[i].channel;
-    result->push_back(ap);
-  }
-  CrosFreeDeviceNetworkList(network_list);
-  return true;
+  return CrosGetWifiAccessPoints(result);
 }
 
 void NetworkLibraryImplCros::DisconnectFromNetwork(const Network* network) {