Add support for cellular geolocation

chromeos/geolocation/simple_geolocation_provider.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_provider.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "chromeos/geolocation/geoposition.h"
 #include "chromeos/network/geolocation_handler.h"
 #include "chromeos/network/network_handler.h"
 #include "net/url_request/url_request_context_getter.h"
 
 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;
+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;
+  }
 
-  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;
-  }
-  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
 
 SimpleGeolocationProvider::SimpleGeolocationProvider(
     net::URLRequestContextGetter* url_context_getter,
     const GURL& url)
     : url_context_getter_(url_context_getter), url_(url) {
 }
 
 SimpleGeolocationProvider::~SimpleGeolocationProvider() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 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
   // "this" because destruction of SimpleGeolocationProvider cancels all
   // requests.
   SimpleGeolocationRequest::ResponseCallback callback_tmp(
       base::Bind(&SimpleGeolocationProvider::OnGeolocationResponse,
                  base::Unretained(this),
                  request,
                  callback));
@@ skipping 22 matching lines @@
             return req.get() == request;
           });
   DCHECK(position != requests_.end());
   if (position != requests_.end()) {
     std::swap(*position, *requests_.rbegin());
     requests_.resize(requests_.size() - 1);
   }
 }
 
 }  // namespace chromeos