Do not send stale position in GeolocationProvider::AddObserver.

content/browser/geolocation/geolocation_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 "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
@@ skipping 44 matching lines @@
   // GeolocationObserver
   virtual void OnLocationUpdate(const Geoposition& position) {}
 };
 
 class MockGeolocationObserver : public GeolocationObserver {
  public:
   // GeolocationObserver
   MOCK_METHOD1(OnLocationUpdate, void(const Geoposition& position));
 };
 
+class AsyncMockGeolocationObserver : public MockGeolocationObserver {
+ public:
+  // GeolocationObserver
+  virtual void OnLocationUpdate(const Geoposition& position) {
+    MockGeolocationObserver::OnLocationUpdate(position);
+    MessageLoop::current()->Quit();
+  }
+};
+
 class MockGeolocationCallbackWrapper {
  public:
   MOCK_METHOD1(Callback, void(const Geoposition& position));
 };
 
 class GeopositionEqMatcher
     : public MatcherInterface<const Geoposition&> {
  public:
   explicit GeopositionEqMatcher(const Geoposition& expected)
       : expected_(expected) {}
@@ skipping 37 matching lines @@
         io_thread_(BrowserThread::IO, &message_loop_),
         provider_(new LocationProviderForTestArbitrator) {
   }
 
   ~GeolocationProviderTest() {}
 
   LocationProviderForTestArbitrator* provider() { return provider_.get(); }
 
   // Called on test thread.
   bool ProvidersStarted();
+  void SendMockLocation(const Geoposition& position);
 
  private:
   // Called on provider thread.
   void GetProvidersStarted(bool* started);
 
   MessageLoop message_loop_;
   TestBrowserThread io_thread_;
   scoped_ptr<LocationProviderForTestArbitrator> provider_;
 };
 
@@ skipping 10 matching lines @@
       MessageLoop::QuitClosure());
   message_loop_.Run();
   return started;
 }
 
 void GeolocationProviderTest::GetProvidersStarted(bool* started) {
   DCHECK(MessageLoop::current() == provider_->message_loop());
   *started = provider_->mock_arbitrator()->providers_started();
 }
 
+void GeolocationProviderTest::SendMockLocation(const Geoposition& position) {
+  DCHECK(provider_->IsRunning());
+  DCHECK(MessageLoop::current() == &message_loop_);
+  provider_->message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&GeolocationProvider::OnLocationUpdate,
+                 base::Unretained(provider_.get()),
+                 position));
+}
+
 
 // Regression test for http://crbug.com/59377
 TEST_F(GeolocationProviderTest, OnPermissionGrantedWithoutObservers) {
   EXPECT_FALSE(provider()->HasPermissionBeenGranted());
   provider()->OnPermissionGranted();
   EXPECT_TRUE(provider()->HasPermissionBeenGranted());
 }
 
 TEST_F(GeolocationProviderTest, StartStop) {
   EXPECT_FALSE(provider()->IsRunning());
   NullGeolocationObserver null_observer;
   GeolocationObserverOptions options;
   provider()->AddObserver(&null_observer, options);
   EXPECT_TRUE(provider()->IsRunning());
   EXPECT_TRUE(ProvidersStarted());
 
   provider()->RemoveObserver(&null_observer);
   EXPECT_FALSE(ProvidersStarted());
   EXPECT_TRUE(provider()->IsRunning());
 }
 
+TEST_F(GeolocationProviderTest, StalePositionNotSent) {
+  Geoposition first_position;
+  first_position.latitude = 12;
+  first_position.longitude = 34;
+  first_position.accuracy = 56;
+  first_position.timestamp = base::Time::Now();
+
+  AsyncMockGeolocationObserver first_observer;
+  GeolocationObserverOptions options;
+  EXPECT_CALL(first_observer, OnLocationUpdate(GeopositionEq(first_position)));
+  provider()->AddObserver(&first_observer, options);
+  SendMockLocation(first_position);
+  MessageLoop::current()->Run();
+
+  provider()->RemoveObserver(&first_observer);
+
+  Geoposition second_position;
+  second_position.latitude = 13;
+  second_position.longitude = 34;
+  second_position.accuracy = 56;
+  second_position.timestamp = base::Time::Now();
+
+  AsyncMockGeolocationObserver second_observer;
+  EXPECT_CALL(second_observer,
+              OnLocationUpdate(GeopositionEq(second_position)));
+  provider()->AddObserver(&second_observer, options);
+  SendMockLocation(second_position);
+  MessageLoop::current()->Run();

[bulach, 2012/11/30 16:49:56]

it looks like this test would work even without the patch? :)
I mean, it will _also_ work with the patch, but maybe we could:

208:
- add a second observer.
- post a quit message
- MessageLoop::current()->Run()
- ensure the second observer did NOT get any update.

then keep 215 / 223 as is?

[John Knottenbelt, 2012/11/30 17:26:48]

Actually, the test does indeed fail without the patch, because the geolocaiton
provider sends through the stale position synchronously with the AddObserver
call, which causes the EXPECT_CALL to mismatch (wrong position sent through).

But yes, I think that your suggestion is good. I'll try and work it in.

On 2012/11/30 16:49:56, bulach wrote:

+
+  provider()->RemoveObserver(&second_observer);
+  EXPECT_FALSE(ProvidersStarted());
+}
+
+
 TEST_F(GeolocationProviderTest, OverrideLocationForTesting) {
   Geoposition position;
   position.error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
   provider()->OverrideLocationForTesting(position);
   // Adding an observer when the location is overridden should synchronously
   // update the observer with our overridden position.
   MockGeolocationObserver mock_observer;
   EXPECT_CALL(mock_observer, OnLocationUpdate(GeopositionEq(position)));
   provider()->AddObserver(&mock_observer, GeolocationObserverOptions());
   provider()->RemoveObserver(&mock_observer);
@@ skipping 11 matching lines @@
   position.longitude = 34;
   position.accuracy = 56;
   position.timestamp = base::Time::Now();
   EXPECT_CALL(callback_wrapper, Callback(GeopositionEq(position)));
   provider()->OverrideLocationForTesting(position);
   // Wait for the providers to be stopped now that all clients are gone.
   EXPECT_FALSE(ProvidersStarted());
 }
 
 }  // namespace content