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

content/browser/geolocation/network_location_provider_unittest.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 <stddef.h>
 
 #include <memory>
 #include <utility>
@@ skipping 22 matching lines @@
 const char kAccessTokenString[] = "accessToken";
 
 // Using #define so we can easily paste this into various other strings.
 #define REFERENCE_ACCESS_TOKEN "2:k7j3G6LaL6u_lafw:4iXOeOpTh1glSXe"
 
 // Stops the specified (nested) message loop when the listener is called back.
 class MessageLoopQuitListener {
  public:
   MessageLoopQuitListener()
       : client_message_loop_(base::MessageLoop::current()),
-        updated_provider_(NULL) {
+        updated_provider_(nullptr) {
     CHECK(client_message_loop_);
   }
 
   void OnLocationUpdate(const LocationProvider* provider,
                         const Geoposition& position) {
     EXPECT_EQ(client_message_loop_, base::MessageLoop::current());
     updated_provider_ = provider;
     client_message_loop_->QuitWhenIdle();
   }
 
@@ skipping 38 matching lines @@
       this->RunCallbacks();
   }
 
   void set_got_data(bool got_data) { got_data_ = got_data; }
   int start_calls_;
   int stop_calls_;
 
  private:
   ~MockWifiDataProvider() override {
     CHECK(this == instance_);
-    instance_ = NULL;
+    instance_ = nullptr;
   }
 
   static MockWifiDataProvider* instance_;
 
   WifiData data_;
   bool got_data_;
 
   DISALLOW_COPY_AND_ASSIGN(MockWifiDataProvider);
 };
 
