Do not send stale position in GeolocationProvider::AddObserver.

content/browser/geolocation/geolocation_provider_unittest.cc

Index: content/browser/geolocation/geolocation_provider_unittest.cc
diff --git a/content/browser/geolocation/geolocation_provider_unittest.cc b/content/browser/geolocation/geolocation_provider_unittest.cc
index 99649adc56e4aa54cb18b2c2cc57f1fe1f10412d..d7fc98f990a226d6527fce36ac7de6b50af7e7bf 100644
--- a/content/browser/geolocation/geolocation_provider_unittest.cc
+++ b/content/browser/geolocation/geolocation_provider_unittest.cc
@@ -62,6 +62,15 @@ class MockGeolocationObserver : 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));
@@ -119,6 +128,7 @@ class GeolocationProviderTest : public testing::Test {
 
   // Called on test thread.
   bool ProvidersStarted();
+  void SendMockLocation(const Geoposition& position);
 
  private:
   // Called on provider thread.
@@ -149,6 +159,16 @@ void GeolocationProviderTest::GetProvidersStarted(bool* started) {
   *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) {
@@ -170,6 +190,40 @@ TEST_F(GeolocationProviderTest, StartStop) {
   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;