Geolocation cleanup: corrects uses of content::AccessTokenStore* and net::URLRequestContextGetter*

content/browser/geolocation/network_location_provider.cc

 // Copyright (c) 2012 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 "content/browser/geolocation/network_location_provider.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 39 matching lines @@
     NOTREACHED();  // We never try to add the same key twice.
     CHECK_EQ(cache_.size(), cache_age_list_.size());
     return false;
   }
   cache_age_list_.push_back(result.first);
   DCHECK_EQ(cache_.size(), cache_age_list_.size());
   return true;
 }
 
 // Searches for a cached position response for the current WiFi data. Returns
-// the cached position if available, NULL otherwise.
+// the cached position if available, nullptr otherwise.
 const Geoposition* NetworkLocationProvider::PositionCache::FindPosition(
     const WifiData& wifi_data) {
   base::string16 key;
   if (!MakeKey(wifi_data, &key)) {
-    return NULL;
+    return nullptr;
   }
   CacheMap::const_iterator iter = cache_.find(key);
-  return iter == cache_.end() ? NULL : &iter->second;
+  return iter == cache_.end() ? nullptr : &iter->second;
 }
 
 // Makes the key for the map of cached positions, using the available data.
 // Returns true if a good key was generated, false otherwise.
 //
 // static
 bool NetworkLocationProvider::PositionCache::MakeKey(
     const WifiData& wifi_data,
     base::string16* key) {
   // Currently we use only WiFi data and base the key only on the MAC addresses.
   DCHECK(key);
   key->clear();
   const size_t kCharsPerMacAddress = 6 * 3 + 1;  // e.g. "11:22:33:44:55:66|"
   key->reserve(wifi_data.access_point_data.size() * kCharsPerMacAddress);
   const base::string16 separator(base::ASCIIToUTF16("|"));
   for (const auto& access_point_data : wifi_data.access_point_data) {
     *key += separator;
     *key += access_point_data.mac_address;
     *key += separator;
   }
   // If the key is the empty string, return false, as we don't want to cache a
   // position for such data.
   return !key->empty();
 }
 
 // NetworkLocationProvider factory function
 LocationProviderBase* NewNetworkLocationProvider(
-    AccessTokenStore* access_token_store,
-    net::URLRequestContextGetter* context,
+    const scoped_refptr<AccessTokenStore>& access_token_store,
+    const scoped_refptr<net::URLRequestContextGetter>& context,
     const GURL& url,
     const base::string16& access_token) {
   return new NetworkLocationProvider(
       access_token_store, context, url, access_token);
 }
 
 // NetworkLocationProvider
 NetworkLocationProvider::NetworkLocationProvider(
-    AccessTokenStore* access_token_store,
-    net::URLRequestContextGetter* url_context_getter,
+    const scoped_refptr<AccessTokenStore>& access_token_store,
+    const scoped_refptr<net::URLRequestContextGetter>& url_context_getter,
     const GURL& url,
     const base::string16& access_token)
     : access_token_store_(access_token_store),
-      wifi_data_provider_manager_(NULL),
+      wifi_data_provider_manager_(nullptr),
       wifi_data_update_callback_(
           base::Bind(&NetworkLocationProvider::OnWifiDataUpdate,
                      base::Unretained(this))),
       is_wifi_data_complete_(false),
       access_token_(access_token),
       is_permission_granted_(false),
       is_new_data_available_(false),
+      position_cache_(new PositionCache()),

[Wez, 2016/07/18 17:50:53]

nit: No () if no parameters to pass to ctor.

[mcasas, 2016/07/18 21:48:52]

Done.

       weak_factory_(this) {
-  // Create the position cache.
-  position_cache_.reset(new PositionCache());
-
   request_.reset(new NetworkLocationRequest(
       url_context_getter,
       url,
       base::Bind(&NetworkLocationProvider::OnLocationResponse,
                  base::Unretained(this))));
 }
 
 NetworkLocationProvider::~NetworkLocationProvider() {
   StopProvider();
 }
 
 // LocationProvider implementation
 void NetworkLocationProvider::GetPosition(Geoposition* position) {
   DCHECK(position);
   *position = position_;
 }
 
 void NetworkLocationProvider::RequestRefresh() {
   // TODO(joth): When called via the public (base class) interface, this should
   // poke each data provider to get them to expedite their next scan.
   // Whilst in the delayed start, only send request if all data is ready.
   if (!weak_factory_.HasWeakPtrs() || is_wifi_data_complete_) {

[Wez, 2016/07/18 17:50:53]

For a follow-up: We should fix this to at least explain why HasWeakPtrs() being
false indicates all data is ready, and preferably just not to use it, since I
suspect it does not have the intended behaviour.

[mcasas, 2016/07/18 21:48:53]

Done.

     RequestPosition();
   }
 }
 
 void NetworkLocationProvider::OnPermissionGranted() {
   const bool was_permission_granted = is_permission_granted_;
   is_permission_granted_ = true;
   if (!was_permission_granted && IsStarted()) {
     RequestRefresh();
   }
@@ skipping 24 matching lines @@
   }
 
   // Let listeners know that we now have a position available.
   NotifyCallback(position_);
 }
 
 bool NetworkLocationProvider::StartProvider(bool high_accuracy) {
   DCHECK(CalledOnValidThread());
   if (IsStarted())
     return true;
-  DCHECK(wifi_data_provider_manager_ == NULL);
+  DCHECK(!wifi_data_provider_manager_);
   if (!request_->url().is_valid()) {
     LOG(WARNING) << "StartProvider() : Failed, Bad URL: "
                  << request_->url().possibly_invalid_spec();
     return false;
   }
 
   // Registers a callback with the data provider. The first call to Register
   // will create a singleton data provider and it will be deleted when the last
   // callback is removed with Unregister.
   wifi_data_provider_manager_ =
@@ skipping 16 matching lines @@
 
   is_new_data_available_ = is_wifi_data_complete_;
   RequestRefresh();
 }
 
 void NetworkLocationProvider::StopProvider() {
   DCHECK(CalledOnValidThread());
   if (IsStarted()) {
     wifi_data_provider_manager_->Unregister(&wifi_data_update_callback_);
   }
-  wifi_data_provider_manager_ = NULL;
+  wifi_data_provider_manager_ = nullptr;
   weak_factory_.InvalidateWeakPtrs();
 }
 
 // Other methods
 void NetworkLocationProvider::RequestPosition() {
   DCHECK(CalledOnValidThread());
   if (!is_new_data_available_)
     return;
 
   const Geoposition* cached_position =
@@ skipping 24 matching lines @@
   // NetworkLocationRequest for each and hold a set of pending requests.
   if (request_->is_request_pending()) {
     DVLOG(1) << "NetworkLocationProvider - pre-empting pending network request "
                 "with new data. Wifi APs: "
              << wifi_data_.access_point_data.size();
   }
   request_->MakeRequest(access_token_, wifi_data_, wifi_timestamp_);
 }
 
 bool NetworkLocationProvider::IsStarted() const {
-  return wifi_data_provider_manager_ != NULL;
+  return wifi_data_provider_manager_ != nullptr;
 }
 
 }  // namespace content