Adds EngineGeolocationFeature for Blimp Geolocation project.

blimp/engine/feature/geolocation/blimp_location_provider.cc

Index: blimp/engine/feature/geolocation/blimp_location_provider.cc
diff --git a/blimp/engine/feature/geolocation/blimp_location_provider.cc b/blimp/engine/feature/geolocation/blimp_location_provider.cc
index de86b9fc1e65b31384c8cbdd43cca52128e388ca..cd57693dbe582f693ed08ef95514fd8e8abbd0f1 100644
--- a/blimp/engine/feature/geolocation/blimp_location_provider.cc
+++ b/blimp/engine/feature/geolocation/blimp_location_provider.cc
@@ -4,23 +4,43 @@
 
 #include "blimp/engine/feature/geolocation/blimp_location_provider.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/weak_ptr.h"
 #include "content/public/common/geoposition.h"
 
 namespace blimp {
 namespace engine {
 
-BlimpLocationProvider::BlimpLocationProvider() {}
+BlimpLocationProvider::BlimpLocationProvider(
+    base::WeakPtr<BlimpLocationProvider::Delegate> delegate) {
+  DCHECK(delegate);
+
+  delegate_ = delegate;
+}
 
 BlimpLocationProvider::~BlimpLocationProvider() {
   StopProvider();

[Wez, 2016/07/18 23:55:21]

What's the rationale for removing this call? Don't we want to stop getting
location info if there is no LocationProvider?

nit: This also reminds me that we are assuming that there is at-most-one
BlimpLocationProvider active at any time (i.e. that the content layer will not
create several running in parallel for some reason). We should think about
whether to DCHECK for that somehow.

[CJ, 2016/07/19 20:04:17]

Due to my misunderstanding on how EXPECT_CALL worked, I removed it. It's back
now, along with proper EXPECT_CALL's. 

Would that be handled here or rather in the LocationArbitrator?

 }
 
 bool BlimpLocationProvider::StartProvider(bool high_accuracy) {
-  NOTIMPLEMENTED();
-  return true;
+  if (delegate_) {
+    if (high_accuracy) {
+      delegate_->RequestAccuracy(
+          GeolocationSetInterestLevelMessage::HIGH_ACCURACY);
+    } else {
+      delegate_->RequestAccuracy(
+          GeolocationSetInterestLevelMessage::LOW_ACCURACY);
+    }
+    return true;
+  }
+  return false;
 }
 
 void BlimpLocationProvider::StopProvider() {
+  if (delegate_) {
+    delegate_->RequestAccuracy(GeolocationSetInterestLevelMessage::NO_INTEREST);
+  }
 }
 
 void BlimpLocationProvider::GetPosition(content::Geoposition* position) {
@@ -30,32 +50,18 @@ void BlimpLocationProvider::GetPosition(content::Geoposition* position) {
 }
 
 void BlimpLocationProvider::RequestRefresh() {
-  NOTIMPLEMENTED();
+  if (delegate_) {
+    delegate_->RequestRefresh();
+  }
 }
 
 void BlimpLocationProvider::OnPermissionGranted() {
   RequestRefresh();
-  NOTIMPLEMENTED();
-}
-
-void BlimpLocationProvider::NotifyCallback(
-    const content::Geoposition& position) {
-  DCHECK(!callback_.is_null());
-
-  callback_.Run(this, position);
-}
-
-void BlimpLocationProvider::OnLocationResponse(
-    const content::Geoposition& position) {
-  NotifyCallback(position);
-  NOTIMPLEMENTED();
 }
 
 void BlimpLocationProvider::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
-  DCHECK(!callback.is_null());
-
-  callback_ = callback;
+  delegate_->SetUpdateCallback(base::Bind(callback, base::Unretained(this)));

[Wez, 2016/07/15 01:46:17]

So we are able to assume that the caller will call SetUpdateCallback immediately
after we return this, before delegate_ can become null?

(Not disagreeing, just confirming ;)

[CJ, 2016/07/18 21:11:56]

There shouldn't be any lag between two SetUpdateCallback calls, but perhaps it
would be prudent to add a check to see if the delegate is null in the first
place?

[Wez, 2016/07/18 23:55:21]

That's what I'm getting at - are we able to assume that calling code looks like:

provider = OverrideSystemLocationProvider();
provider->SetUpdateCallback(...);

in which case this is correct, or do we need to support the caller creating the
provider and handing posting a task or something, after which we might find
|delegate_| as gone, before it calls SetUpdateCallback?

[CJ, 2016/07/19 20:04:17]

It seems pretty sequential, but a check was added anyway.
https://cs.chromium.org/chromium/src/content/browser/geolocation/location_arb...

 }
 
 }  // namespace engine