Adds EngineGeolocationFeature for Blimp Geolocation project.

blimp/engine/feature/geolocation/blimp_location_provider.cc

 // Copyright (c) 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/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) {
   DCHECK(position);

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

nit: No need for this, since *position = foo will give a nice clean crash if
position is null. :D

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

Done.

 
   *position = 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
 }  // namespace blimp