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

content/browser/geolocation/geolocation_service_impl.h

 // Copyright 2014 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/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_
 
 namespace content {
 
 class GeolocationProvider;
 class GeolocationServiceContext;
 
 // Implements the GeolocationService Mojo interface.
@@ skipping 10 matching lines @@
 
   // Pauses and resumes sending updates to the client of this instance.
   void PauseUpdates();
   void ResumeUpdates();
 
   // Enables and disables geolocation override.
   void SetOverride(const Geoposition& position);
   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);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_