Add support for cellular geolocation

chromeos/geolocation/simple_geolocation_request.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/geolocation/simple_geolocation_request.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 26 matching lines @@
                              base::TimeDelta::FromMinutes(2),       \
                              50)
 
 namespace chromeos {
 
 namespace {
 
 // The full request text. (no parameters are supported by now)
 const char kSimpleGeolocationRequestBody[] = "{\"considerIp\": \"true\"}";
 
-// Request data
+// TODO(skylarc): kill these and use dbus-constants instead?
+// Top-level request data fields

[stevenjb, 2017/02/03 02:12:02]

That would be nice :)
In the meanwhile, we should convert all of these to use constexpr.

[can Skylar cook, 2017/02/03 21:47:06]

Done.

 const char kConsiderIp[] = "considerIp";
 const char kWifiAccessPoints[] = "wifiAccessPoints";
+const char kCellTowers[] = "cellTowers";
+
+// Shared Wifi and Cell Tower objects
+const char kAge[] = "age";
+const char kSignalStrength[] = "signalStrength";
 
 // WiFi access point objects.
 const char kMacAddress[] = "macAddress";
-const char kSignalStrength[] = "signalStrength";
-const char kAge[] = "age";
 const char kChannel[] = "channel";
 const char kSignalToNoiseRatio[] = "signalToNoiseRatio";
 
+// Cell tower objects
+const char kCellId[] = "cellId";
+const char kLocationAreaCode[] = "locationAreaCode";
+const char kMobileCountryCode[] = "mobileCountryCode";
+const char kMobileNetworkCode[] = "mobileNetworkCode";
+
 // Response data.
 const char kLocationString[] = "location";
 const char kLatString[] = "lat";
 const char kLngString[] = "lng";
 const char kAccuracyString[] = "accuracy";
 // Error object and its contents.
 const char kErrorString[] = "error";
 // "errors" array in "erorr" object is ignored.
 const char kCodeString[] = "code";
 const char kMessageString[] = "message";
@@ skipping 216 matching lines @@
     RecordUmaEvent(SIMPLE_GEOLOCATION_REQUEST_EVENT_RESPONSE_NOT_OK);
     return false;
   }
 
   return ParseServerResponse(server_url, response_body, position);
 }
 
 void ReportUmaHasWiFiAccessPoints(bool value) {
   UMA_HISTOGRAM_BOOLEAN("SimpleGeolocation.Request.HasWiFiAccessPoints", value);
 }
+void ReportUmaHasCellTowers(bool value) {
+  UMA_HISTOGRAM_BOOLEAN("SimpleGeolocation.Request.HasCellTowers", value);
+}
 
 }  // namespace
 
 SimpleGeolocationRequest::SimpleGeolocationRequest(
     net::URLRequestContextGetter* url_context_getter,
     const GURL& service_url,
     base::TimeDelta timeout,
-    std::unique_ptr<WifiAccessPointVector> wifi_data)
+    std::unique_ptr<WifiAccessPointVector> wifi_data,
+    std::unique_ptr<CellTowerVector> cell_tower_data)
     : url_context_getter_(url_context_getter),
       service_url_(service_url),
       retry_sleep_on_server_error_(base::TimeDelta::FromSeconds(
           kResolveGeolocationRetrySleepOnServerErrorSeconds)),
       retry_sleep_on_bad_response_(base::TimeDelta::FromSeconds(
           kResolveGeolocationRetrySleepBadResponseSeconds)),
       timeout_(timeout),
       retries_(0),
-      wifi_data_(wifi_data.release()) {}
+      wifi_data_(wifi_data.release()),
+      cell_tower_data_(cell_tower_data.release()) {}
 
 SimpleGeolocationRequest::~SimpleGeolocationRequest() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // If callback is not empty, request is cancelled.
   if (!callback_.is_null()) {
     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_) {
+  if (!wifi_data_)
     ReportUmaHasWiFiAccessPoints(false);
+
+  if (!cell_tower_data_)
+    ReportUmaHasCellTowers(false);
+
+  if (!cell_tower_data_ && !wifi_data_)
     return std::string(kSimpleGeolocationRequestBody);
-  }
 
   std::unique_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);
