Clean up WifiDataProvider(Manager)

* No need to pass manager in callback
* No need to keep pointer to manager in provider
* Rename test methods Foo to FooForTesting

Review URL: https://codereview.chromium.org/486203004

Cr-Commit-Position: refs/heads/master@{#291209}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291209 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 23 insertions, 35 deletions

diff --git a/content/browser/geolocation/network_location_provider.cc b/content/browser/geolocation/network_location_provider.cc
index 2a48a5a..13a6504 100644
--- a/content/browser/geolocation/network_location_provider.cc
+++ b/content/browser/geolocation/network_location_provider.cc
@@ -154,9 +154,8 @@ void NetworkLocationProvider::OnPermissionGranted() {
   }
 }
 
-void NetworkLocationProvider::OnWifiDataUpdate(
-    WifiDataProviderManager* manager) {
-  DCHECK(manager == wifi_data_provider_manager_);
+void NetworkLocationProvider::OnWifiDataUpdate() {
+  DCHECK(wifi_data_provider_manager_);
   is_wifi_data_complete_ = wifi_data_provider_manager_->GetData(&wifi_data_);
   OnWifiDataUpdated();
 }
diff --git a/content/browser/geolocation/network_location_provider.h b/content/browser/geolocation/network_location_provider.h
index 7d0d6b3..1040887 100644
--- a/content/browser/geolocation/network_location_provider.h
+++ b/content/browser/geolocation/network_location_provider.h
@@ -83,7 +83,7 @@ class NetworkLocationProvider
   void RequestPosition();
 
   // Gets called when new wifi data is available.
-  void OnWifiDataUpdate(WifiDataProviderManager* manager);
+  void OnWifiDataUpdate();
 
   // Internal helper used by OnWifiDataUpdate.
   void OnWifiDataUpdated();
diff --git a/content/browser/geolocation/network_location_provider_unittest.cc b/content/browser/geolocation/network_location_provider_unittest.cc
index f3f5742..caa07b4 100644
--- a/content/browser/geolocation/network_location_provider_unittest.cc
+++ b/content/browser/geolocation/network_location_provider_unittest.cc
@@ -51,7 +51,7 @@ class MessageLoopQuitListener {
 // http://gears.googlecode.com/svn/trunk/gears/geolocation/geolocation_test.cc
 class MockWifiDataProvider : public WifiDataProvider {
  public:
-  // Factory method for use with WifiDataProvider::SetFactory.
+  // Factory method for use with WifiDataProvider::SetFactoryForTesting.
   static WifiDataProvider* GetInstance() {
     CHECK(instance_);
     return instance_;
@@ -117,7 +117,7 @@ class GeolocationNetworkProviderTest : public testing::Test {
     wifi_data_provider_ = MockWifiDataProvider::CreateInstance();
   }
 
-  virtual void TearDown() { WifiDataProviderManager::ResetFactory(); }
+  virtual void TearDown() { WifiDataProviderManager::ResetFactoryForTesting(); }
 
   LocationProvider* CreateProvider(bool set_permission_granted) {
     LocationProvider* provider = NewNetworkLocationProvider(
@@ -134,7 +134,8 @@ class GeolocationNetworkProviderTest : public testing::Test {
   GeolocationNetworkProviderTest() {
     // TODO(joth): Really these should be in SetUp, not here, but they take no
     // effect on Mac OS Release builds if done there. I kid not. Figure out why.
-    WifiDataProviderManager::SetFactory(MockWifiDataProvider::GetInstance);
+    WifiDataProviderManager::SetFactoryForTesting(
+        MockWifiDataProvider::GetInstance);
   }
 
   // Returns the current url fetcher (if any) and advances the id ready for the
diff --git a/content/browser/geolocation/wifi_data_provider.cc b/content/browser/geolocation/wifi_data_provider.cc
index 497747d..cd739e7 100644
--- a/content/browser/geolocation/wifi_data_provider.cc
+++ b/content/browser/geolocation/wifi_data_provider.cc
@@ -7,17 +7,13 @@
 namespace content {
 
 WifiDataProvider::WifiDataProvider()
-    : container_(NULL), client_loop_(base::MessageLoop::current()) {
+    : client_loop_(base::MessageLoop::current()) {
   DCHECK(client_loop_);
 }
 
 WifiDataProvider::~WifiDataProvider() {
 }
 
-void WifiDataProvider::SetContainer(WifiDataProviderManager* container) {
-  container_ = container;
-}
-
 void WifiDataProvider::AddCallback(WifiDataUpdateCallback* callback) {
   callbacks_.insert(callback);
 }
@@ -44,13 +40,13 @@ base::MessageLoop* WifiDataProvider::client_loop() const {
 }
 
 void WifiDataProvider::DoRunCallbacks() {
-  // It's possible that all the callbacks (and the container) went away
-  // whilst this task was pending. This is fine; the loop will be a no-op.
+  // It's possible that all the callbacks went away whilst this task was
+  // pending. This is fine; the loop will be a no-op.
   CallbackSet::const_iterator iter = callbacks_.begin();
   while (iter != callbacks_.end()) {
     WifiDataUpdateCallback* callback = *iter;
     ++iter;  // Advance iter before running, in case callback unregisters.
-    callback->Run(container_);
+    callback->Run();
   }
 }
 
diff --git a/content/browser/geolocation/wifi_data_provider.h b/content/browser/geolocation/wifi_data_provider.h
index 207b3ff..98d2296 100644
--- a/content/browser/geolocation/wifi_data_provider.h
+++ b/content/browser/geolocation/wifi_data_provider.h
@@ -19,8 +19,6 @@
 
 namespace content {
 
-class WifiDataProviderManager;
-
 class CONTENT_EXPORT WifiDataProvider
     : public base::RefCountedThreadSafe<WifiDataProvider> {
  public:
@@ -39,11 +37,7 @@ class CONTENT_EXPORT WifiDataProvider
   // obtain.
   virtual bool GetData(WifiData* data) = 0;
 
-  // Sets the container of this class, which is of type WifiDataProviderManager.
-  // This is required to pass as a parameter when calling a callback.
-  void SetContainer(WifiDataProviderManager* container);
-
-  typedef base::Callback<void(WifiDataProviderManager*)> WifiDataUpdateCallback;
+  typedef base::Closure WifiDataUpdateCallback;
 
   void AddCallback(WifiDataUpdateCallback* callback);
 
@@ -68,8 +62,6 @@ class CONTENT_EXPORT WifiDataProvider
  private:
   void DoRunCallbacks();
 
-  WifiDataProviderManager* container_;
-
   // Reference to the client's message loop. All callbacks should happen in this
   // context.
   base::MessageLoop* client_loop_;
diff --git a/content/browser/geolocation/wifi_data_provider_common_unittest.cc b/content/browser/geolocation/wifi_data_provider_common_unittest.cc
index a9357ba..b6519d6 100644
--- a/content/browser/geolocation/wifi_data_provider_common_unittest.cc
+++ b/content/browser/geolocation/wifi_data_provider_common_unittest.cc
@@ -68,7 +68,7 @@ class MessageLoopQuitter {
     CHECK(message_loop_to_quit_ != NULL);
   }
 
-  void OnWifiDataUpdate(WifiDataProviderManager* manager) {
+  void OnWifiDataUpdate() {
     // Provider should call back on client's thread.
     EXPECT_EQ(base::MessageLoop::current(), message_loop_to_quit_);
     message_loop_to_quit_->QuitNow();
@@ -229,11 +229,12 @@ TEST_F(GeolocationWifiDataProviderCommonTest, MAYBE_DoScanWithResults) {
 
 TEST_F(GeolocationWifiDataProviderCommonTest, RegisterUnregister) {
   MessageLoopQuitter loop_quitter(&main_message_loop_);
-  WifiDataProviderManager::SetFactory(CreateWifiDataProviderCommonWithMock);
+  WifiDataProviderManager::SetFactoryForTesting(
+      CreateWifiDataProviderCommonWithMock);
   WifiDataProviderManager::Register(&loop_quitter.callback_);
   main_message_loop_.Run();
   WifiDataProviderManager::Unregister(&loop_quitter.callback_);
-  WifiDataProviderManager::ResetFactory();
+  WifiDataProviderManager::ResetFactoryForTesting();
 }
 
 }  // namespace content
diff --git a/content/browser/geolocation/wifi_data_provider_manager.cc b/content/browser/geolocation/wifi_data_provider_manager.cc
index 8433212e..cf53657 100644
--- a/content/browser/geolocation/wifi_data_provider_manager.cc
+++ b/content/browser/geolocation/wifi_data_provider_manager.cc
@@ -16,13 +16,13 @@ WifiDataProviderManager::ImplFactoryFunction
     WifiDataProviderManager::factory_function_ = DefaultFactoryFunction;
 
 // static
-void WifiDataProviderManager::SetFactory(
+void WifiDataProviderManager::SetFactoryForTesting(
     ImplFactoryFunction factory_function_in) {
   factory_function_ = factory_function_in;
 }
 
 // static
-void WifiDataProviderManager::ResetFactory() {
+void WifiDataProviderManager::ResetFactoryForTesting() {
   factory_function_ = DefaultFactoryFunction;
 }
 
@@ -65,12 +65,10 @@ WifiDataProviderManager::WifiDataProviderManager() {
   DCHECK(factory_function_);
   impl_ = (*factory_function_)();
   DCHECK(impl_.get());
-  impl_->SetContainer(this);
 }
 
 WifiDataProviderManager::~WifiDataProviderManager() {
   DCHECK(impl_.get());
-  impl_->SetContainer(NULL);
 }
 
 bool WifiDataProviderManager::GetData(WifiData* data) {
diff --git a/content/browser/geolocation/wifi_data_provider_manager.h b/content/browser/geolocation/wifi_data_provider_manager.h
index b91d10c..966c03a 100644
--- a/content/browser/geolocation/wifi_data_provider_manager.h
+++ b/content/browser/geolocation/wifi_data_provider_manager.h
@@ -38,17 +38,18 @@ class WifiDataProvider;
 // Register and Unregister methods.
 class CONTENT_EXPORT WifiDataProviderManager {
  public:
+  typedef WifiDataProvider* (*ImplFactoryFunction)(void);
+
   // Sets the factory function which will be used by Register to create the
   // implementation used by the singleton instance. This factory approach is
   // used both to abstract accross platform-specific implementations and to
   // inject mock implementations for testing.
-  typedef WifiDataProvider* (*ImplFactoryFunction)(void);
-  static void SetFactory(ImplFactoryFunction factory_function_in);
+  static void SetFactoryForTesting(ImplFactoryFunction factory_function_in);
 
   // Resets the factory function to the default.
-  static void ResetFactory();
+  static void ResetFactoryForTesting();
 
-  typedef base::Callback<void(WifiDataProviderManager*)> WifiDataUpdateCallback;
+  typedef base::Closure WifiDataUpdateCallback;
 
   // Registers a callback, which will be run whenever new data is available.
   // Instantiates the singleton if necessary, and always returns it.