Remove ScopedTaskEnvironment from NetTestSuite.

Remove global ScopedTaskEnvironment instance that lives across all of
net_unittests.  Add a local variable or test fixture member to whichever
test would crash/fail without that.  Change type from default to IO
for whichever test would crash/fail without that.

In some cases, like HttpServerPropertiesManagerTest, make member private
and add protected accessors.  In most other cases make member protected
and access directly from tests.  I was mostly doing this by occurrence,
and I'm happy to change any test in either direction.

I made scoped_task_environment_ the last member unless it was necessary
to construct it before the constructor of some other members.  I
generally made it a private member, unless it is manipulated from a test
and I was too lazy to write an accessor, or unless it needed to be
created before other, non-private members, because I did not feel like
the ugliness of multiple alternating private and protected sections is
justified.  I'm happy to change this is necessary.

Bug: 791831
Change-Id: I578690820c07264372cff9dbd8bd9c944c243ba8
Reviewed-on: https://chromium-review.googlesource.com/1037405
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556667}

net/BUILD.gn

@@ -2678,6 +2678,7 @@ static_library("test_support") {
     "test/test_certificate_data.h",
     "test/test_data_directory.cc",
     "test/test_data_directory.h",
+    "test/test_with_scoped_task_environment.h",
     "test/url_request/ssl_certificate_error_job.cc",
     "test/url_request/ssl_certificate_error_job.h",
     "test/url_request/url_request_failed_job.cc",

net/android/http_auth_negotiate_android_unittest.cc

@@ -2,9 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "net/android/http_auth_negotiate_android.h"
+
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "net/android/dummy_spnego_authenticator.h"
-#include "net/android/http_auth_negotiate_android.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/http/http_auth_challenge_tokenizer.h"
@@ -15,6 +17,8 @@ namespace net {
 namespace android {
 
 TEST(HttpAuthNegotiateAndroidTest, GenerateAuthToken) {
+  base::test::ScopedTaskEnvironment scoped_task_environment;
+
   DummySpnegoAuthenticator::EnsureTestAccountExists();
 
   std::string auth_token;

net/android/network_change_notifier_android_unittest.cc

@@ -4,16 +4,18 @@
 
 // See network_change_notifier_android.h for design explanations.
 
+#include "net/android/network_change_notifier_android.h"
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/run_loop.h"
-#include "net/android/network_change_notifier_android.h"
 #include "net/android/network_change_notifier_delegate_android.h"
 #include "net/base/ip_address.h"
 #include "net/base/network_change_notifier.h"
 #include "net/dns/dns_config_service.h"
 #include "net/dns/dns_protocol.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -177,7 +179,8 @@ class TestNetworkObserver : public NetworkChangeNotifier::NetworkObserver {
 
 }  // namespace
 
-class BaseNetworkChangeNotifierAndroidTest : public testing::Test {
+class BaseNetworkChangeNotifierAndroidTest
+    : public TestWithScopedTaskEnvironment {
  protected:
   typedef NetworkChangeNotifier::ConnectionType ConnectionType;
   typedef NetworkChangeNotifier::ConnectionSubtype ConnectionSubtype;

net/android/traffic_stats_unittest.cc

@@ -5,6 +5,7 @@
 #include "net/android/traffic_stats.h"
 
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -15,6 +16,9 @@ namespace net {
 namespace {
 
 TEST(TrafficStatsAndroidTest, BasicsTest) {
+  base::test::ScopedTaskEnvironment scoped_task_environment(
+      base::test::ScopedTaskEnvironment::MainThreadType::IO);
+
   EmbeddedTestServer embedded_test_server;
   embedded_test_server.ServeFilesFromDirectory(
       base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest")));
@@ -48,6 +52,9 @@ TEST(TrafficStatsAndroidTest, BasicsTest) {
 }
 
 TEST(TrafficStatsAndroidTest, UIDBasicsTest) {
+  base::test::ScopedTaskEnvironment scoped_task_environment(
+      base::test::ScopedTaskEnvironment::MainThreadType::IO);
+
   EmbeddedTestServer embedded_test_server;
   embedded_test_server.ServeFilesFromDirectory(
       base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest")));

net/base/address_tracker_linux_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/bind_helpers.h"
 #include "base/synchronization/spin_wait.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/threading/simple_thread.h"
 #include "net/base/ip_address.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -720,6 +721,8 @@ class GetCurrentConnectionTypeRunner
 };
 
 TEST_F(AddressTrackerLinuxTest, BroadcastInit) {
+  base::test::ScopedTaskEnvironment scoped_task_environment(
+      base::test::ScopedTaskEnvironment::MainThreadType::IO);
   InitializeAddressTracker(true);
 
   GetCurrentConnectionTypeRunner runner1(tracker_.get(), "waiter_thread_1");

net/base/directory_lister_unittest.cc

@@ -16,6 +16,7 @@
 #include "net/base/directory_lister.h"
 #include "net/base/net_errors.h"
 #include "net/test/gtest_util.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -130,7 +131,8 @@ class ListerDelegate : public DirectoryLister::DirectoryListerDelegate {
 
 }  // namespace
 
-class DirectoryListerTest : public PlatformTest {
+class DirectoryListerTest : public PlatformTest,
+                            public WithScopedTaskEnvironment {
  public:
   DirectoryListerTest()
       : total_created_file_system_objects_in_temp_root_dir_(0),

net/base/elements_upload_data_stream_unittest.cc

@@ -27,6 +27,7 @@
 #include "net/base/upload_file_element_reader.h"
 #include "net/log/net_log_with_source.h"
 #include "net/test/gtest_util.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -139,7 +140,8 @@ class MockUploadElementReader : public UploadElementReader {
 
 }  // namespace
 
-class ElementsUploadDataStreamTest : public PlatformTest {
+class ElementsUploadDataStreamTest : public PlatformTest,
+                                     public WithScopedTaskEnvironment {
  public:
   void SetUp() override {
     PlatformTest::SetUp();

net/base/file_stream_unittest.cc

@@ -25,6 +25,7 @@
 #include "net/base/test_completion_callback.h"
 #include "net/log/test_net_log.h"
 #include "net/test/gtest_util.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -52,7 +53,7 @@ IOBufferWithSize* CreateTestDataBuffer() {
 
 }  // namespace
 
-class FileStreamTest : public PlatformTest {
+class FileStreamTest : public PlatformTest, public WithScopedTaskEnvironment {
  public:
   void SetUp() override {
     PlatformTest::SetUp();

net/base/layered_network_delegate_unittest.cc

@@ -21,6 +21,7 @@
 #include "net/proxy_resolution/proxy_config_service.h"
 #include "net/proxy_resolution/proxy_info.h"
 #include "net/proxy_resolution/proxy_retry_info.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_test_util.h"
@@ -371,7 +372,7 @@ class TestLayeredNetworkDelegate : public LayeredNetworkDelegate {
 
 }  // namespace
 
-class LayeredNetworkDelegateTest : public testing::Test {
+class LayeredNetworkDelegateTest : public TestWithScopedTaskEnvironment {
  public:
   LayeredNetworkDelegateTest() {
     std::unique_ptr<TestNetworkDelegateImpl> test_network_delegate(

net/base/network_change_notifier_win_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/single_thread_task_runner.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/network_change_notifier_factory.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -77,7 +78,7 @@ bool ExitMessageLoopAndReturnFalse() {
 
 }  // namespace
 
-class NetworkChangeNotifierWinTest : public testing::Test {
+class NetworkChangeNotifierWinTest : public TestWithScopedTaskEnvironment {
  public:
   // Calls WatchForAddressChange, and simulates a WatchForAddressChangeInternal
   // success.  Expects that |network_change_notifier_| has just been created, so

net/base/test_completion_callback_unittest.cc

@@ -4,14 +4,16 @@
 
 // Illustrates how to use net::TestCompletionCallback.
 
+#include "net/base/test_completion_callback.h"
+
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "net/base/completion_callback.h"
-#include "net/base/test_completion_callback.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -108,7 +110,8 @@ bool ExampleEmployer::DoSomething(const CompletionCallback& callback) {
 
 }  // namespace
 
-typedef PlatformTest TestCompletionCallbackTest;
+class TestCompletionCallbackTest : public PlatformTest,
+                                   public WithScopedTaskEnvironment {};
 
 TEST_F(TestCompletionCallbackTest, Simple) {
   ExampleEmployer boss;

net/base/upload_file_element_reader_unittest.cc

@@ -17,6 +17,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/test/gtest_util.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -32,7 +33,8 @@ namespace net {
 // When the parameter is false, the UploadFileElementReader is passed only a
 // FilePath and needs to open the file itself. When it's true, it's passed an
 // already open base::File.
-class UploadFileElementReaderTest : public testing::TestWithParam<bool> {
+class UploadFileElementReaderTest : public testing::TestWithParam<bool>,
+                                    public WithScopedTaskEnvironment {
  protected:
   void SetUp() override {
     // Some tests (*.ReadPartially) rely on bytes_.size() being even.

net/cert/multi_threaded_cert_verifier_unittest.cc

@@ -19,6 +19,7 @@
 #include "net/test/cert_test_util.h"
 #include "net/test/gtest_util.h"
 #include "net/test/test_data_directory.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -60,7 +61,7 @@ class MockCertVerifyProc : public CertVerifyProc {
 
 }  // namespace
 
-class MultiThreadedCertVerifierTest : public ::testing::Test {
+class MultiThreadedCertVerifierTest : public TestWithScopedTaskEnvironment {
  public:
   MultiThreadedCertVerifierTest() : verifier_(new MockCertVerifyProc()) {}
   ~MultiThreadedCertVerifierTest() override = default;

net/cert/nss_cert_database_chromeos_unittest.cc

@@ -8,16 +8,14 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/run_loop.h"
-#include "base/test/scoped_task_environment.h"
 #include "crypto/nss_util_internal.h"
 #include "crypto/scoped_test_nss_chromeos_user.h"
 #include "crypto/scoped_test_nss_db.h"
 #include "net/cert/cert_database.h"
 #include "net/cert/x509_util_nss.h"
 #include "net/test/cert_test_util.h"
-#include "net/test/net_test_suite.h"
 #include "net/test/test_data_directory.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -51,7 +49,7 @@ void SwapCertLists(ScopedCERTCertificateList* destination,
 
 }  // namespace
 
-class NSSCertDatabaseChromeOSTest : public testing::Test,
+class NSSCertDatabaseChromeOSTest : public TestWithScopedTaskEnvironment,
                                     public CertDatabase::Observer {
  public:
   NSSCertDatabaseChromeOSTest()
@@ -173,7 +171,7 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) {
   EXPECT_FALSE(IsCertInCertificateList(certs_2[0].get(), user_1_certlist));
 
   // Run the message loop so the observer notifications get processed.
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
   // Should have gotten two OnCertDBChanged notifications.
   ASSERT_EQ(2, db_changed_count_);
 
@@ -185,7 +183,7 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) {
   db_2_->ListCerts(
       base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async)));
 
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
 
   EXPECT_TRUE(IsCertInCertificateList(certs_1[0].get(), user_1_certlist_async));
   EXPECT_FALSE(
@@ -233,7 +231,7 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) {
   EXPECT_FALSE(IsCertInCertificateList(certs_2[0].get(), user_1_certlist));
 
   // Run the message loop so the observer notifications get processed.
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
   // TODO(mattm): ImportServerCert doesn't actually cause any observers to
   // fire. Is that correct?
   EXPECT_EQ(0, db_changed_count_);
@@ -246,7 +244,7 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) {
   db_2_->ListCerts(
       base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async)));
 
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
 
   EXPECT_TRUE(IsCertInCertificateList(certs_1[0].get(), user_1_certlist_async));
   EXPECT_FALSE(
@@ -266,7 +264,7 @@ TEST_F(NSSCertDatabaseChromeOSTest, NoCrashIfShutdownBeforeDoneOnWorkerPool) {
 
   db_1_.reset();
 
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
 
   EXPECT_LT(0U, certlist.size());
 }

net/cert/nss_cert_database_unittest.cc

@@ -19,7 +19,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_task_environment.h"
 #include "crypto/scoped_nss_types.h"
 #include "crypto/scoped_test_nss_db.h"
 #include "net/base/hash_value.h"
@@ -31,8 +30,8 @@
 #include "net/cert/x509_util_nss.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/gtest_util.h"
-#include "net/test/net_test_suite.h"
 #include "net/test/test_data_directory.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "net/third_party/mozilla_security_manager/nsNSSCertificateDB.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -60,7 +59,7 @@ std::string GetSubjectCN(CERTCertificate* cert) {
 
 }  // namespace
 
-class CertDatabaseNSSTest : public testing::Test {
+class CertDatabaseNSSTest : public TestWithScopedTaskEnvironment {
  public:
   void SetUp() override {
     ASSERT_TRUE(test_nssdb_.is_open());
@@ -148,7 +147,7 @@ TEST_F(CertDatabaseNSSTest, ListCerts) {
   cert_db_->ListCerts(base::Bind(&SwapCertList, base::Unretained(&certs)));
   EXPECT_EQ(0U, certs.size());
 
-  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
+  RunUntilIdle();
 
   // The test DB is empty, but let's assume there will always be something in
   // the other slots.

net/cert_net/cert_net_fetcher_impl_unittest.cc

@@ -20,6 +20,7 @@
 #include "net/http/http_server_properties_impl.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/gtest_util.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "net/test/url_request/url_request_hanging_read_job.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request_filter.h"
@@ -226,7 +227,8 @@ class CertNetFetcherImplTest : public PlatformTest {
 
 // Installs URLRequestHangingReadJob handlers and clears them on teardown.
 class CertNetFetcherImplTestWithHangingReadHandler
-    : public CertNetFetcherImplTest {
+    : public CertNetFetcherImplTest,
+      public WithScopedTaskEnvironment {
  protected:
   void SetUp() override { URLRequestHangingReadJob::AddUrlHandler(); }
 

net/cert_net/nss_ocsp_unittest.cc

@@ -26,6 +26,7 @@
 #include "net/test/cert_test_util.h"
 #include "net/test/gtest_util.h"
 #include "net/test/test_data_directory.h"
+#include "net/test/test_with_scoped_task_environment.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_interceptor.h"
 #include "net/url_request/url_request_test_job.h"
@@ -76,7 +77,7 @@ class AiaResponseHandler : public URLRequestInterceptor {
 
 }  // namespace
 
-class NssHttpTest : public ::testing::Test {
+class NssHttpTest : public TestWithScopedTaskEnvironment {
  public:
   NssHttpTest()
       : context_(false),