Add support for cellular geolocation

chromeos/geolocation/simple_geolocation_provider.cc

Index: chromeos/geolocation/simple_geolocation_provider.cc
diff --git a/chromeos/geolocation/simple_geolocation_provider.cc b/chromeos/geolocation/simple_geolocation_provider.cc
index 45d6981dcb8b08bdf1d25cbf8ec6dc848915c429..edc7d4f38eeb107dddec37d7df5e13d26671fe70 100644
--- a/chromeos/geolocation/simple_geolocation_provider.cc
+++ b/chromeos/geolocation/simple_geolocation_provider.cc
@@ -17,21 +17,39 @@
 namespace chromeos {
 
 namespace {
+
+struct WifiCellTowerPair {
+  std::unique_ptr<WifiAccessPointVector> wifi_vector;
+  std::unique_ptr<CellTowerVector> cell_vector;
+};
+
 const char kDefaultGeolocationProviderUrl[] =
     "https://www.googleapis.com/geolocation/v1/geolocate?";
 
-std::unique_ptr<WifiAccessPointVector> GetAccessPointData() {
-  if (!chromeos::NetworkHandler::Get()->geolocation_handler()->wifi_enabled())
-    return nullptr;
-
-  std::unique_ptr<WifiAccessPointVector> result(
-      new chromeos::WifiAccessPointVector);
-  int64_t age_ms = 0;
-  if (!NetworkHandler::Get()->geolocation_handler()->GetWifiAccessPoints(
-          result.get(), &age_ms)) {
-    return nullptr;
+WifiCellTowerPair GetNetworkInfo(bool send_wifi_access_points,

[stevenjb, 2017/02/07 23:54:29]

use unique_ptr<WifiCellTowerPair> (also, see note below)

[can Skylar cook, 2017/02/08 21:15:49]

Done.

+                                 bool send_cell_towers) {
+  WifiCellTowerPair network_info;
+  network_info.wifi_vector = nullptr;
+  network_info.cell_vector = nullptr;
+  // Mostly necessary for testing, or rare cases where NetworkHandler is not
+  // initialized: in that case, calls to Get() will fail.
+  if (!send_wifi_access_points && !send_cell_towers) {
+    return network_info;
   }
-  return result;
+
+  auto cell_vector = base::MakeUnique<chromeos::CellTowerVector>();
+  auto wifi_vector = base::MakeUnique<chromeos::WifiAccessPointVector>();
+
+  NetworkHandler::Get()->geolocation_handler()->GetNetworkInformation(
+      wifi_vector.get(), cell_vector.get());
+
+  if (send_wifi_access_points && (wifi_vector->size() > 0))
+    network_info.wifi_vector = std::move(wifi_vector);
+
+  if (send_cell_towers && (cell_vector->size() > 0))
+    network_info.cell_vector = std::move(cell_vector);
+
+  return network_info;
 }
 
 }  // namespace
@@ -49,12 +67,16 @@ SimpleGeolocationProvider::~SimpleGeolocationProvider() {
 void SimpleGeolocationProvider::RequestGeolocation(
     base::TimeDelta timeout,
     bool send_wifi_access_points,
+    bool send_cell_towers,
     SimpleGeolocationRequest::ResponseCallback callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  SimpleGeolocationRequest* request(new SimpleGeolocationRequest(
-      url_context_getter_.get(), url_, timeout,
-      send_wifi_access_points ? GetAccessPointData() : nullptr));
+  auto network_info = GetNetworkInfo(send_wifi_access_points, send_cell_towers);

[stevenjb, 2017/02/07 23:54:29]

We tend to avoid this use of auto since it isn't obvious what is returned. e.g.
it's not obvious that this (currently) returns a struct and not an unowned
pointer (which would be bad). Declaring this as a unique_ptr<WifiCellTowerPair>
will make ownership crystal clear.

[can Skylar cook, 2017/02/08 21:15:49]

Acknowledged.

+
+  SimpleGeolocationRequest* request(
+      new SimpleGeolocationRequest(url_context_getter_.get(), url_, timeout,
+                                   std::move(network_info.wifi_vector),
+                                   std::move(network_info.cell_vector)));

[stevenjb, 2017/02/07 23:54:29]

It would be more clear to make WifiCellTowerPair a member of
SimpleGeolocationRequest and then we can pass |network_info| to
SimpleGeolocationRequest(). Right now we end up with a struct containing empty
vectors, which is a little bit awkward.

Alternately, just inline GetNetworkInfo, then you don't need the awkward struct
at all.

[can Skylar cook, 2017/02/08 21:15:49]

Done.

   requests_.push_back(base::WrapUnique(request));
 
   // SimpleGeolocationProvider owns all requests. It is safe to pass unretained