Adds EngineGeolocationFeature for Blimp Geolocation project.

blimp/engine/feature/geolocation/blimp_location_provider.h

Index: blimp/engine/feature/geolocation/blimp_location_provider.h
diff --git a/blimp/engine/feature/geolocation/blimp_location_provider.h b/blimp/engine/feature/geolocation/blimp_location_provider.h
index d3d16f22a9041915f79a05dd2465a446497b9479..3bf96636bea649354865aaf4737a1f3862f643dd 100644
--- a/blimp/engine/feature/geolocation/blimp_location_provider.h
+++ b/blimp/engine/feature/geolocation/blimp_location_provider.h
@@ -5,6 +5,8 @@
 #ifndef BLIMP_ENGINE_FEATURE_GEOLOCATION_BLIMP_LOCATION_PROVIDER_H_
 #define BLIMP_ENGINE_FEATURE_GEOLOCATION_BLIMP_LOCATION_PROVIDER_H_
 
+#include "base/memory/weak_ptr.h"
+#include "blimp/common/proto/geolocation.pb.h"
 #include "content/public/browser/location_provider.h"
 #include "content/public/common/geoposition.h"
 
@@ -14,7 +16,21 @@ namespace engine {
 // Location provider for Blimp using the device's provider over the network.
 class BlimpLocationProvider : public content::LocationProvider {
  public:
-  BlimpLocationProvider();
+  // A delegate that handles outgoing geolocation messages.

[Kevin M, 2016/07/19 16:01:41]

nit: messages are N/A at this layer

[CJ, 2016/07/19 20:35:58]

Done.

+  class Delegate {
+   public:
+    virtual ~Delegate() {}
+
+    // Implements subset of LocationProvider's functions.
+    virtual void RequestAccuracy(
+        GeolocationSetInterestLevelMessage::Level level) = 0;
+    virtual void RequestRefresh() = 0;
+
+    virtual void SetUpdateCallback(
+        const base::Callback<void(const content::Geoposition&)>& callback) = 0;

[Kevin M, 2016/07/19 16:01:41]

suggestion: create an alias for the callback type with "using"

[CJ, 2016/07/19 20:35:58]

Done.

+  };
+
+  explicit BlimpLocationProvider(base::WeakPtr<Delegate> delegate);
   ~BlimpLocationProvider() override;
 
   // content::LocationProvider implementation.
@@ -23,15 +39,16 @@ class BlimpLocationProvider : public content::LocationProvider {
   void GetPosition(content::Geoposition* position) override;
   void RequestRefresh() override;
   void OnPermissionGranted() override;
-
- private:
-  void NotifyCallback(const content::Geoposition& position);
-  void OnLocationResponse(const content::Geoposition& position);
   void SetUpdateCallback(
       const LocationProviderUpdateCallback& callback) override;
 
-  LocationProviderUpdateCallback callback_;
+ private:
+  // This delegate handles a subset of the LocationProvider functionality.
+  // The lifetime of the delegate relative to the BlimpLocationProvider

[Kevin M, 2016/07/19 16:01:41]

nit: I think the lifetime comment is already implied by the use of a WeakPtr, as
weak pointers are a defensive measure against this kind of uncertainty. Consider
removing the comment

[CJ, 2016/07/19 20:35:58]

Done.

+  // is unknown.
+  base::WeakPtr<Delegate> delegate_;
 
+  // Needed for GetPosition(), which returns a cached position immediately.
   content::Geoposition position_;

[Kevin M, 2016/07/19 16:01:41]

I think that we might be able to remove the comment if we called this
cached_position_.

[CJ, 2016/07/19 20:35:58]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(BlimpLocationProvider);