-MockWifiDataProvider* MockWifiDataProvider::instance_ = NULL;
+MockWifiDataProvider* MockWifiDataProvider::instance_ = nullptr;
 
 // Main test fixture
 class GeolocationNetworkProviderTest : public testing::Test {
  public:
-  void SetUp() override {
-    test_server_url_ = GURL(kTestServerUrl);
-    access_token_store_ = new FakeAccessTokenStore;
-    wifi_data_provider_ = MockWifiDataProvider::CreateInstance();
-  }
-
   void TearDown() override {
     WifiDataProviderManager::ResetFactoryForTesting();
   }
 
   LocationProvider* CreateProvider(bool set_permission_granted) {
     LocationProvider* provider = NewNetworkLocationProvider(
-        access_token_store_.get(),
-        NULL,  // No URLContextGetter needed, as using test urlfecther factory.
+        access_token_store_,
+        nullptr,  // No URLContextGetter needed, using test urlfecther factory.
         test_server_url_,
         access_token_store_->access_token_map_[test_server_url_]);
     if (set_permission_granted)
       provider->OnPermissionGranted();
     return provider;
   }
 
  protected:
-  GeolocationNetworkProviderTest() {
+  GeolocationNetworkProviderTest()
+      : test_server_url_(kTestServerUrl),
+        access_token_store_(new FakeAccessTokenStore),
+        wifi_data_provider_(MockWifiDataProvider::CreateInstance()) {
     // TODO(joth): Really these should be in SetUp, not here, but they take no
     // effect on Mac OS Release builds if done there. I kid not. Figure out why.
     WifiDataProviderManager::SetFactoryForTesting(
         MockWifiDataProvider::GetInstance);
   }
 
   // Returns the current url fetcher (if any) and advances the id ready for the
   // next test step.
   net::TestURLFetcher* get_url_fetcher_and_advance_id() {
     net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(
@@ skipping 110 matching lines @@
     // network provider url.
     bool is_default_url = UrlWithoutQuery(request_url) ==
         UrlWithoutQuery(LocationArbitratorImpl::DefaultNetworkProviderURL());
     EXPECT_EQ(is_default_url, !request_url.query().empty());
 
     const std::string& upload_data = request.upload_data();
     ASSERT_FALSE(upload_data.empty());
     std::string json_parse_error_msg;
     std::unique_ptr<base::Value> parsed_json =
         base::JSONReader::ReadAndReturnError(upload_data, base::JSON_PARSE_RFC,
-                                             NULL, &json_parse_error_msg);
+                                             nullptr, &json_parse_error_msg);
     EXPECT_TRUE(json_parse_error_msg.empty());
-    ASSERT_TRUE(parsed_json.get() != NULL);
+    ASSERT_TRUE(parsed_json.get() != nullptr);

[Wez, 2016/07/16 00:35:10]

nit: ASSERT_FALSE(parsed_json)

[mcasas, 2016/07/16 01:17:46]

You mean ASSERT_TRUE(parsed_json);
Done

 
     const base::DictionaryValue* request_json;
     ASSERT_TRUE(parsed_json->GetAsDictionary(&request_json));
 
     if (!is_default_url) {
       if (expected_access_token.empty())
         ASSERT_FALSE(request_json->HasKey(kAccessTokenString));
       else {
         std::string access_token;
         EXPECT_TRUE(request_json->GetString(kAccessTokenString, &access_token));
@@ skipping 25 matching lines @@
         ASSERT_TRUE(JsonFieldEquals("signalToNoiseRatio", *expected_json,
                                     *actual_json));
       }
     } else {
       ASSERT_FALSE(request_json->HasKey("wifiAccessPoints"));
     }
     EXPECT_TRUE(request_url.is_valid());
   }
 
   GURL test_server_url_;
-  base::MessageLoop main_message_loop_;
-  scoped_refptr<FakeAccessTokenStore> access_token_store_;
-  net::TestURLFetcherFactory url_fetcher_factory_;
-  scoped_refptr<MockWifiDataProvider> wifi_data_provider_;
+  const base::MessageLoop main_message_loop_;
+  const scoped_refptr<FakeAccessTokenStore> access_token_store_;
+  const net::TestURLFetcherFactory url_fetcher_factory_;
+  const scoped_refptr<MockWifiDataProvider> wifi_data_provider_;

[Wez, 2016/07/16 00:35:10]

Is it useful to const-ify all these test members?

[mcasas, 2016/07/16 01:17:46]

Absolutely, please see my reply before.

 };
 
 TEST_F(GeolocationNetworkProviderTest, CreateDestroy) {
   // Test fixture members were SetUp correctly.
   EXPECT_EQ(&main_message_loop_, base::MessageLoop::current());
   std::unique_ptr<LocationProvider> provider(CreateProvider(true));
-  EXPECT_TRUE(NULL != provider.get());
+  EXPECT_TRUE(nullptr != provider.get());

[Wez, 2016/07/16 00:35:10]

EXPECT_TRUE(provider)

[mcasas, 2016/07/16 01:17:46]

Done.

   provider.reset();
   SUCCEED();
 }
 
 TEST_F(GeolocationNetworkProviderTest, StartProvider) {
   std::unique_ptr<LocationProvider> provider(CreateProvider(true));
   EXPECT_TRUE(provider->StartProvider(false));
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);

[Wez, 2016/07/16 00:35:10]

ASSERT_TRUE(fetcher) here and below.

[mcasas, 2016/07/16 01:17:46]

Done here and elsewhere.

   CheckRequestIsValid(*fetcher, 0, 0, 0, std::string());
 }
 
 TEST_F(GeolocationNetworkProviderTest, StartProviderDefaultUrl) {
   test_server_url_ = LocationArbitratorImpl::DefaultNetworkProviderURL();
   std::unique_ptr<LocationProvider> provider(CreateProvider(true));
   EXPECT_TRUE(provider->StartProvider(false));
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
   CheckRequestIsValid(*fetcher, 0, 0, 0, std::string());
 }
 
 TEST_F(GeolocationNetworkProviderTest, StartProviderLongRequest) {
   std::unique_ptr<LocationProvider> provider(CreateProvider(true));
   EXPECT_TRUE(provider->StartProvider(false));
   const int kFirstScanAps = 20;
   wifi_data_provider_->SetData(CreateReferenceWifiScanData(kFirstScanAps));
   base::RunLoop().RunUntilIdle();
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
   // The request url should have been shortened to less than 2048 characters
   // in length by not including access points with the lowest signal strength
   // in the request.
   EXPECT_LT(fetcher->GetOriginalURL().spec().size(), size_t(2048));
   CheckRequestIsValid(*fetcher, 0, 16, 4, std::string());
 }
 
 TEST_F(GeolocationNetworkProviderTest, MultipleWifiScansComplete) {
   std::unique_ptr<LocationProvider> provider(CreateProvider(true));
   EXPECT_TRUE(provider->StartProvider(false));
 
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
   EXPECT_TRUE(IsTestServerUrl(fetcher->GetOriginalURL()));
 
   // Complete the network request with bad position fix.
   const char* kNoFixNetworkResponse =
       "{"
       "  \"status\": \"ZERO_RESULTS\""
       "}";
   fetcher->set_url(test_server_url_);
   fetcher->set_status(net::URLRequestStatus());
   fetcher->set_response_code(200);  // OK
   fetcher->SetResponseString(kNoFixNetworkResponse);
   fetcher->delegate()->OnURLFetchComplete(fetcher);
 
   Geoposition position;
   provider->GetPosition(&position);
   EXPECT_FALSE(position.Validate());
 
   // Now wifi data arrives -- SetData will notify listeners.
   const int kFirstScanAps = 6;
   wifi_data_provider_->SetData(CreateReferenceWifiScanData(kFirstScanAps));
   base::RunLoop().RunUntilIdle();
   fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
   // The request should have the wifi data.
   CheckRequestIsValid(*fetcher, 0, kFirstScanAps, 0, std::string());
 
   // Send a reply with good position fix.
   const char* kReferenceNetworkResponse =
       "{"
       "  \"accessToken\": \"" REFERENCE_ACCESS_TOKEN "\","
       "  \"accuracy\": 1200.4,"
       "  \"location\": {"
       "    \"lat\": 51.0,"
@@ skipping 94 matching lines @@
 }
 
 TEST_F(GeolocationNetworkProviderTest, NetworkRequestDeferredForPermission) {
   std::unique_ptr<LocationProvider> provider(CreateProvider(false));
   EXPECT_TRUE(provider->StartProvider(false));
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
   EXPECT_FALSE(fetcher);
   provider->OnPermissionGranted();
 
   fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
 
   EXPECT_TRUE(IsTestServerUrl(fetcher->GetOriginalURL()));
 }
 
 TEST_F(GeolocationNetworkProviderTest,
        NetworkRequestWithWifiDataDeferredForPermission) {
   access_token_store_->access_token_map_[test_server_url_] =
       base::UTF8ToUTF16(REFERENCE_ACCESS_TOKEN);
   std::unique_ptr<LocationProvider> provider(CreateProvider(false));
   EXPECT_TRUE(provider->StartProvider(false));
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
   EXPECT_FALSE(fetcher);
 
   static const int kScanCount = 4;
   wifi_data_provider_->SetData(CreateReferenceWifiScanData(kScanCount));
   base::RunLoop().RunUntilIdle();
 
   fetcher = get_url_fetcher_and_advance_id();
   EXPECT_FALSE(fetcher);
 
   provider->OnPermissionGranted();
 
   fetcher = get_url_fetcher_and_advance_id();
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher != nullptr);
 
   CheckRequestIsValid(*fetcher, 0, kScanCount, 0, REFERENCE_ACCESS_TOKEN);
 }
 
 TEST_F(GeolocationNetworkProviderTest, NetworkPositionCache) {
   NetworkLocationProvider::PositionCache cache;
 
   const int kCacheSize = NetworkLocationProvider::PositionCache::kMaximumSize;
   for (int i = 1; i < kCacheSize * 2 + 1; ++i) {
     Geoposition pos = CreateReferencePosition(i);
     bool ret = cache.CachePosition(CreateReferenceWifiScanData(i), pos);
     EXPECT_TRUE(ret)  << i;
     const Geoposition* item =
         cache.FindPosition(CreateReferenceWifiScanData(i));
     ASSERT_TRUE(item) << i;
     EXPECT_EQ(pos.latitude, item->latitude)  << i;
     EXPECT_EQ(pos.longitude, item->longitude)  << i;
     if (i <= kCacheSize) {
       // Nothing should have spilled yet; check oldest item is still there.
       EXPECT_TRUE(cache.FindPosition(CreateReferenceWifiScanData(1)));
     } else {
       const int evicted = i - kCacheSize;
       EXPECT_FALSE(cache.FindPosition(CreateReferenceWifiScanData(evicted)));
       EXPECT_TRUE(cache.FindPosition(CreateReferenceWifiScanData(evicted + 1)));
     }
   }
 }
 
 }  // namespace content