Revise DiscoveryNetworkMonitor testing

This change lets other test files avoid using the
DiscoveryNetworkMonitor singleton and converts TestBrowserThreadBundle
to ScopedTaskEnvironment.

Bug: 698943
Change-Id: Ib69cd42901b739e651a87cafa2ca4a2e214c48e2
Reviewed-on: https://chromium-review.googlesource.com/595017
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491550}

chrome/browser/media/router/discovery/discovery_network_monitor.cc

@@ -52,13 +52,12 @@ DiscoveryNetworkMonitor* DiscoveryNetworkMonitor::GetInstance() {
   return g_discovery_monitor.Pointer();
 }
 
-void DiscoveryNetworkMonitor::RebindNetworkChangeObserverForTest() {
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
-}
-
-void DiscoveryNetworkMonitor::SetNetworkInfoFunctionForTest(
-    NetworkInfoFunction strategy) {
-  network_info_function_ = strategy;
+// static
+std::unique_ptr<DiscoveryNetworkMonitor>
+DiscoveryNetworkMonitor::CreateInstanceForTest(NetworkInfoFunction strategy) {
+  auto* discovery_network_monitor = new DiscoveryNetworkMonitor();
+  discovery_network_monitor->SetNetworkInfoFunctionForTest(std::move(strategy));
+  return std::unique_ptr<DiscoveryNetworkMonitor>(discovery_network_monitor);
 }
 
 void DiscoveryNetworkMonitor::AddObserver(Observer* const observer) {
@@ -100,6 +99,11 @@ DiscoveryNetworkMonitor::~DiscoveryNetworkMonitor() {
   net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
 }
 
+void DiscoveryNetworkMonitor::SetNetworkInfoFunctionForTest(
+    NetworkInfoFunction strategy) {
+  network_info_function_ = strategy;
+}
+
 void DiscoveryNetworkMonitor::OnNetworkChanged(
     net::NetworkChangeNotifier::ConnectionType) {
   task_runner_->PostTask(

chrome/browser/media/router/discovery/discovery_network_monitor.h

@@ -50,8 +50,8 @@ class DiscoveryNetworkMonitor
 
   static DiscoveryNetworkMonitor* GetInstance();
 
-  void RebindNetworkChangeObserverForTest();
-  void SetNetworkInfoFunctionForTest(NetworkInfoFunction);
+  static std::unique_ptr<DiscoveryNetworkMonitor> CreateInstanceForTest(
+      NetworkInfoFunction strategy);
 
   void AddObserver(Observer* const observer);
   void RemoveObserver(Observer* const observer);
@@ -67,12 +67,14 @@ class DiscoveryNetworkMonitor
   void GetNetworkId(NetworkIdCallback callback);
 
  private:
-  friend class DiscoveryNetworkMonitorTest;
+  friend struct std::default_delete<DiscoveryNetworkMonitor>;
   friend struct base::LazyInstanceTraitsBase<DiscoveryNetworkMonitor>;
 
   DiscoveryNetworkMonitor();
   ~DiscoveryNetworkMonitor() override;
 
+  void SetNetworkInfoFunctionForTest(NetworkInfoFunction strategy);
+
   // net::NetworkChangeNotifier::NetworkChangeObserver
   void OnNetworkChanged(
       net::NetworkChangeNotifier::ConnectionType type) override;

chrome/browser/media/router/discovery/discovery_network_monitor_unittest.cc

@@ -6,18 +6,13 @@
 
 #include <memory>
 
-#include "base/run_loop.h"
 #include "base/task_scheduler/task_scheduler.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/test/test_browser_thread_bundle.h"
-#include "content/public/test/test_utils.h"
+#include "base/test/scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
-using content::BrowserThread;
-
 using testing::Invoke;
 using testing::_;
 
@@ -30,19 +25,13 @@ class MockDiscoveryObserver : public DiscoveryNetworkMonitor::Observer {
 
 class DiscoveryNetworkMonitorTest : public testing::Test {
  protected:
-  void SetUp() override {
-    fake_network_info.clear();
-    discovery_network_monitor->SetNetworkInfoFunctionForTest(
-        &FakeGetNetworkInfo);
-  }
-
-  void TearDown() override { delete discovery_network_monitor; }
+  void SetUp() override { fake_network_info.clear(); }
 
   static std::vector<DiscoveryNetworkInfo> FakeGetNetworkInfo() {
     return fake_network_info;
   }
 
-  content::TestBrowserThreadBundle test_browser_thread_bundle;
+  base::test::ScopedTaskEnvironment scoped_task_environment;
   MockDiscoveryObserver mock_observer;
 
   std::vector<DiscoveryNetworkInfo> fake_ethernet_info{
@@ -55,10 +44,8 @@ class DiscoveryNetworkMonitorTest : public testing::Test {
       base::WrapUnique(net::NetworkChangeNotifier::CreateMock());
 
   static std::vector<DiscoveryNetworkInfo> fake_network_info;
-  // A raw pointer is used here with the delete in TearDown to avoid making the
-  // destructor public.
-  DiscoveryNetworkMonitor* discovery_network_monitor =
-      new DiscoveryNetworkMonitor();
+  std::unique_ptr<DiscoveryNetworkMonitor> discovery_network_monitor =
+      DiscoveryNetworkMonitor::CreateInstanceForTest(&FakeGetNetworkInfo);
 };
 
 // static
@@ -79,7 +66,7 @@ TEST_F(DiscoveryNetworkMonitorTest, NetworkIdIsConsistent) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   std::string ethernet_network_id = current_network_id;
 
@@ -89,7 +76,7 @@ TEST_F(DiscoveryNetworkMonitorTest, NetworkIdIsConsistent) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_NONE);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   fake_network_info = fake_wifi_info;
   EXPECT_CALL(mock_observer, OnNetworksChanged(_))
@@ -97,7 +84,7 @@ TEST_F(DiscoveryNetworkMonitorTest, NetworkIdIsConsistent) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_WIFI);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   std::string wifi_network_id = current_network_id;
   fake_network_info = fake_ethernet_info;
@@ -106,7 +93,7 @@ TEST_F(DiscoveryNetworkMonitorTest, NetworkIdIsConsistent) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   EXPECT_EQ(ethernet_network_id, current_network_id);
   EXPECT_NE(ethernet_network_id, wifi_network_id);
@@ -122,14 +109,14 @@ TEST_F(DiscoveryNetworkMonitorTest, RemoveObserverStopsNotifications) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   discovery_network_monitor->RemoveObserver(&mock_observer);
   fake_network_info.clear();
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_NONE);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 }
 
 TEST_F(DiscoveryNetworkMonitorTest, RefreshIndependentOfChangeObserver) {
@@ -145,7 +132,7 @@ TEST_F(DiscoveryNetworkMonitorTest, RefreshIndependentOfChangeObserver) {
   };
 
   discovery_network_monitor->Refresh(base::BindOnce(force_refresh_callback));
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 }
 
 TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithoutRefresh) {
@@ -155,7 +142,7 @@ TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithoutRefresh) {
     EXPECT_EQ(DiscoveryNetworkMonitor::kNetworkIdDisconnected, network_id);
   };
   discovery_network_monitor->GetNetworkId(base::BindOnce(check_network_id));
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 }
 
 TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithRefresh) {
@@ -172,7 +159,7 @@ TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithRefresh) {
   };
   discovery_network_monitor->Refresh(
       base::BindOnce(capture_network_id, &current_network_id));
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   auto check_network_id = [](const std::string& refresh_network_id,
                              const std::string& network_id) {
@@ -180,7 +167,7 @@ TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithRefresh) {
   };
   discovery_network_monitor->GetNetworkId(
       base::BindOnce(check_network_id, base::ConstRef(current_network_id)));
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 }
 
 TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithObserver) {
@@ -191,7 +178,7 @@ TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithObserver) {
 
   net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
       net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 
   std::string current_network_id;
   auto check_network_id = [](const std::string& network_id) {
@@ -201,5 +188,5 @@ TEST_F(DiscoveryNetworkMonitorTest, GetNetworkIdWithObserver) {
               network_id);
   };
   discovery_network_monitor->GetNetworkId(base::BindOnce(check_network_id));
-  content::RunAllBlockingPoolTasksUntilIdle();
+  scoped_task_environment.RunUntilIdle();
 }