Reimplement CrosGetWifiAccessPoints without Libcros

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


Review URL: http://codereview.chromium.org/10202009

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

6 files changed, 222 insertions, 20 deletions

diff --git a/chrome/browser/chromeos/cros/cros_network_functions.cc b/chrome/browser/chromeos/cros/cros_network_functions.cc
index e0fdd81..20a696e 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions.cc
+++ b/chrome/browser/chromeos/cros/cros_network_functions.cc
@@ -13,6 +13,7 @@
 #include "chromeos/dbus/flimflam_device_client.h"
 #include "chromeos/dbus/flimflam_ipconfig_client.h"
 #include "chromeos/dbus/flimflam_manager_client.h"
+#include "chromeos/dbus/flimflam_network_client.h"
 #include "chromeos/dbus/flimflam_profile_client.h"
 #include "chromeos/dbus/flimflam_service_client.h"
 #include "dbus/object_path.h"
@@ -775,26 +776,112 @@ void CrosFreeIPConfigStatus(IPConfigStatus* status) {
 }
 
 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;
+  if (g_libcros_network_functions_enabled) {
+    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;
+  } else {
+    scoped_ptr<base::DictionaryValue> manager_properties(
+        DBusThreadManager::Get()->GetFlimflamManagerClient()->
+        CallGetPropertiesAndBlock());
+    if (!manager_properties.get()) {
+      LOG(WARNING) << "Couldn't read managers's properties";
+      return false;
+    }
+
+    base::ListValue* devices = NULL;
+    if (!manager_properties->GetListWithoutPathExpansion(
+            flimflam::kDevicesProperty, &devices)) {
+      LOG(WARNING) << flimflam::kDevicesProperty << " property not found";
+      return false;
+    }
+    const base::Time now = base::Time::Now();
+    bool found_at_least_one_device = false;
+    result->clear();
+    for (size_t i = 0; i < devices->GetSize(); i++) {
+      std::string device_path;
+      if (!devices->GetString(i, &device_path)) {
+        LOG(WARNING) << "Couldn't get devices[" << i << "]";
+        continue;
+      }
+      scoped_ptr<base::DictionaryValue> device_properties(
+          DBusThreadManager::Get()->GetFlimflamDeviceClient()->
+          CallGetPropertiesAndBlock(dbus::ObjectPath(device_path)));
+      if (!device_properties.get()) {
+        LOG(WARNING) << "Couldn't read device's properties " << device_path;
+        continue;
+      }
+
+      base::ListValue* networks = NULL;
+      if (!device_properties->GetListWithoutPathExpansion(
+              flimflam::kNetworksProperty, &networks))
+        continue;  // Some devices do not list networks, e.g. ethernet.
+
+      base::Value* device_powered_value = NULL;
+      bool device_powered = false;
+      if (device_properties->GetWithoutPathExpansion(
+              flimflam::kPoweredProperty, &device_powered_value) &&
+          device_powered_value->GetAsBoolean(&device_powered) &&
+          !device_powered)
+        continue;  // Skip devices that are not powered up.
+
+      int scan_interval = 0;
+      device_properties->GetIntegerWithoutPathExpansion(
+          flimflam::kScanIntervalProperty, &scan_interval);
+
+      found_at_least_one_device = true;
+      for (size_t j = 0; j < networks->GetSize(); j++) {
+        std::string network_path;
+        if (!networks->GetString(j, &network_path)) {
+          LOG(WARNING) << "Couldn't get networks[" << j << "]";
+          continue;
+        }
+
+        scoped_ptr<base::DictionaryValue> network_properties(
+            DBusThreadManager::Get()->GetFlimflamNetworkClient()->
+            CallGetPropertiesAndBlock(dbus::ObjectPath(network_path)));
+        if (!network_properties.get()) {
+          LOG(WARNING) << "Couldn't read network's properties " << network_path;
+          continue;
+        }
+
+        // Using the scan interval as a proxy for approximate age.
+        // TODO(joth): Replace with actual age, when available from dbus.
+        const int age_seconds = scan_interval;
+        WifiAccessPoint ap;
+        network_properties->GetStringWithoutPathExpansion(
+            flimflam::kAddressProperty, &ap.mac_address);
+        network_properties->GetStringWithoutPathExpansion(
+            flimflam::kNameProperty, &ap.name);
+        ap.timestamp = now - base::TimeDelta::FromSeconds(age_seconds);
+        network_properties->GetIntegerWithoutPathExpansion(
+            flimflam::kSignalStrengthProperty, &ap.signal_strength);
+        network_properties->GetIntegerWithoutPathExpansion(
+            flimflam::kWifiChannelProperty, &ap.channel);
+        result->push_back(ap);
+      }
+    }
+    if (!found_at_least_one_device)
+      return false;  // No powered device found that has a 'Networks' array.
+    return true;
+  }
 }
 
 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 2671e6b..acc559f 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
