Adds EngineGeolocationFeature for Blimp Geolocation project.

blimp/engine/feature/geolocation/engine_geolocation_feature.cc

Index: blimp/engine/feature/geolocation/engine_geolocation_feature.cc
diff --git a/blimp/engine/feature/geolocation/engine_geolocation_feature.cc b/blimp/engine/feature/geolocation/engine_geolocation_feature.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d4949f38651b3c392a0a66eab1758db4b1d73525
--- /dev/null
+++ b/blimp/engine/feature/geolocation/engine_geolocation_feature.cc
@@ -0,0 +1,130 @@
+// 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.
+
+#include "blimp/common/create_blimp_message.h"
+#include "blimp/common/proto/blimp_message.pb.h"
+#include "blimp/common/proto/geolocation.pb.h"
+#include "blimp/engine/feature/geolocation/engine_geolocation_feature.h"
+#include "content/public/common/geoposition.h"
+
+namespace blimp {
+namespace engine {
+
+void EngineGeolocationFeature::set_geolocation_message_sender(
+    std::unique_ptr<BlimpMessageProcessor> message_processor) {
+  DCHECK(message_processor);
+  geolocation_message_sender_ = std::move(message_processor);
+}
+
+void EngineGeolocationFeature::SendUpdateListenStateMessage(
+    bool enable_high_accuracy) {
+  GeolocationMessage* geolocation_message;
+  std::unique_ptr<BlimpMessage> blimp_message =
+      CreateBlimpMessage(&geolocation_message);
+
+  UpdateListenStateMessage* details =
+      geolocation_message->mutable_update_listen_state();
+  (enable_high_accuracy)

[Kevin M, 2016/06/27 16:57:41]

Use conventional if/else syntax.

[CJ, 2016/06/27 22:04:26]

Done.

+      ? details->set_listen_state(UpdateListenStateMessage::ACCURACY_HIGH)
+      : details->set_listen_state(UpdateListenStateMessage::ACCURACY_LOW);
+
+  geolocation_message_sender_->ProcessMessage(std::move(blimp_message),
+                                              net::CompletionCallback());
+}
+
+void EngineGeolocationFeature::SendStopListenStateMessage() {
+  GeolocationMessage* geolocation_message;
+  std::unique_ptr<BlimpMessage> blimp_message =
+      CreateBlimpMessage(&geolocation_message);
+
+  UpdateListenStateMessage* details =
+      geolocation_message->mutable_update_listen_state();
+  details->set_listen_state(UpdateListenStateMessage::STOPPED);
+
+  geolocation_message_sender_->ProcessMessage(std::move(blimp_message),
+                                              net::CompletionCallback());
+}
+
+void EngineGeolocationFeature::SendRequestRefreshMessage() {
+  GeolocationMessage* geolocation_message;
+  std::unique_ptr<BlimpMessage> blimp_message =
+      CreateBlimpMessage(&geolocation_message);
+
+  geolocation_message->mutable_request_refresh();
+
+  geolocation_message_sender_->ProcessMessage(std::move(blimp_message),
+                                              net::CompletionCallback());
+}
+
+void EngineGeolocationFeature::SetDelegate(
+    GeolocationMessageDelegate* delegate) {
+  DCHECK(delegate);

[Kevin M, 2016/06/27 16:57:41]

Also check !delegate_ ?

[CJ, 2016/06/27 22:04:26]

Done.

+
+  delegate_ = delegate;
+}
+
+void EngineGeolocationFeature::ProcessMessage(
+    std::unique_ptr<BlimpMessage> message,
+    const net::CompletionCallback& callback) {
+  DCHECK(!callback.is_null());
+  DCHECK(message->feature_case() == BlimpMessage::kGeolocation);

[Kevin M, 2016/06/27 16:57:41]

DCHECK_EQ(expected, actual)

[CJ, 2016/06/27 22:04:26]

Done.

+  DCHECK(delegate_);
+
+  GeolocationMessage geolocation_message = message->geolocation();

[Kevin M, 2016/06/27 16:57:41]

Use "const GeolocationMessage&" instead, so that you don't make a copy here.

[CJ, 2016/06/27 22:04:26]

Done.

+  content::Geoposition geoposition;

[Kevin M, 2016/06/27 16:57:41]

"output" ?

[CJ, 2016/06/27 22:04:26]

Done.

+
+  switch (geolocation_message.type_case()) {
+    case GeolocationMessage::kLocation: {
+      LocationMessage location = geolocation_message.location();
+
+      geoposition.latitude = location.latitude();

[Kevin M, 2016/06/27 16:57:41]

Put this in an anonymous namespace method. Too much disjoint LoC in a switch
statement makes code hard to decipher.

[CJ, 2016/06/27 22:04:26]

Done.

+      geoposition.longitude = location.longitude();
+      geoposition.altitude = location.altitude();
+      geoposition.accuracy = location.accuracy();
+      geoposition.altitude_accuracy = location.altitude_accuracy();
+      geoposition.heading = location.heading();
+      geoposition.speed = location.speed();
+      geoposition.timestamp =
+          base::Time::FromJsTime(location.timestamp_millis());

[Kevin M, 2016/06/27 16:57:41]

DCHECK if geoposition is valid (using Geoposition::Validate())

[CJ, 2016/06/27 22:04:26]

This already gets handled upstream by the LocationArbitratorImpl
https://cs.chromium.org/chromium/src/content/browser/geolocation/location_arb...
. Perhaps it should still be handled there?

+
+      UpdateGeolocation(&geoposition);
+      break;
+    }
+    case GeolocationMessage::kError: {
+      ErrorMessage error_message = geolocation_message.error();

[Kevin M, 2016/06/27 16:57:41]

Store this as a const reference or just refer to the error() field inline

[CJ, 2016/06/27 22:04:26]

Done.

+      switch (error_message.error_code()) {

[Kevin M, 2016/06/27 16:57:41]

Put this in an anonymous namespace method for mapping error codes

[CJ, 2016/06/27 22:04:26]

Done.

+        case ErrorMessage::ERROR_CODE_NONE:
+          geoposition.error_code =
+              content::Geoposition::ErrorCode::ERROR_CODE_NONE;
+          break;
+        case ErrorMessage::ERROR_CODE_PERMISSION_DENIED:
+          geoposition.error_code =
+              content::Geoposition::ErrorCode::ERROR_CODE_PERMISSION_DENIED;
+          break;
+        case ErrorMessage::ERROR_CODE_POSITION_UNAVAILABLE:
+          geoposition.error_code =
+              content::Geoposition::ErrorCode::ERROR_CODE_POSITION_UNAVAILABLE;
+        case ErrorMessage::ERROR_CODE_TIMEOUT:
+          geoposition.error_code =
+              content::Geoposition::ErrorCode::ERROR_CODE_TIMEOUT;
+          break;
+      }
+      geoposition.error_message = error_message.error_message();
+
+      UpdateGeolocation(&geoposition);
+      break;
+    }
+    default:

[Kevin M, 2016/06/27 16:57:41]

Handle TYPE_NOT_SET instead of using "default" so that the enum coverage checker
will work

[CJ, 2016/06/27 22:04:26]

Done.

+      NOTREACHED();
+  }
+}
+
+void EngineGeolocationFeature::UpdateGeolocation(

[Kevin M, 2016/06/27 16:57:41]

This is just a one-liner method whose implementation body is pretty trivial;
recommend just calling the delegate directly instead of going through a helper
function.

[CJ, 2016/06/27 22:04:26]

Done.

+    content::Geoposition* geoposition) {

[Kevin M, 2016/06/27 16:57:41]

Take a const reference instead of a pointer here. Pointers are only used for
read/write objects or in cases where the pointer is meant to be held beyond the
lifetime of the function call.

[CJ, 2016/06/27 22:04:26]

Done.

+  DCHECK(delegate_);
+  delegate_->OnLocationResponse(*geoposition);
+}
+
+}  // namespace engine
+}  // namespace blimp