base::Bind: Cleanup in geolocation.

BUG=none
TEST=none

R=csilv@chromium.org

Review URL: http://codereview.chromium.org/8214001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104774 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 79 insertions, 66 deletions

diff --git a/content/browser/geolocation/core_location_data_provider_mac.mm b/content/browser/geolocation/core_location_data_provider_mac.mm
index b198cbd..09e7cf3 100644
--- a/content/browser/geolocation/core_location_data_provider_mac.mm
+++ b/content/browser/geolocation/core_location_data_provider_mac.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -11,6 +11,7 @@
 
 #include "content/browser/geolocation/core_location_data_provider_mac.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "content/browser/geolocation/core_location_provider_mac.h"
@@ -206,24 +207,25 @@ bool CoreLocationDataProviderMac::
   DCHECK(!provider_) << "StartUpdating called twice";
   if (![wrapper_ locationDataAvailable]) return false;
   provider_ = provider;
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-     NewRunnableMethod(this, &CoreLocationDataProviderMac::StartUpdatingTask));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&CoreLocationDataProviderMac::StartUpdatingTask, this));
   return true;
 }
 
 // Clears provider_ so that any leftover messages from CoreLocation get ignored
 void CoreLocationDataProviderMac::StopUpdating() {
   provider_ = NULL;
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-              NewRunnableMethod(this,
-                               &CoreLocationDataProviderMac::StopUpdatingTask));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&CoreLocationDataProviderMac::StopUpdatingTask, this));
 }
 
 void CoreLocationDataProviderMac::UpdatePosition(Geoposition *position) {
-  GeolocationProvider::GetInstance()->message_loop()->PostTask(FROM_HERE,
-              NewRunnableMethod(this,
-                               &CoreLocationDataProviderMac::PositionUpdated,
-                               *position));
+  GeolocationProvider::GetInstance()->message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&CoreLocationDataProviderMac::PositionUpdated, this,
+                 *position));
 }
 
 // Runs in BrowserThread::UI