+  if (wifi_data_) {
+    base::ListValue* wifi_access_points(new base::ListValue);
+    request->SetWithoutPathExpansion(kWifiAccessPoints, wifi_access_points);

[stevenjb, 2017/02/03 02:12:02]

This is deprecated; use a unique_ptr to hold wifi_access_points and move it
after appending all of the access points.

[can Skylar cook, 2017/02/03 21:47:06]

Whoops, meant to change that after Ben's comment on cellular below. Done.

+    for (const WifiAccessPoint& access_point : *wifi_data_) {
+      auto access_point_dictionary = base::MakeUnique<base::DictionaryValue>();
 
-  for (const WifiAccessPoint& access_point : *wifi_data_) {
-    auto access_point_dictionary = base::MakeUnique<base::DictionaryValue>();
+      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->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);

[stevenjb, 2017/02/03 02:12:02]

nit: This would be more readable with the above put into a helper function, i.e.
something like:
wifi_access_points->Append(CreateAccessPointDictionary(access_point));

[can Skylar cook, 2017/02/03 21:47:06]

Done.

+
+      wifi_access_points->Append(std::move(access_point_dictionary));
     }
+  }
 
-    access_point_dictionary->SetIntegerWithoutPathExpansion(
-        kChannel, access_point.channel);
-    access_point_dictionary->SetIntegerWithoutPathExpansion(
-        kSignalToNoiseRatio, access_point.signal_to_noise);
+  if (cell_tower_data_) {
+    auto cell_towers = base::MakeUnique<base::ListValue>();

[stevenjb, 2017/02/03 02:12:02]

Like this :)

[can Skylar cook, 2017/02/03 21:47:06]

Acknowledged.

 
-    wifi_access_points->Append(std::move(access_point_dictionary));
+    for (const CellTower& cell_tower : *cell_tower_data_) {
+      auto cell_tower_dictionary = base::MakeUnique<base::DictionaryValue>();
+      cell_tower_dictionary->SetStringWithoutPathExpansion(kCellId,
+                                                           cell_tower.ci);
+      cell_tower_dictionary->SetStringWithoutPathExpansion(kLocationAreaCode,
+                                                           cell_tower.lac);
+      cell_tower_dictionary->SetStringWithoutPathExpansion(kMobileCountryCode,
+                                                           cell_tower.mcc);
+      cell_tower_dictionary->SetStringWithoutPathExpansion(kMobileNetworkCode,
+                                                           cell_tower.mnc);
+
+      if (!cell_tower.timestamp.is_null()) {
+        cell_tower_dictionary->SetStringWithoutPathExpansion(
+            kAge,
+            base::Int64ToString(
+                (base::Time::Now() - cell_tower.timestamp).InMilliseconds()));
+      }

[stevenjb, 2017/02/03 02:12:02]

Ditto about moving the above to a helper function.

[can Skylar cook, 2017/02/03 21:47:06]

Done.

+      cell_towers->Append(std::move(cell_tower_dictionary));
+      request->SetWithoutPathExpansion(kCellTowers, std::move(cell_towers));
+    }
   }
+
   std::string result;
   if (!base::JSONWriter::Write(*request, &result)) {
-    ReportUmaHasWiFiAccessPoints(false);
+    // If there's no data for a network type, we will have already reported
+    // false above
+    if (wifi_data_)
+      ReportUmaHasWiFiAccessPoints(false);
+    if (cell_tower_data_)
+      ReportUmaHasCellTowers(false);
+
     return std::string(kSimpleGeolocationRequestBody);
   }
-  ReportUmaHasWiFiAccessPoints(wifi_data_->size());
+
+  if (wifi_data_)
+    ReportUmaHasWiFiAccessPoints(wifi_data_->size());
+  if (cell_tower_data_)
+    ReportUmaHasCellTowers(cell_tower_data_->size());
 
   return result;
 }
 
 void SimpleGeolocationRequest::StartRequest() {
   DCHECK(thread_checker_.CalledOnValidThread());
   RecordUmaEvent(SIMPLE_GEOLOCATION_REQUEST_EVENT_REQUEST_START);
   ++retries_;
 
   const std::string request_body = FormatRequestBody();
@@ skipping 100 matching lines @@
            ? SIMPLE_GEOLOCATION_REQUEST_RESULT_SERVER_ERROR
            : SIMPLE_GEOLOCATION_REQUEST_RESULT_FAILURE);
   RecordUmaResult(result, retries_);
   position_.status = Geoposition::STATUS_TIMEOUT;
   const base::TimeDelta elapsed = base::Time::Now() - request_started_at_;
   ReplyAndDestroySelf(elapsed, true /* server_error */);
   // "this" is already destroyed here.
 }
 
 }  // namespace chromeos