Partially convert geolocation IPC to Mojo.

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
new file mode 100644
index 0000000000000000000000000000000000000000..e0f261728521912c180d26ce5a63c92fbadbf956
--- /dev/null
+++ b/content/browser/geolocation/geolocation_service_impl.h
@@ -0,0 +1,54 @@
+// 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"
+
+#ifndef CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_
+#define CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_
+
+namespace content {
+
+class GeolocationProvider;
+class GeolocationServiceImplContext;
+
+// Implements the GeolocationService Mojo interface.
+class GeolocationServiceImpl : public mojo::InterfaceImpl<GeolocationService> {

[Michael van Ouwerkerk, 2014/10/08 14:13:08]

Who owns this service? How many instances are there?

[blundell, 2014/10/09 08:32:52]

The service was bound to the lifetime of the pipe to which it was connected;
there is one instance of the service per connection requested by the renderer
(i.e., per call to ConnectToRemoteService() that is made from the renderer
side).

I've now changed the service ownership model so that it's owned by
GeolocationServiceImplContext, as I think that's easier to understand. The
difference to enable that is using WeakBindToRequest() as opposed to
BindToRequest().

+ public:
+  // |context| is the geolocation context in which created services should
+  // operate. |on_location_update_callback| will be called when services send
+  // location updates to their clients.
+  static void Create(GeolocationServiceImplContext* context,
+                     const base::Closure& on_location_update_callback,

[Michael van Ouwerkerk, 2014/10/08 14:13:08]

Nit: drop the on_ prefix. Or even: location_callback

[blundell, 2014/10/09 08:32:52]

Changed to update_callback.

+                     mojo::InterfaceRequest<GeolocationService> request);
+
+  // Pauses and resumes sending updates to the client of this instance.
+  void PauseUpdates();
+  void ResumeUpdates();
+
+ private:
+  GeolocationServiceImpl(GeolocationServiceImplContext* context,
+                         const base::Closure& on_location_update_callback);
+  virtual ~GeolocationServiceImpl();
+
+  // GeolocationService:
+  virtual void StartUpdating(bool high_accuracy) OVERRIDE;
+
+  void StartListeningForUpdates();
+  void OnLocationUpdate(const Geoposition& position);
+
+  // Must outlive this object.
+  GeolocationServiceImplContext* context_;

[Michael van Ouwerkerk, 2014/10/08 14:13:08]

I'm having a slow day. Service outlives context or vice versa?

[blundell, 2014/10/09 08:32:52]

Before, either could outlive the other, which as you point out, is a confusing
model. I've changed the model so that the context owns the services.

+  scoped_ptr<GeolocationProvider::Subscription> geolocation_subscription_;
+  base::Closure on_location_update_callback_;
+
+  // Whether this instance is currently observing location updates with high
+  // accuracy.
+  bool high_accuracy_;
+};

[Michael van Ouwerkerk, 2014/10/08 14:13:08]

Does this need DISALLOW_COPY_AND_ASSIGN?

[blundell, 2014/10/09 08:32:52]

Done.

+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_GEOLOCATION_GEOLOCATION_SERVICE_IMPL_H_