diff --git a/content/browser/geolocation/device_data_provider.h b/content/browser/geolocation/device_data_provider.h
index 64e52ad..ba0fdc4 100644
--- a/content/browser/geolocation/device_data_provider.h
+++ b/content/browser/geolocation/device_data_provider.h
@@ -28,6 +28,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
@@ -193,8 +194,9 @@ class DeviceDataProviderImplBase : public DeviceDataProviderImplBaseHack {
   void NotifyListeners() {
     // Always make the nitofy callback via a posted task, se we can unwind
     // callstack here and make callback without causing client re-entrancy.
-    client_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
-        &DeviceDataProviderImplBase<DataType>::NotifyListenersInClientLoop));
+    client_loop_->PostTask(FROM_HERE, base::Bind(
+        &DeviceDataProviderImplBase<DataType>::NotifyListenersInClientLoop,
+        this));
   }
 
   bool CalledOnClientThread() const {
diff --git a/content/browser/geolocation/geolocation_provider.cc b/content/browser/geolocation/geolocation_provider.cc
index 582b28a..8090ddf 100644
--- a/content/browser/geolocation/geolocation_provider.cc
+++ b/content/browser/geolocation/geolocation_provider.cc
@@ -4,14 +4,12 @@
 
 #include "content/browser/geolocation/geolocation_provider.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/memory/singleton.h"
 #include "base/threading/thread_restrictions.h"
 #include "content/browser/geolocation/location_arbitrator.h"
 
-// This class is guaranteed to outlive its internal thread, so ref counting
-// is not required.
-DISABLE_RUNNABLE_METHOD_REFCOUNT(GeolocationProvider);
-
 GeolocationProvider* GeolocationProvider::GetInstance() {
   return Singleton<GeolocationProvider>::get();
 }
@@ -46,22 +44,24 @@ bool GeolocationProvider::RemoveObserver(GeolocationObserver* observer) {
 
 void GeolocationProvider::OnObserversChanged() {
   DCHECK(OnClientThread());
-  Task* task = NULL;
+  base::Closure task;
   if (observers_.empty()) {
     DCHECK(IsRunning());
-    task = NewRunnableMethod(this, &GeolocationProvider::StopProviders);
+    task = base::Bind(&GeolocationProvider::StopProviders,
+                      base::Unretained(this));
   } else {
     if (!IsRunning()) {
       Start();
       if (HasPermissionBeenGranted())
         InformProvidersPermissionGranted(most_recent_authorized_frame_);
     }
+
     // The high accuracy requirement may have changed.
-    task = NewRunnableMethod(
-        this,
-        &GeolocationProvider::StartProviders,
-        GeolocationObserverOptions::Collapse(observers_));
+    task = base::Bind(&GeolocationProvider::StartProviders,
+                      base::Unretained(this),
+                      GeolocationObserverOptions::Collapse(observers_));
   }
+
   message_loop()->PostTask(FROM_HERE, task);
 }
 
@@ -103,11 +103,10 @@ void GeolocationProvider::InformProvidersPermissionGranted(
   DCHECK(IsRunning());
   DCHECK(requesting_frame.is_valid());
   if (!OnGeolocationThread()) {
-    Task* task = NewRunnableMethod(
-        this,
-        &GeolocationProvider::InformProvidersPermissionGranted,
-        requesting_frame);
-    message_loop()->PostTask(FROM_HERE, task);
+    message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&GeolocationProvider::InformProvidersPermissionGranted,
+                   base::Unretained(this), requesting_frame));
     return;
   }
   DCHECK(OnGeolocationThread());
@@ -129,10 +128,10 @@ void GeolocationProvider::CleanUp() {
 
 void GeolocationProvider::OnLocationUpdate(const Geoposition& position) {
   DCHECK(OnGeolocationThread());
-  Task* task = NewRunnableMethod(this,
-                                 &GeolocationProvider::NotifyObservers,
-                                 position);
-  client_loop_->PostTask(FROM_HERE, task);
+  client_loop_->PostTask(
+      FROM_HERE,
+      base::Bind(&GeolocationProvider::NotifyObservers,
+                 base::Unretained(this), position));
 }
 
 bool GeolocationProvider::HasPermissionBeenGranted() const {
diff --git a/content/browser/geolocation/gps_location_provider_linux.cc b/content/browser/geolocation/gps_location_provider_linux.cc
index 4b5f787..52c8050 100644
--- a/content/browser/geolocation/gps_location_provider_linux.cc
+++ b/content/browser/geolocation/gps_location_provider_linux.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <cmath>
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -46,7 +47,7 @@ bool PositionsDifferSiginificantly(const Geoposition& position_1,
 
 GpsLocationProviderLinux::GpsLocationProviderLinux(LibGpsFactory libgps_factory)
     : libgps_factory_(libgps_factory),
-      ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK(libgps_factory_);
 }
 
@@ -59,7 +60,7 @@ bool GpsLocationProviderLinux::StartProvider(bool high_accuracy) {
     return true;  // Not an error condition, so still return true.
   }
   if (gps_ != NULL) {
-    DCHECK(!task_factory_.empty());
+    DCHECK(weak_factory_.HasWeakPtrs());
     return true;
   }
   position_.error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
@@ -73,7 +74,7 @@ bool GpsLocationProviderLinux::StartProvider(bool high_accuracy) {
 }
 
 void GpsLocationProviderLinux::StopProvider() {
-  task_factory_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
   gps_.reset();
 }
 
@@ -115,10 +116,11 @@ void GpsLocationProviderLinux::DoGpsPollTask() {
 }
 
 void GpsLocationProviderLinux::ScheduleNextGpsPoll(int interval) {
-  task_factory_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      task_factory_.NewRunnableMethod(&GpsLocationProviderLinux::DoGpsPollTask),
+      base::Bind(&GpsLocationProviderLinux::DoGpsPollTask,
+                 weak_factory_.GetWeakPtr()),
       interval);
 }
 
diff --git a/content/browser/geolocation/gps_location_provider_linux.h b/content/browser/geolocation/gps_location_provider_linux.h
index 7778620..0d19991 100644
--- a/content/browser/geolocation/gps_location_provider_linux.h
+++ b/content/browser/geolocation/gps_location_provider_linux.h
@@ -12,6 +12,7 @@
 #pragma once
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/task.h"
 #include "content/browser/geolocation/location_provider.h"
 #include "content/common/content_export.h"
@@ -51,7 +52,7 @@ class CONTENT_EXPORT GpsLocationProviderLinux : public LocationProviderBase {
   Geoposition position_;
 
   // Holder for the tasks which run on the thread; takes care of cleanup.
-  ScopedRunnableMethodFactory<GpsLocationProviderLinux> task_factory_;
+  base::WeakPtrFactory<GpsLocationProviderLinux> weak_factory_;
 };
 
 #endif  // CONTENT_BROWSER_GEOLOCATION_GPS_LOCATION_PROVIDER_LINUX_H_
diff --git a/content/browser/geolocation/mock_location_provider.cc b/content/browser/geolocation/mock_location_provider.cc
index 4a89613..22b4c30 100644
--- a/content/browser/geolocation/mock_location_provider.cc
+++ b/content/browser/geolocation/mock_location_provider.cc
@@ -7,16 +7,14 @@
 
 #include "content/browser/geolocation/mock_location_provider.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/task.h"
 
-// The provider will always be destroyed before the thread it runs on, so ref
-// counting is not required.
-DISABLE_RUNNABLE_METHOD_REFCOUNT(MockLocationProvider);
-
 MockLocationProvider* MockLocationProvider::instance_ = NULL;
 
 MockLocationProvider::MockLocationProvider(MockLocationProvider** self_ref)
@@ -40,9 +38,10 @@ void MockLocationProvider::HandlePositionChanged(const Geoposition& position) {
     position_ = position;
     UpdateListeners();
   } else {
-    Task* task = NewRunnableMethod(
-        this, &MockLocationProvider::HandlePositionChanged, position);
-    provider_loop_->PostTask(FROM_HERE, task);
+    provider_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&MockLocationProvider::HandlePositionChanged,
+                   base::Unretained(this), position));
   }
 }
 