+++ b/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
@@ -13,6 +13,7 @@
 #include "chromeos/dbus/mock_flimflam_device_client.h"
 #include "chromeos/dbus/mock_flimflam_ipconfig_client.h"
 #include "chromeos/dbus/mock_flimflam_manager_client.h"
+#include "chromeos/dbus/mock_flimflam_network_client.h"
 #include "chromeos/dbus/mock_flimflam_profile_client.h"
 #include "chromeos/dbus/mock_flimflam_service_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -778,6 +779,8 @@ class CrosNetworkFunctionsTest : public testing::Test {
         mock_dbus_thread_manager->mock_flimflam_ipconfig_client();
     mock_manager_client_ =
         mock_dbus_thread_manager->mock_flimflam_manager_client();
+    mock_network_client_ =
+        mock_dbus_thread_manager->mock_flimflam_network_client();
     mock_profile_client_ =
         mock_dbus_thread_manager->mock_flimflam_profile_client();
     mock_service_client_ =
@@ -829,6 +832,7 @@ class CrosNetworkFunctionsTest : public testing::Test {
   MockFlimflamDeviceClient* mock_device_client_;
   MockFlimflamIPConfigClient* mock_ipconfig_client_;
   MockFlimflamManagerClient* mock_manager_client_;
+  MockFlimflamNetworkClient* mock_network_client_;
   MockFlimflamProfileClient* mock_profile_client_;
   MockFlimflamServiceClient* mock_service_client_;
   const base::DictionaryValue* dictionary_value_result_;
@@ -1330,6 +1334,70 @@ TEST_F(CrosNetworkFunctionsTest, CrosRemoveIPConfig) {
   CrosRemoveIPConfig(&config);
 }
 
+TEST_F(CrosNetworkFunctionsTest, CrosGetWifiAccessPoints) {
+  const std::string device_path = "/device/path";
+  base::ListValue* devices = new base::ListValue;
+  devices->Append(base::Value::CreateStringValue(device_path));
+  base::DictionaryValue* manager_properties = new base::DictionaryValue;
+  manager_properties->SetWithoutPathExpansion(
+      flimflam::kDevicesProperty, devices);
+
+  const int kScanInterval = 42;
+  const std::string network_path = "/network/path";
+  base::ListValue* networks = new base::ListValue;
+  networks->Append(base::Value::CreateStringValue(network_path));
+  base::DictionaryValue* device_properties = new base::DictionaryValue;
+  device_properties->SetWithoutPathExpansion(
+      flimflam::kNetworksProperty, networks);
+  device_properties->SetWithoutPathExpansion(
+      flimflam::kPoweredProperty, base::Value::CreateBooleanValue(true));
+  device_properties->SetWithoutPathExpansion(
+      flimflam::kScanIntervalProperty,
+      base::Value::CreateIntegerValue(kScanInterval));
+
+  const int kSignalStrength = 10;
+  const int kWifiChannel = 3;
+  const std::string address = "address";
+  const std::string name = "name";
+  const base::Time expected_timestamp =
+      base::Time::Now() - base::TimeDelta::FromSeconds(kScanInterval);
+  const base::TimeDelta acceptable_timestamp_range =
+      base::TimeDelta::FromSeconds(1);
+
+  base::DictionaryValue* network_properties = new base::DictionaryValue;
+  network_properties->SetWithoutPathExpansion(
+      flimflam::kAddressProperty, base::Value::CreateStringValue(address));
+  network_properties->SetWithoutPathExpansion(
+      flimflam::kNameProperty, base::Value::CreateStringValue(name));
+  network_properties->SetWithoutPathExpansion(
+      flimflam::kSignalStrengthProperty,
+      base::Value::CreateIntegerValue(kSignalStrength));
+  network_properties->SetWithoutPathExpansion(
+      flimflam::kWifiChannelProperty,
+      base::Value::CreateIntegerValue(kWifiChannel));
+
+  // Set expectations.
+  EXPECT_CALL(*mock_manager_client_, CallGetPropertiesAndBlock())
+      .WillOnce(Return(manager_properties));
+  EXPECT_CALL(*mock_device_client_,
+              CallGetPropertiesAndBlock(dbus::ObjectPath(device_path)))
+      .WillOnce(Return(device_properties));
+  EXPECT_CALL(*mock_network_client_,
+              CallGetPropertiesAndBlock(dbus::ObjectPath(network_path)))
+      .WillOnce(Return(network_properties));
+
+  // Call function.
+  WifiAccessPointVector aps;
+  ASSERT_TRUE(CrosGetWifiAccessPoints(&aps));
+  ASSERT_EQ(1U, aps.size());
+  EXPECT_EQ(address, aps[0].mac_address);
+  EXPECT_EQ(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(kSignalStrength, aps[0].signal_strength);
+  EXPECT_EQ(kWifiChannel, aps[0].channel);
+}
+
 TEST_F(CrosNetworkFunctionsTest, CrosConfigureService) {
   const std::string key1 = "key1";
   const std::string string1 = "string1";
diff --git a/chromeos/dbus/flimflam_manager_client.cc b/chromeos/dbus/flimflam_manager_client.cc
index 2374854..d09d34d 100644
--- a/chromeos/dbus/flimflam_manager_client.cc
+++ b/chromeos/dbus/flimflam_manager_client.cc
@@ -73,6 +73,12 @@ class FlimflamManagerClientImpl : public FlimflamManagerClient {
     helper_.CallDictionaryValueMethod(&method_call, callback);
   }
 
+  virtual base::DictionaryValue* CallGetPropertiesAndBlock() OVERRIDE {
+    dbus::MethodCall method_call(flimflam::kFlimflamManagerInterface,
+                                 flimflam::kGetPropertiesFunction);
+    return helper_.CallDictionaryValueMethodAndBlock(&method_call);
+  }
+
   virtual void SetProperty(const std::string& name,
                            const base::Value& value,
                            const VoidCallback& callback) OVERRIDE {
@@ -162,6 +168,11 @@ class FlimflamManagerClientStubImpl : public FlimflamManagerClient {
   }
 
   // FlimflamManagerClient override.
+  virtual base::DictionaryValue* CallGetPropertiesAndBlock() OVERRIDE {
+    return new base::DictionaryValue;
+  }
+
+  // FlimflamManagerClient override.
   virtual void SetProperty(const std::string& name,
                            const base::Value& value,
                            const VoidCallback& callback) OVERRIDE {
diff --git a/chromeos/dbus/flimflam_manager_client.h b/chromeos/dbus/flimflam_manager_client.h
index 45b6806..b767703 100644
--- a/chromeos/dbus/flimflam_manager_client.h
+++ b/chromeos/dbus/flimflam_manager_client.h
@@ -49,6 +49,14 @@ class CHROMEOS_EXPORT FlimflamManagerClient {
   // |callback| is called after the method call succeeds.
   virtual void GetProperties(const DictionaryValueCallback& callback) = 0;
 
+  // DEPRECATED DO NOT USE: Calls GetProperties method and blocks until the
+  // method call finishes.  The caller is responsible to delete the result.
+  // Thie method returns NULL when method call fails.
+  //
+  // TODO(hashimoto): Refactor CrosGetWifiAccessPoints and remove this method.
+  // crosbug.com/29902
+  virtual base::DictionaryValue* CallGetPropertiesAndBlock() = 0;
+
   // Calls SetProperty method.
   // |callback| is called after the method call succeeds.
   virtual void SetProperty(const std::string& name,
diff --git a/chromeos/dbus/flimflam_manager_client_unittest.cc b/chromeos/dbus/flimflam_manager_client_unittest.cc
index 6dcc0df..502f67f 100644
--- a/chromeos/dbus/flimflam_manager_client_unittest.cc
+++ b/chromeos/dbus/flimflam_manager_client_unittest.cc
@@ -169,6 +169,33 @@ TEST_F(FlimflamManagerClientTest, GetProperties) {
   message_loop_.RunAllPending();
 }
 
+TEST_F(FlimflamManagerClientTest, CallGetPropertiesAndBlock) {
+  // Create response.
+  scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  dbus::MessageWriter array_writer(NULL);
+  writer.OpenArray("{sv}", &array_writer);
+  dbus::MessageWriter entry_writer(NULL);
+  array_writer.OpenDictEntry(&entry_writer);
+  entry_writer.AppendString(flimflam::kOfflineModeProperty);
+  entry_writer.AppendVariantOfBool(true);
+  array_writer.CloseContainer(&entry_writer);
+  writer.CloseContainer(&array_writer);
+
+  // Create the expected value.
+  base::DictionaryValue value;
+  value.SetWithoutPathExpansion(flimflam::kOfflineModeProperty,
+                                base::Value::CreateBooleanValue(true));
+  // Set expectations.
+  PrepareForMethodCall(flimflam::kGetPropertiesFunction,
+                       base::Bind(&ExpectNoArgument),
+                       response.get());
+  // Call method.
+  scoped_ptr<base::DictionaryValue> result(
+      client_->CallGetPropertiesAndBlock());
+  EXPECT_TRUE(value.Equals(result.get()));
+}
+
 TEST_F(FlimflamManagerClientTest, SetProperty) {
   // Create response.
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
diff --git a/chromeos/dbus/mock_flimflam_manager_client.h b/chromeos/dbus/mock_flimflam_manager_client.h
index ca50f9b..f412ff8 100644
--- a/chromeos/dbus/mock_flimflam_manager_client.h
+++ b/chromeos/dbus/mock_flimflam_manager_client.h
@@ -20,6 +20,7 @@ class MockFlimflamManagerClient : public FlimflamManagerClient {
       const PropertyChangedHandler& handler));
   MOCK_METHOD0(ResetPropertyChangedHandler, void());
   MOCK_METHOD1(GetProperties, void(const DictionaryValueCallback& callback));
+  MOCK_METHOD0(CallGetPropertiesAndBlock, base::DictionaryValue*());
   MOCK_METHOD3(SetProperty, void(const std::string& name,
                                  const base::Value& value,
                                  const VoidCallback& callback));