Stop using [Client=...] feature of Mojom for GeolocationService

content/browser/geolocation/geolocation_service_impl.h

Index: content/browser/geolocation/geolocation_service_impl.h
diff --git a/content/browser/geolocation/geolocation_service_impl.h b/content/browser/geolocation/geolocation_service_impl.h
index 172e4e196c8feb9b6d612811205f0e9a1e4226c6..16d4a3c53d14bcbdf21fb7c21ef94f4bfe26ff65 100644
--- a/content/browser/geolocation/geolocation_service_impl.h
+++ b/content/browser/geolocation/geolocation_service_impl.h
@@ -5,6 +5,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/geolocation/geolocation_provider_impl.h"
 #include "content/common/geolocation_service.mojom.h"
+#include "content/public/common/mojo_geoposition.mojom.h"
 
 #ifndef CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_
 #define CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_
@@ -35,27 +36,37 @@ class GeolocationServiceImpl : public mojo::InterfaceImpl<GeolocationService> {
   void ClearOverride();
 
  private:
+  typedef mojo::Callback<void(MojoGeopositionPtr)> PositionCallback;
+
   // GeolocationService:
   void SetHighAccuracy(bool high_accuracy) override;
+  void QueryNextPosition(const PositionCallback& callback) override;
 
   // mojo::InterfaceImpl:
   void OnConnectionError() override;
 
   void OnLocationUpdate(const Geoposition& position);
+  void ReportCurrentPosition();
 
   // Owns this object.
   GeolocationServiceContext* context_;
   scoped_ptr<GeolocationProvider::Subscription> geolocation_subscription_;
   base::Closure update_callback_;

[blundell, 2015/01/09 07:37:25]

I should have documented this variable. Could you do it now, something like

"Callback that allows the instantiator of this class to be notified on position
updates".

 
+  std::vector<PositionCallback> position_callbacks_;

[blundell, 2015/01/09 07:37:26]

Hmm, we now have |update_callback_| and |position_callbacks_|, which seems
confusing (at least it was confusing to me, and I wrote this code :). What about
|delegate_update_callback_| and |one_shot_update_callbacks_| or
|client_update_callbacks_| or |query_update_callbacks_|?

Related, could you add documentation to this variable?

+
   // Valid iff SetOverride() has been called and ClearOverride() has not
   // subsequently been called.
   Geoposition position_override_;
 
+  MojoGeoposition current_position_;
+
   // Whether this instance is currently observing location updates with high
   // accuracy.
   bool high_accuracy_;
 
+  bool has_position_to_report_;
+
   DISALLOW_COPY_AND_ASSIGN(GeolocationServiceImpl);
 };