Checks to see if the position is valid before OnLocationUpdate call.

blimp/client/core/geolocation/geolocation_feature.cc

 // Copyright 2016 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 "blimp/client/core/geolocation/geolocation_feature.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 
@@ skipping 125 matching lines @@
   error->set_error_message(error_message);
 
   am_sending_message_ = true;
   outgoing_message_processor_->ProcessMessage(
       std::move(blimp_message),
       base::Bind(&GeolocationFeature::OnSendComplete, base::Unretained(this)));
 }
 
 void GeolocationFeature::OnSendComplete(int result) {
   am_sending_message_ = false;
-  if (need_to_send_message_) {
-    need_to_send_message_ = false;
-    OnLocationUpdate(location_provider_.get(),
-                     location_provider_->GetPosition());
+  device::Geoposition new_position = location_provider_->GetPosition();
+  if (need_to_send_message_ && new_position.Validate()) {

[Wez, 2016/09/13 20:26:08]

Hmmm, actually isn't Validate() a proxy for whether the LocationProvider is
started? Should we just track whether we started it, and use that, and then
DCHECK that the position is valid? Or are there other reasons it may become
invalid?

[CJ, 2016/09/13 22:45:22]

It has to be this way because even if the LocationProvider is considered
started, it isnt until the first update that we can a non-valid new_position. So
there's an edge case where the LocationProvider is started and has not yet
returned a geoposition update.  

[Wez, 2016/09/16 01:20:27]

But will we see an OnSendComplete() in that case? If we're stopped then
presumably nothing is sending? Or is there a race between start/stop and message
send?

nit: I'd suggest turning the need_to_send_message_ check into an early-exit, so
that you remove the overhead of GetPosition() in the case where there's nothing
to send (though that's fairly minor :).

[CJ, 2016/09/16 18:27:12]

There's that race case. A Stop message gets received on the Client while a
Coordinate message is in flight. Since the Stop message was just a Stop call and
not the result of something getting destructed, the Coordinate message gets sent
just fine, but the Stop call already resets the LocationArbitrator, causing the
cache location to be invalid. 

+    OnLocationUpdate(location_provider_.get(), new_position);
   }
+  need_to_send_message_ = false;
 }
 
 }  // namespace client
 }  // namespace blimp