Refactor to make BlimpLocationProvider accessible to content layer.

content/browser/geolocation/location_arbitrator_impl_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/location_arbitrator_impl.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "content/browser/geolocation/fake_access_token_store.h"
 #include "content/browser/geolocation/mock_location_provider.h"
 #include "content/public/common/geoposition.h"
+#include "content/test/test_content_browser_client.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::NiceMock;
 
 namespace content {
 
 class MockLocationObserver {
  public:
   // Need a vtable for GMock.
@@ skipping 34 matching lines @@
   ASSERT_TRUE(position.Validate());
   provider->HandlePositionChanged(position);
 }
 
 void SetReferencePosition(MockLocationProvider* provider) {
   SetPositionFix(provider, 51.0, -0.1, 400);
 }
 
 namespace {
 
+class GeolocationContentBrowserClient : public TestContentBrowserClient {
+ public:
+  void SetUseNetwork(bool use_network) { use_network_ = use_network; }

[Wez, 2016/06/10 23:40:35]

nit: As per Chromium-style guide, simple setters and getters should use
unix_hacker_style() naming.

[CJ, 2016/06/13 23:45:20]

Done.

+
+  LocationProvider* OverrideSystemLocationProvider() override {

[Wez, 2016/06/10 23:40:35]

Note that in general we do not allow virtual functions to be declared in-line in
Chromium - the only reason we're allowing it here is that that is already the
style this code file follows, and it's a test file. Please don't use inline
virtuals in production code, or new test code. :)

[CJ, 2016/06/13 23:45:20]

Acknowledged.

+    provider_ = new MockLocationProvider;
+    return provider_;
+  }
+
+  bool UseNetworkLocationProviders() override { return use_network_; }
+
+  MockLocationProvider* provider_ = nullptr;

[Wez, 2016/06/10 23:40:34]

Please add a comment here to clarify that this pointer does not own the object -
when it is returned by Override...() the caller takes ownership, so this is just
a reference we can use for mocking purposes.

(I was about to ask why we need this, since we can just set expectations on the
mock and then pass ownership in to the location framework, but looking at the
MockLocationProvider, it seems it's actually _not_ a mock (thing that looks like
Foo that just lets you test whether Foo APIs were called) but a fake (thing that
is designed to actually pretend to behave like a real Foo). :-/ )

[CJ, 2016/06/13 23:45:20]

Done.

+
+ private:
+  bool use_network_ = true;
+};

[Wez, 2016/06/10 23:40:35]

nit: DISALLOW_COPY_AND_ASSIGN

[CJ, 2016/06/13 23:45:20]

Done.

