Adds GeolocationFeature for Blimp Geolocation project.

blimp/client/core/geolocation/geolocation_feature.h

+// 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.
+
+#ifndef BLIMP_CLIENT_CORE_GEOLOCATION_GEOLOCATION_FEATURE_H_
+#define BLIMP_CLIENT_CORE_GEOLOCATION_GEOLOCATION_FEATURE_H_
+
+#include <memory>
+#include <string>
+
+#include "blimp/common/proto/geolocation.pb.h"
+#include "blimp/net/blimp_message_processor.h"
+#include "device/geolocation/geoposition.h"
+#include "device/geolocation/location_provider.h"
+
+namespace blimp {
+namespace client {
+
+// Client-side feature handling geolocation messages.
+class GeolocationFeature : public BlimpMessageProcessor {
+ public:
+  explicit GeolocationFeature(
+      std::unique_ptr<device::LocationProvider> location_provider);
+  ~GeolocationFeature() override;
+
+  // Sets the BlimpMessageProcessor that will be used to send
+  // BlimpMessage::GEOLOCATION messages to the engine.
+  void set_outgoing_message_processor(
+      std::unique_ptr<BlimpMessageProcessor> processor);
+
+  // BlimpMessageProcessor implementation.
+  void ProcessMessage(std::unique_ptr<BlimpMessage> message,
+                      const net::CompletionCallback& callback) override;
+
+ private:
+  // Sends engine an update of the client's geoposition.
+  void OnLocationUpdate(const device::LocationProvider* location_provider,
+                        const device::Geoposition& position);
+
+  // Communicates to the client a change of interest level from the engine.

[Wez, 2016/08/19 18:50:25]

nit: This wording is very similar to the preceding "Sends a..." - whereas the
preceding API triggers an "outgoing" Client->Engine message, this is effectively
a message-handler.

I'd suggest a comment like "Handles a request from the Engine to change the
level of Geolocation information reported to it." or similar.

[CJ, 2016/08/19 22:22:33]

Done.

+  void SetInterestLevel(GeolocationSetInterestLevelMessage::Level level);
+
+  // Creates a GeolocationPositionMessage that reflects the given
+  // geoposition and sends it to the engine.

[Wez, 2016/08/19 18:50:25]

nit: "Creates ... and sends ..." is redundant - suggest just "Sends ..."

[CJ, 2016/08/19 22:22:33]

Done.

+  void SendGeolocationPositionMessage(const device::Geoposition& position);
+
+  // Creates a GeolocationErrorMessage that reflects the error defined
+  // in the given geoposition and sends it.
+  void SendGeolocationErrorMessage(
+      GeolocationErrorMessage::ErrorCode error_code,
+      const std::string& error_message);
+
+  // Callback function when message finishes sending.

[Wez, 2016/08/19 18:50:25]

nit: Grammar; I think you mean "Called when a message send completes"?

[CJ, 2016/08/19 22:22:33]

Done.

+  void OnSendComplete(int result);
+
+  // Used to obtain the client's location.
+  std::unique_ptr<device::LocationProvider> location_provider_;
+
+  // Used to send BlimpMessage::GEOLOCATION message to the engine.
+  std::unique_ptr<BlimpMessageProcessor> outgoing_message_processor_;
+
+  // Run when message is finished sending.

[Wez, 2016/08/19 18:50:25]

Not clear from the context who is sending the message (i.e. outgoing or
incoming..?) - is this just a place-holder for Bind(..., OnSendComplete)?  If so
then I'd suggest calling it on_send_complete_callback_.

However, is there any reason to store this at all? Could we just Bind() it at
the call-site and avoid the need for a member?

[CJ, 2016/08/19 22:22:33]

Good point. Not stored now.

+  net::CompletionCallback completion_callback_;
+
+  // True if a message is in the process of being sent.
+  bool sending_message_ = false;

[Wez, 2016/08/19 18:50:25]

nit: Suggest am_sending_message_

In general you'll see that bools members have very verbose names that make their
role very explicit, and they often start with is_, am_, etc, to make clear that
they are just true/false values when we refer to them.

[CJ, 2016/08/19 22:22:33]

Cool thanks, fixed.

+
+  // True if location has been updated but a message cannot be sent at the
+  // moment.
+  bool need_to_send_message_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(GeolocationFeature);
+};
+
+}  // namespace client
+}  // namespace blimp
+
+#endif  // BLIMP_CLIENT_CORE_GEOLOCATION_GEOLOCATION_FEATURE_H_