SimpleGeolocation should support sending WiFi AP data.

chromeos/geolocation/simple_geolocation_request.cc

Index: chromeos/geolocation/simple_geolocation_request.cc
diff --git a/chromeos/geolocation/simple_geolocation_request.cc b/chromeos/geolocation/simple_geolocation_request.cc
index 01f48a086cef36e1a480bb9218ebdc31de0b9142..a9d6e9080ca2698ccb61851486b2523dc0120906 100644
--- a/chromeos/geolocation/simple_geolocation_request.cc
+++ b/chromeos/geolocation/simple_geolocation_request.cc
@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
@@ -17,6 +18,7 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chromeos/geolocation/simple_geolocation_provider.h"
+#include "chromeos/geolocation/simple_geolocation_request_test_monitor.h"
 #include "google_apis/google_api_keys.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
@@ -38,7 +40,18 @@ namespace chromeos {
 namespace {
 
 // The full request text. (no parameters are supported by now)
-const char kSimpleGeolocationRequestBody[] = "{\"considerIP\": \"true\"}";
+const char kSimpleGeolocationRequestBody[] = "{\"considerIp\": \"true\"}";
+
+// Request data
+const char kConsiderIp[] = "considerIp";
+const char kWifiAccessPoints[] = "wifiAccessPoints";
+
+// WiFi access point objects.
+const char kMacAddress[] = "macAddress";
+const char kSignalStrength[] = "signalStrength";
+const char kAge[] = "age";
+const char kChannel[] = "channel";
+const char kSignalToNoiseRatio[] = "signalToNoiseRatio";
 
 // Response data.
 const char kLocationString[] = "location";
@@ -88,6 +101,8 @@ enum SimpleGeolocationRequestResult {
   SIMPLE_GEOLOCATION_REQUEST_RESULT_COUNT = 4
 };
 
+SimpleGeolocationRequestTestMonitor* g_test_request_hook = nullptr;
+
 // Too many requests (more than 1) mean there is a problem in implementation.
 void RecordUmaEvent(SimpleGeolocationRequestEvent event) {
   UMA_HISTOGRAM_ENUMERATION("SimpleGeolocation.Request.Event",
@@ -247,6 +262,9 @@ bool GetGeolocationFromResponse(bool http_success,
                                 const std::string& response_body,
                                 const GURL& server_url,
                                 Geoposition* position) {
+  VLOG(1) << "GetGeolocationFromResponse(http_success=" << http_success
+          << ", status_code=" << status_code << "): response_body:\n"
+          << response_body;
 
   // HttpPost can fail for a number of reasons. Most likely this is because
   // we're offline, or there was no response.
@@ -271,7 +289,8 @@ bool GetGeolocationFromResponse(bool http_success,
 SimpleGeolocationRequest::SimpleGeolocationRequest(
     net::URLRequestContextGetter* url_context_getter,
     const GURL& service_url,
-    base::TimeDelta timeout)
+    base::TimeDelta timeout,
+    scoped_ptr<WifiAccessPointVector> wifi_data)
     : url_context_getter_(url_context_getter),
       service_url_(service_url),
       retry_sleep_on_server_error_(base::TimeDelta::FromSeconds(
@@ -279,8 +298,8 @@ SimpleGeolocationRequest::SimpleGeolocationRequest(
       retry_sleep_on_bad_response_(base::TimeDelta::FromSeconds(
           kResolveGeolocationRetrySleepBadResponseSeconds)),
       timeout_(timeout),
-      retries_(0) {
-}
+      retries_(0),
+      wifi_data_(wifi_data.release()) {}
 
 SimpleGeolocationRequest::~SimpleGeolocationRequest() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -290,6 +309,54 @@ SimpleGeolocationRequest::~SimpleGeolocationRequest() {
     RecordUmaResponseTime(base::Time::Now() - request_started_at_, false);
     RecordUmaResult(SIMPLE_GEOLOCATION_REQUEST_RESULT_CANCELLED, retries_);
   }
+
+  if (g_test_request_hook)
+    g_test_request_hook->OnRequestCreated(this);
+}
+
+std::string SimpleGeolocationRequest::FormatRequestBody() const {
+  if (!wifi_data_) {
+    UMA_HISTOGRAM_BOOLEAN("SimpleGeolocation.Request.HasWiFiAccessPoints",

[Steven Holte, 2016/03/23 18:41:46]

It would be better to wrap a function call around this macro invocation, since
it will create a static variable pointing at the Histogram object.

[Alexander Alekseev, 2016/03/23 22:08:23]

Done.

+                          false);
+    return std::string(kSimpleGeolocationRequestBody);
+  }
+
+  scoped_ptr<base::DictionaryValue> request(new base::DictionaryValue);
+  request->SetBooleanWithoutPathExpansion(kConsiderIp, true);
+
+  base::ListValue* wifi_access_points(new base::ListValue);
+  request->SetWithoutPathExpansion(kWifiAccessPoints, wifi_access_points);
+
+  for (const WifiAccessPoint& access_point : *wifi_data_) {
+    base::DictionaryValue* access_point_dictionary = new base::DictionaryValue;
+    wifi_access_points->Append(access_point_dictionary);
+
+    access_point_dictionary->SetStringWithoutPathExpansion(
+        kMacAddress, access_point.mac_address);
+    access_point_dictionary->SetIntegerWithoutPathExpansion(
+        kSignalStrength, access_point.signal_strength);
+    if (!access_point.timestamp.is_null()) {
+      access_point_dictionary->SetStringWithoutPathExpansion(
+          kAge,
+          base::Int64ToString(
+              (base::Time::Now() - access_point.timestamp).InMilliseconds()));
+    }
+
+    access_point_dictionary->SetIntegerWithoutPathExpansion(
+        kChannel, access_point.channel);
+    access_point_dictionary->SetIntegerWithoutPathExpansion(
+        kSignalToNoiseRatio, access_point.signal_to_noise);
+  }
+  std::string result;
+  if (!base::JSONWriter::Write(*request, &result)) {
+    UMA_HISTOGRAM_BOOLEAN("SimpleGeolocation.Request.HasWiFiAccessPoints",
+                          false);
+    return std::string(kSimpleGeolocationRequestBody);
+  }
+  UMA_HISTOGRAM_BOOLEAN("SimpleGeolocation.Request.HasWiFiAccessPoints",
+                        wifi_data_->size());
+
+  return result;
 }
 
 void SimpleGeolocationRequest::StartRequest() {
@@ -297,16 +364,24 @@ void SimpleGeolocationRequest::StartRequest() {
   RecordUmaEvent(SIMPLE_GEOLOCATION_REQUEST_EVENT_REQUEST_START);
   ++retries_;
 
+  const std::string request_body = FormatRequestBody();
+  VLOG(1) << "SimpleGeolocationRequest::StartRequest(): request body:\n"
+          << request_body;
+
   url_fetcher_ =
       net::URLFetcher::Create(request_url_, net::URLFetcher::POST, this);
   url_fetcher_->SetRequestContext(url_context_getter_.get());
-  url_fetcher_->SetUploadData("application/json",
-                              std::string(kSimpleGeolocationRequestBody));
+  url_fetcher_->SetUploadData("application/json", request_body);
   url_fetcher_->SetLoadFlags(net::LOAD_BYPASS_CACHE |
                              net::LOAD_DISABLE_CACHE |
                              net::LOAD_DO_NOT_SAVE_COOKIES |
                              net::LOAD_DO_NOT_SEND_COOKIES |
                              net::LOAD_DO_NOT_SEND_AUTH_DATA);
+
+  // Call test hook before asynchronous request actually starts.
+  if (g_test_request_hook)
+    g_test_request_hook->OnStart(this);
+
   url_fetcher_->Start();
 }
 
@@ -319,6 +394,16 @@ void SimpleGeolocationRequest::MakeRequest(const ResponseCallback& callback) {
   StartRequest();
 }
 
+// static
+void SimpleGeolocationRequest::SetTestMonitor(
+    SimpleGeolocationRequestTestMonitor* monitor) {
+  g_test_request_hook = monitor;
+}
+
+std::string SimpleGeolocationRequest::FormatRequestBodyForTesting() const {
+  return FormatRequestBody();
+}
+
 void SimpleGeolocationRequest::Retry(bool server_error) {
   base::TimeDelta delay(server_error ? retry_sleep_on_server_error_
                                      : retry_sleep_on_bad_response_);