Makes use of EngineGeolocationFeature weak_ptr threadsafe.

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 "base/task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "device/geolocation/geoposition.h"
 
 namespace blimp {
 namespace engine {
+namespace {
+
+void InvokeGeopositionCallback(
+    scoped_refptr<base::TaskRunner> task_runner,
+    const BlimpLocationProvider::LocationProviderUpdateCallback& callback,
+    base::WeakPtr<BlimpLocationProvider> provider,
+    const device::Geoposition& geoposition) {
+  if (provider)

[Wez, 2016/09/17 01:47:42]

Isn't InvokeGeopositionCallback called from the delegate_task_runner_, though,
hence why the PostTask() is here?

It's at the point when the task reaches the geolocation thread, where the caller
may have deleted the BlimpLocationProvider in the meantime, that we need to
check this.

[CJ, 2016/09/23 18:03:58]

InvokeGeopositionCallback is called on the geolocation thread. I'm not sure I
follow you.

[Wez, 2016/09/23 22:28:36]

Are you sure? InvokeGepositionCallback is used to create the callback passed to
the BlimpLocationProvider::Delegate, which runs on the delegate task runner, so
presumably will call the calback there. That callback then needs to jump back to
the geolocation thread, via PostTask, to run the LocationProviderUpdateCallback
- but it needs to check that the BlimpLocationProvider still exists before it
runs it.

+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(callback, provider.get(), geoposition));

[Kevin M, 2016/10/06 19:30:52]

Weak pointers must be dereferenced and deleted on the same thread.

In this case, we are dereferencing it on the delegate thread, but we are
deleting the factory elsewhere on the main thread. That'll definitely blow up.
We need to thread here hop BEFORE we poke at |provider|.

+}
+
+}  // namespace
 
 BlimpLocationProvider::BlimpLocationProvider(
-    base::WeakPtr<BlimpLocationProvider::Delegate> delegate)
-    : delegate_(delegate), is_started_(false) {}
+    base::WeakPtr<BlimpLocationProvider::Delegate> delegate,
+    scoped_refptr<base::SequencedTaskRunner> delegate_task_runner)
+    : delegate_(delegate),
+      delegate_task_runner_(delegate_task_runner),
+      is_started_(false),
+      weak_factory_(this) {}
 
 BlimpLocationProvider::~BlimpLocationProvider() {
   if (is_started_) {
     StopProvider();
   }
 }
 
 bool BlimpLocationProvider::StartProvider(bool high_accuracy) {
-  if (delegate_) {
-    if (high_accuracy) {
-      delegate_->RequestAccuracy(
-          GeolocationSetInterestLevelMessage::HIGH_ACCURACY);
-    } else {
-      delegate_->RequestAccuracy(
-          GeolocationSetInterestLevelMessage::LOW_ACCURACY);
-    }
-    is_started_ = true;
-  }
+  is_started_ = true;
+  delegate_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(
+          &BlimpLocationProvider::Delegate::RequestAccuracy, delegate_,
+          (high_accuracy ? GeolocationSetInterestLevelMessage::HIGH_ACCURACY
+                         : GeolocationSetInterestLevelMessage::LOW_ACCURACY)));
   return is_started_;
 }
 
 void BlimpLocationProvider::StopProvider() {
   DCHECK(is_started_);
-  if (delegate_) {
-    delegate_->RequestAccuracy(GeolocationSetInterestLevelMessage::NO_INTEREST);
-    is_started_ = false;
-  }
+  delegate_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&BlimpLocationProvider::Delegate::RequestAccuracy, delegate_,
+                 GeolocationSetInterestLevelMessage::NO_INTEREST));
+  is_started_ = false;
 }
 
 const device::Geoposition& BlimpLocationProvider::GetPosition() {
   return cached_position_;
 }
 
 void BlimpLocationProvider::OnPermissionGranted() {
   DCHECK(is_started_);
-  if (delegate_) {
-    delegate_->OnPermissionGranted();
-  }
+  delegate_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&BlimpLocationProvider::Delegate::OnPermissionGranted,
+                 delegate_));
 }
 
 void BlimpLocationProvider::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
-  if (delegate_) {
-    delegate_->SetUpdateCallback(base::Bind(callback, base::Unretained(this)));
-  }
+  delegate_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&BlimpLocationProvider::Delegate::SetUpdateCallback, delegate_,
+                 base::Bind(&InvokeGeopositionCallback,
+                            base::ThreadTaskRunnerHandle::Get(), callback,
+                            weak_factory_.GetWeakPtr())));
 }
 
 }  // namespace engine
 }  // namespace blimp