+
 class TestingLocationArbitrator : public LocationArbitratorImpl {
  public:
   TestingLocationArbitrator(
       const LocationArbitratorImpl::LocationUpdateCallback& callback,
       AccessTokenStore* access_token_store)
       : LocationArbitratorImpl(callback),
-        cell_(NULL),
-        gps_(NULL),
-        access_token_store_(access_token_store) {
-  }
+        cell_(nullptr),
+        gps_(nullptr),
+        access_token_store_(access_token_store) {}
 
   base::Time GetTimeNow() const override { return GetTimeNowForTest(); }
 
   AccessTokenStore* NewAccessTokenStore() override {
     return access_token_store_.get();
   }
 
   LocationProvider* NewNetworkLocationProvider(
       AccessTokenStore* access_token_store,
       net::URLRequestContextGetter* context,
       const GURL& url,
       const base::string16& access_token) override {
-    return new MockLocationProvider(&cell_);
+    cell_ = new MockLocationProvider;
+    return cell_;
   }
 
   LocationProvider* NewSystemLocationProvider() override {
-    return new MockLocationProvider(&gps_);
+    gps_ = new MockLocationProvider;
+    return gps_;
   }
 
   // Two location providers, with nice short names to make the tests more
   // readable. Note |gps_| will only be set when there is a high accuracy
   // observer registered (and |cell_| when there's at least one observer of any
   // type).
+
+  // TODO(mvanouwerkerk): rename |cell_| to |network_location_provider_| and
+  // |gps_| to |system_location_provider_|
   MockLocationProvider* cell_;
   MockLocationProvider* gps_;
   scoped_refptr<AccessTokenStore> access_token_store_;
 };
 
 }  // namespace
 
 class GeolocationLocationArbitratorTest : public testing::Test {
  protected:
   // testing::Test
   void SetUp() override {
     access_token_store_ = new NiceMock<FakeAccessTokenStore>;
     observer_.reset(new MockLocationObserver);
     LocationArbitratorImpl::LocationUpdateCallback callback =
         base::Bind(&MockLocationObserver::OnLocationUpdate,
                    base::Unretained(observer_.get()));
     arbitrator_.reset(new TestingLocationArbitrator(
         callback, access_token_store_.get()));
+    override_content_browser_client_.reset(
+        new GeolocationContentBrowserClient());
   }
 
   // testing::Test
   void TearDown() override {}
 
   void CheckLastPositionInfo(double latitude,
                              double longitude,
                              double accuracy) {
     Geoposition geoposition = observer_->last_position_;
     EXPECT_TRUE(geoposition.Validate());
     EXPECT_DOUBLE_EQ(latitude, geoposition.latitude);
     EXPECT_DOUBLE_EQ(longitude, geoposition.longitude);
     EXPECT_DOUBLE_EQ(accuracy, geoposition.accuracy);
   }
 
   base::TimeDelta SwitchOnFreshnessCliff() {
     // Add 1, to ensure it meets any greater-than test.
     return base::TimeDelta::FromMilliseconds(
         LocationArbitratorImpl::kFixStaleTimeoutMilliseconds + 1);
   }
 
   MockLocationProvider* cell() {
     return arbitrator_->cell_;
   }
 
   MockLocationProvider* gps() {
     return arbitrator_->gps_;
   }
 
+  MockLocationProvider* GetSystemLocationProviderOverride() {
+    return override_content_browser_client_->provider_;
+  }
+
   scoped_refptr<FakeAccessTokenStore> access_token_store_;
   std::unique_ptr<MockLocationObserver> observer_;
   std::unique_ptr<TestingLocationArbitrator> arbitrator_;
   base::MessageLoop loop_;
+  std::unique_ptr<GeolocationContentBrowserClient>
+      override_content_browser_client_;
 };
 
 TEST_F(GeolocationLocationArbitratorTest, CreateDestroy) {
   EXPECT_TRUE(access_token_store_.get());
-  EXPECT_TRUE(arbitrator_ != NULL);
+  EXPECT_TRUE(arbitrator_);
   arbitrator_.reset();
   SUCCEED();
 }
 
 TEST_F(GeolocationLocationArbitratorTest, OnPermissionGranted) {
   EXPECT_FALSE(arbitrator_->HasPermissionBeenGranted());
   arbitrator_->OnPermissionGranted();
   EXPECT_TRUE(arbitrator_->HasPermissionBeenGranted());
   // Can't check the provider has been notified without going through the
   // motions to create the provider (see next test).
   EXPECT_FALSE(cell());
   EXPECT_FALSE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
 }
 
 TEST_F(GeolocationLocationArbitratorTest, NormalUsage) {
   ASSERT_TRUE(access_token_store_.get());
-  ASSERT_TRUE(arbitrator_ != NULL);
+  ASSERT_TRUE(arbitrator_);
 
   EXPECT_FALSE(cell());
   EXPECT_FALSE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
   arbitrator_->StartProviders(false);
 
   EXPECT_TRUE(access_token_store_->access_token_map_.empty());
-  EXPECT_TRUE(access_token_store_->access_token_map_.empty());
 
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(cell());
   EXPECT_TRUE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, cell()->state_);
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, gps()->state_);
   EXPECT_FALSE(observer_->last_position_.Validate());
   EXPECT_EQ(Geoposition::ERROR_CODE_NONE,
             observer_->last_position_.error_code);
 
   SetReferencePosition(cell());
 
   EXPECT_TRUE(observer_->last_position_.Validate() ||
               observer_->last_position_.error_code !=
                   Geoposition::ERROR_CODE_NONE);
   EXPECT_EQ(cell()->position_.latitude,
             observer_->last_position_.latitude);
 
   EXPECT_FALSE(cell()->is_permission_granted_);
   EXPECT_FALSE(arbitrator_->HasPermissionBeenGranted());
   arbitrator_->OnPermissionGranted();
   EXPECT_TRUE(arbitrator_->HasPermissionBeenGranted());
   EXPECT_TRUE(cell()->is_permission_granted_);
 }
 
