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

content/browser/geolocation/location_arbitrator_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
 #define CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
 
 #include <stdint.h>
 #include <vector>
 
@@ skipping 42 matching lines @@
   void StartProviders(bool enable_high_accuracy) override;
   void StopProviders() override;
   void OnPermissionGranted() override;
   bool HasPermissionBeenGranted() const override;
 
  protected:
   AccessTokenStore* GetAccessTokenStore();
 
   // These functions are useful for injection of dependencies in derived
   // testing classes.
-  virtual AccessTokenStore* NewAccessTokenStore();
+  virtual scoped_refptr<AccessTokenStore> NewAccessTokenStore();

[Wez, 2016/07/12 21:55:47]

If you're changing this then I'd suggest also updating:

scoped_refptr<> GetAccessTokenStore()

.. NewNetworkLocationProvider(
  const scoped_refptr<>& access_token_store,
  const scoped_refptr<>& url_request_context_getter,
  ...
  )

Updating the URLRequestContextGetter* instances to scoped_refptr should be
pretty self-contained to this file, though I notice that there is also a bare
reference in the AccessTokenStore callback signature.

[mcasas, 2016/07/15 22:45:54]

Done.

   virtual std::unique_ptr<LocationProvider> NewNetworkLocationProvider(
       AccessTokenStore* access_token_store,
       net::URLRequestContextGetter* context,
       const GURL& url,
       const base::string16& access_token);
   virtual std::unique_ptr<LocationProvider> NewSystemLocationProvider();
   virtual base::Time GetTimeNow() const;
 
  private:
   friend class TestingLocationArbitrator;
@@ skipping 39 matching lines @@
   Geoposition position_;
 
   // Tracks whether providers should be running.
   bool is_running_;
 
   DISALLOW_COPY_AND_ASSIGN(LocationArbitratorImpl);
 };
 
 // Factory functions for the various types of location provider to abstract
 // over the platform-dependent implementations.
 LocationProvider* NewSystemLocationProvider();

[Wez, 2016/07/12 21:55:47]

nit: While we're here... it's weird that this is a bare pointer when the version
in the class is not.

Why do we even need both the static and member function versions?

[Michael van Ouwerkerk, 2016/07/13 10:56:34]

The virtual member function NewSystemLocationProvider is overridden in
location_arbitrator_impl_unittest.cc. It seems like an unusual pattern, nowadays
we would use something like setFooForTesting instead, if possible.

[mcasas, 2016/07/15 22:45:54]

Done.

 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_