@@ -71,7 +70,7 @@ class AutoMockLocationProvider : public MockLocationProvider {
   AutoMockLocationProvider(bool has_valid_location,
                            bool requires_permission_to_start)
       : MockLocationProvider(&instance_),
-        ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)),
+        ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
         requires_permission_to_start_(requires_permission_to_start),
         listeners_updated_(false) {
     if (has_valid_location) {
@@ -104,12 +103,13 @@ class AutoMockLocationProvider : public MockLocationProvider {
     if (!listeners_updated_) {
       listeners_updated_ = true;
       MessageLoop::current()->PostTask(
-          FROM_HERE, task_factory_.NewRunnableMethod(
-          &MockLocationProvider::HandlePositionChanged, position_));
+          FROM_HERE,
+          base::Bind(&MockLocationProvider::HandlePositionChanged,
+                     weak_factory_.GetWeakPtr(), position_));
     }
   }
 
-  ScopedRunnableMethodFactory<MockLocationProvider> task_factory_;
+  base::WeakPtrFactory<MockLocationProvider> weak_factory_;
   const bool requires_permission_to_start_;
   bool listeners_updated_;
 };
diff --git a/content/browser/geolocation/network_location_provider.cc b/content/browser/geolocation/network_location_provider.cc
index 28913d8..3041300 100644
--- a/content/browser/geolocation/network_location_provider.cc
+++ b/content/browser/geolocation/network_location_provider.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/geolocation/network_location_provider.h"
 