+TEST_F(GeolocationLocationArbitratorTest, OverrideNoNetworkUsage) {

[Wez, 2016/06/10 23:40:35]

nit: Suggest calling this case CustomSystemProviderOnly

[CJ, 2016/06/13 23:45:20]

Done.

+  override_content_browser_client_->SetUseNetwork(false);
+  SetBrowserClientForTesting(override_content_browser_client_.get());
+  ASSERT_TRUE(arbitrator_);
+
+  EXPECT_FALSE(cell());
+  EXPECT_FALSE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
+  arbitrator_->StartProviders(false);
+
+  ASSERT_FALSE(cell());
+  EXPECT_FALSE(gps());
+  EXPECT_TRUE(GetSystemLocationProviderOverride());
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY,
+            GetSystemLocationProviderOverride()->state_);
+  EXPECT_FALSE(observer_->last_position_.Validate());
+  EXPECT_EQ(Geoposition::ERROR_CODE_NONE, observer_->last_position_.error_code);
+
+  SetReferencePosition(GetSystemLocationProviderOverride());
+
+  EXPECT_TRUE(observer_->last_position_.Validate() ||
+              observer_->last_position_.error_code !=
+                  Geoposition::ERROR_CODE_NONE);
+  EXPECT_EQ(GetSystemLocationProviderOverride()->position_.latitude,
+            observer_->last_position_.latitude);
+
+  EXPECT_FALSE(GetSystemLocationProviderOverride()->is_permission_granted_);
+  EXPECT_FALSE(arbitrator_->HasPermissionBeenGranted());
+  arbitrator_->OnPermissionGranted();
+  EXPECT_TRUE(arbitrator_->HasPermissionBeenGranted());
+  EXPECT_TRUE(GetSystemLocationProviderOverride()->is_permission_granted_);
+}
+
+TEST_F(GeolocationLocationArbitratorTest, OverrideAndNetworkUsage) {

[Wez, 2016/06/10 23:40:35]

nit: Suggest CustomSystemAndDefaultNetworkProviders

[CJ, 2016/06/13 23:45:20]

Done.

+  override_content_browser_client_->SetUseNetwork(true);
+  content::SetBrowserClientForTesting(override_content_browser_client_.get());
+  ASSERT_TRUE(arbitrator_);
+
+  EXPECT_FALSE(cell());
+  EXPECT_FALSE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
+  arbitrator_->StartProviders(false);
+
+  EXPECT_TRUE(access_token_store_->access_token_map_.empty());
+
+  access_token_store_->NotifyDelegateTokensLoaded();
+
+  ASSERT_TRUE(cell());
+  EXPECT_FALSE(gps());
+  EXPECT_TRUE(GetSystemLocationProviderOverride());
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY,
+            GetSystemLocationProviderOverride()->state_);
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, cell()->state_);
+  EXPECT_FALSE(observer_->last_position_.Validate());
+  EXPECT_EQ(Geoposition::ERROR_CODE_NONE, observer_->last_position_.error_code);
+
+  SetReferencePosition(cell());
+
+  EXPECT_TRUE(observer_->last_position_.Validate() ||
+              observer_->last_position_.error_code !=
+                  Geoposition::ERROR_CODE_NONE);
+  EXPECT_EQ(cell()->position_.latitude, observer_->last_position_.latitude);
+
+  EXPECT_FALSE(cell()->is_permission_granted_);
+  EXPECT_FALSE(arbitrator_->HasPermissionBeenGranted());
+  arbitrator_->OnPermissionGranted();
+  EXPECT_TRUE(arbitrator_->HasPermissionBeenGranted());
+  EXPECT_TRUE(cell()->is_permission_granted_);
+}
+
 TEST_F(GeolocationLocationArbitratorTest, SetObserverOptions) {
   arbitrator_->StartProviders(false);
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(cell());
   ASSERT_TRUE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, cell()->state_);
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, gps()->state_);
   SetReferencePosition(cell());
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, cell()->state_);
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, gps()->state_);
   arbitrator_->StartProviders(true);
   EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, cell()->state_);
   EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, gps()->state_);
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
 }
 
 TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
   arbitrator_->StartProviders(false);
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(cell());
   ASSERT_TRUE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
 
   SetPositionFix(cell(), 1, 2, 150);
 
   // First position available
   EXPECT_TRUE(observer_->last_position_.Validate());
   CheckLastPositionInfo(1, 2, 150);
 
   SetPositionFix(gps(), 3, 4, 50);
 
   // More accurate fix available
@@ skipping 56 matching lines @@
   // Arrive in station. Cell moves but GPS is stale. Switch to fresher cell.
   SetPositionFix(cell(), 3.5658700, 139.069979, 1000);
   CheckLastPositionInfo(3.5658700, 139.069979, 1000);
 }
 
 TEST_F(GeolocationLocationArbitratorTest, TwoOneShotsIsNewPositionBetter) {
   arbitrator_->StartProviders(false);
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(cell());
   ASSERT_TRUE(gps());
+  EXPECT_FALSE(GetSystemLocationProviderOverride());
 
   // Set the initial position.
   SetPositionFix(cell(), 3, 139, 100);
   CheckLastPositionInfo(3, 139, 100);
 
   // Restart providers to simulate a one-shot request.
   arbitrator_->StopProviders();
 
   // To test 240956, perform a throwaway alloc.
   // This convinces the allocator to put the providers in a new memory location.
-  MockLocationProvider* fakeMockProvider = NULL;
-  LocationProvider* fakeProvider =
-      new MockLocationProvider(&fakeMockProvider);
+  std::unique_ptr<MockLocationProvider> fakeMockProvider(

[Wez, 2016/06/10 23:40:35]

nit: Follow unix_hacker_style naming for variables.

Also, "fake mock provider" is a super confusing name (see comment re mocks and
fakes, above) - since we're modifying this anyway, let's call it dummy_provider,
say?

[CJ, 2016/06/13 23:45:20]

Done.

+      new MockLocationProvider);
 
   arbitrator_->StartProviders(false);
   access_token_store_->NotifyDelegateTokensLoaded();
 
   // Advance the time a short while to simulate successive calls.
   AdvanceTimeNow(base::TimeDelta::FromMilliseconds(5));
 
   // Update with a less accurate position to verify 240956.
   SetPositionFix(cell(), 3, 139, 150);
   CheckLastPositionInfo(3, 139, 150);
-
-  // No delete required for fakeMockProvider. It points to fakeProvider.
-  delete fakeProvider;
 }
 
 }  // namespace content