+#include "base/bind.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "content/browser/geolocation/access_token_store.h"
@@ -114,7 +115,7 @@ NetworkLocationProvider::NetworkLocationProvider(
       is_wifi_data_complete_(false),
       access_token_(access_token),
       is_new_data_available_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(delayed_start_task_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   // Create the position cache.
   position_cache_.reset(new PositionCache());
 
@@ -135,7 +136,7 @@ void NetworkLocationProvider::UpdatePosition() {
   // TODO(joth): When called via the public (base class) interface, this should
   // poke each data provider to get them to expedite their next scan.
   // Whilst in the delayed start, only send request if all data is ready.
-  if (delayed_start_task_.empty() ||
+  if (!weak_factory_.HasWeakPtrs() ||
       (is_radio_data_complete_ && is_wifi_data_complete_)) {
     RequestPosition();
   }
@@ -208,8 +209,8 @@ bool NetworkLocationProvider::StartProvider(bool high_accuracy) {
 
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      delayed_start_task_.NewRunnableMethod(
-          &NetworkLocationProvider::RequestPosition),
+      base::Bind(&NetworkLocationProvider::RequestPosition,
+                 weak_factory_.GetWeakPtr()),
       kDataCompleteWaitPeriod);
   // Get the device data.
   is_radio_data_complete_ = radio_data_provider_->GetData(&radio_data_);
@@ -227,7 +228,7 @@ void NetworkLocationProvider::StopProvider() {
   }
   radio_data_provider_ = NULL;
   wifi_data_provider_ = NULL;
-  delayed_start_task_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 // Other methods
@@ -257,7 +258,7 @@ void NetworkLocationProvider::RequestPosition() {
   if (most_recent_authorized_host_.empty())
     return;
 
-  delayed_start_task_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
   is_new_data_available_ = false;
 
   // TODO(joth): Rather than cancel pending requests, we should create a new
diff --git a/content/browser/geolocation/network_location_provider.h b/content/browser/geolocation/network_location_provider.h
index 738dca8..dab8293 100644
--- a/content/browser/geolocation/network_location_provider.h
+++ b/content/browser/geolocation/network_location_provider.h
@@ -12,6 +12,7 @@
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "base/threading/thread.h"
 #include "content/browser/geolocation/device_data_provider.h"
@@ -127,7 +128,7 @@ class NetworkLocationProvider
   // The network location request object, and the url it uses.
   scoped_ptr<NetworkLocationRequest> request_;
 
-  ScopedRunnableMethodFactory<NetworkLocationProvider> delayed_start_task_;
+  base::WeakPtrFactory<NetworkLocationProvider> weak_factory_;
   // The cache of positions.
   scoped_ptr<PositionCache> position_cache_;
 
diff --git a/content/browser/geolocation/wifi_data_provider_common.cc b/content/browser/geolocation/wifi_data_provider_common.cc
index 7f92ece..16c11df 100644
--- a/content/browser/geolocation/wifi_data_provider_common.cc
+++ b/content/browser/geolocation/wifi_data_provider_common.cc
@@ -1,9 +1,10 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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 "content/browser/geolocation/wifi_data_provider_common.h"
 
+#include "base/bind.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 
@@ -24,7 +25,7 @@ string16 MacAddressAsString16(const uint8 mac_as_int[6]) {
 WifiDataProviderCommon::WifiDataProviderCommon()
     : Thread("Geolocation_wifi_provider"),
       is_first_scan_complete_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
 WifiDataProviderCommon::~WifiDataProviderCommon() {
@@ -102,6 +103,7 @@ void WifiDataProviderCommon::DoWifiScanTask() {
 void WifiDataProviderCommon::ScheduleNextScan(int interval) {
   message_loop()->PostDelayedTask(
       FROM_HERE,
-      task_factory_.NewRunnableMethod(&WifiDataProviderCommon::DoWifiScanTask),
+      base::Bind(&WifiDataProviderCommon::DoWifiScanTask,
+                 weak_factory_.GetWeakPtr()),
       interval);
 }
diff --git a/content/browser/geolocation/wifi_data_provider_common.h b/content/browser/geolocation/wifi_data_provider_common.h
index 1f11ef8..df3d681 100644
--- a/content/browser/geolocation/wifi_data_provider_common.h
+++ b/content/browser/geolocation/wifi_data_provider_common.h
@@ -10,6 +10,7 @@
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "base/task.h"
 #include "base/threading/thread.h"
@@ -119,7 +120,7 @@ class CONTENT_EXPORT WifiDataProviderCommon
   scoped_ptr<PollingPolicyInterface> polling_policy_;
 
   // Holder for the tasks which run on the thread; takes care of cleanup.
-  ScopedRunnableMethodFactory<WifiDataProviderCommon> task_factory_;
+  base::WeakPtrFactory<WifiDataProviderCommon> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WifiDataProviderCommon);
 };
diff --git a/content/browser/geolocation/win7_location_provider_win.cc b/content/browser/geolocation/win7_location_provider_win.cc
index 2e5c487..3a3c6aeb 100644
--- a/content/browser/geolocation/win7_location_provider_win.cc
+++ b/content/browser/geolocation/win7_location_provider_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <cmath>
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -43,7 +44,7 @@ bool PositionsDifferSiginificantly(const Geoposition& position_1,
 }
 
 Win7LocationProvider::Win7LocationProvider(Win7LocationApi* api)
-    : ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
+    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK(api != NULL);
   api_.reset(api);
 }
@@ -56,13 +57,13 @@ bool Win7LocationProvider::StartProvider(bool high_accuracy){
   if (api_ == NULL)
     return false;
   api_->SetHighAccuracy(high_accuracy);
-  if (task_factory_.empty())
+  if (!weak_factory_.HasWeakPtrs())
     ScheduleNextPoll(0);
   return true;
 }
 
 void Win7LocationProvider::StopProvider() {
-  task_factory_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 void Win7LocationProvider::GetPosition(Geoposition* position) {
@@ -94,7 +95,7 @@ void Win7LocationProvider::DoPollTask() {
 void Win7LocationProvider::ScheduleNextPoll(int interval) {
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      task_factory_.NewRunnableMethod(&Win7LocationProvider::DoPollTask),
+      base::Bind(&Win7LocationProvider::DoPollTask, weak_factory_.GetWeakPtr()),
       interval);
 }
 
diff --git a/content/browser/geolocation/win7_location_provider_win.h b/content/browser/geolocation/win7_location_provider_win.h
index ff49494..5439184 100644
--- a/content/browser/geolocation/win7_location_provider_win.h
+++ b/content/browser/geolocation/win7_location_provider_win.h
@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_GEOLOCATION_WIN7_LOCATION_PROVIDER_WIN_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/task.h"
 #include "content/browser/geolocation/location_provider.h"
 #include "content/browser/geolocation/win7_location_api_win.h"
@@ -39,7 +40,7 @@ class CONTENT_EXPORT Win7LocationProvider : public LocationProviderBase {
   scoped_ptr<Win7LocationApi> api_;
   Geoposition position_;
   // Holder for the tasks which run on the thread; takes care of cleanup.
-  ScopedRunnableMethodFactory<Win7LocationProvider> task_factory_;
+  base::WeakPtrFactory<Win7LocationProvider> weak_factory_;
 };
 
 #endif  // CONTENT_BROWSER_GEOLOCATION_WIN7_LOCATION_PROVIDER_WIN_H_