Remove the NoAtExitBaseTestSuite anti-pattern.

A bunch of unit tests were deriving from base::TestSuite and telling it to not create an AtExitManager, while at the same time they were creating an AtExitManager. Just use the base::TestSuite directly which creates the AtExitManager. This follows similar cleanup I did in r362157. Review URL: https://codereview.chromium.org/1512053002 Cr-Commit-Position: refs/heads/master@{#364590}
Denger-Network · Dec 11, 2015 · 5ccf985 · 5ccf985
1 parent 2b9c473
commit 5ccf985
Show file tree

Hide file tree

Showing 18 changed files with 43 additions and 188 deletions.
diff --git a/base/test/run_all_unittests.cc b/base/test/run_all_unittests.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/test/test_suite.h"
@@ -12,29 +11,13 @@
 #include "base/test/test_file_util.h"
 #endif
 
-namespace {
-
-class NoAtExitBaseTestSuite : public base::TestSuite {
- public:
-  NoAtExitBaseTestSuite(int argc, char** argv)
-      : base::TestSuite(argc, argv, false) {
-  }
-};
-
-int RunTestSuite(int argc, char** argv) {
-  return NoAtExitBaseTestSuite(argc, argv).Run();
-}
-
-}  // namespace
-
 int main(int argc, char** argv) {
 #if defined(OS_ANDROID)
   JNIEnv* env = base::android::AttachCurrentThread();
   base::RegisterContentUriTestUtils(env);
-#else
-  base::AtExitManager at_exit;
 #endif
-  return base::LaunchUnitTests(argc,
-                               argv,
-                               base::Bind(&RunTestSuite, argc, argv));
+  base::TestSuite test_suite(argc, argv);
+  return base::LaunchUnitTests(
+      argc, argv,
+      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
 }
diff --git a/base/test/test_suite.cc b/base/test/test_suite.cc
@@ -97,24 +97,18 @@ int RunUnitTestsUsingBaseTestSuite(int argc, char **argv) {
 }
 
 TestSuite::TestSuite(int argc, char** argv) : initialized_command_line_(false) {
-  PreInitialize(true);
+  PreInitialize();
   InitializeFromCommandLine(argc, argv);
 }
 
 #if defined(OS_WIN)
 TestSuite::TestSuite(int argc, wchar_t** argv)
     : initialized_command_line_(false) {
-  PreInitialize(true);
+  PreInitialize();
   InitializeFromCommandLine(argc, argv);
 }
 #endif  // defined(OS_WIN)
 
-TestSuite::TestSuite(int argc, char** argv, bool create_at_exit_manager)
-    : initialized_command_line_(false) {
-  PreInitialize(create_at_exit_manager);
-  InitializeFromCommandLine(argc, argv);
-}
-
 TestSuite::~TestSuite() {
   if (initialized_command_line_)
     CommandLine::Reset();
@@ -139,7 +133,7 @@ void TestSuite::InitializeFromCommandLine(int argc, wchar_t** argv) {
 }
 #endif  // defined(OS_WIN)
 
-void TestSuite::PreInitialize(bool create_at_exit_manager) {
+void TestSuite::PreInitialize() {
 #if defined(OS_WIN)
   testing::GTEST_FLAG(catch_exceptions) = false;
 #endif
@@ -154,8 +148,7 @@ void TestSuite::PreInitialize(bool create_at_exit_manager) {
   // On Android, AtExitManager is created in
   // testing/android/native_test_wrapper.cc before main() is called.
 #if !defined(OS_ANDROID)
-  if (create_at_exit_manager)
-    at_exit_manager_.reset(new AtExitManager);
+  at_exit_manager_.reset(new AtExitManager);
 #endif
 
   // Don't add additional code to this function.  Instead add it to

diff --git a/base/test/test_suite.h b/base/test/test_suite.h
@@ -49,11 +49,6 @@ class TestSuite {
   int Run();
 
  protected:
-  // This constructor is only accessible to specialized test suite
-  // implementations which need to control the creation of an AtExitManager
-  // instance for the duration of the test.
-  TestSuite(int argc, char** argv, bool create_at_exit_manager);
-
   // By default fatal log messages (e.g. from DCHECKs) result in error dialogs
   // which gum up buildbots. Use a minimalistic assert handler which just
   // terminates the process.
@@ -79,7 +74,7 @@ class TestSuite {
 #endif  // defined(OS_WIN)
 
   // Basic initialization for the test suite happens here.
-  void PreInitialize(bool create_at_exit_manager);
+  void PreInitialize();
 
   test::TraceToFile trace_to_file_;
 

diff --git a/dbus/run_all_unittests.cc b/dbus/run_all_unittests.cc
@@ -2,29 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/test/test_suite.h"
 
-namespace {
-
-class NoAtExitBaseTestSuite : public base::TestSuite {
- public:
-  NoAtExitBaseTestSuite(int argc, char** argv)
-      : base::TestSuite(argc, argv, false) {
-  }
-};
-
-int RunTestSuite(int argc, char** argv) {
-  return NoAtExitBaseTestSuite(argc, argv).Run();
-}
-
-}  // namespace
-
 int main(int argc, char** argv) {
-  base::AtExitManager at_exit;
-  return base::LaunchUnitTestsSerially(argc,
-                                       argv,
-                                       base::Bind(&RunTestSuite, argc, argv));
+  base::TestSuite test_suite(argc, argv);
+  return base::LaunchUnitTests(
+      argc, argv,
+      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
 }
diff --git a/gpu/command_buffer/client/cmd_buffer_helper_test.cc b/gpu/command_buffer/client/cmd_buffer_helper_test.cc
@@ -259,6 +259,7 @@ class CommandBufferHelperTest : public testing::Test {
   std::list<linked_ptr<std::vector<CommandBufferEntry> > > test_command_args_;
   unsigned int test_command_next_id_;
   Sequence sequence_;
+  base::MessageLoop message_loop_;
 };
 
 // Checks immediate_entry_count_ changes based on 'usable' state.

diff --git a/gpu/command_buffer/client/fenced_allocator_test.cc b/gpu/command_buffer/client/fenced_allocator_test.cc
@@ -74,6 +74,7 @@ class BaseFencedAllocatorTest : public testing::Test {
   scoped_ptr<CommandBufferService> command_buffer_;
   scoped_ptr<GpuScheduler> gpu_scheduler_;
   scoped_ptr<CommandBufferHelper> helper_;
+  base::MessageLoop message_loop_;
 };
 
 #ifndef _MSC_VER

diff --git a/gpu/command_buffer/client/mapped_memory_unittest.cc b/gpu/command_buffer/client/mapped_memory_unittest.cc
@@ -72,6 +72,7 @@ class MappedMemoryTestBase : public testing::Test {
   scoped_ptr<CommandBufferService> command_buffer_;
   scoped_ptr<GpuScheduler> gpu_scheduler_;
   scoped_ptr<CommandBufferHelper> helper_;
+  base::MessageLoop message_loop_;
 };
 
 #ifndef _MSC_VER

diff --git a/gpu/command_buffer/client/ring_buffer_test.cc b/gpu/command_buffer/client/ring_buffer_test.cc
@@ -101,6 +101,7 @@ class BaseRingBufferTest : public testing::Test {
 
   scoped_ptr<int8[]> buffer_;
   int8* buffer_start_;
+  base::MessageLoop message_loop_;
 };
 
 #ifndef _MSC_VER

diff --git a/gpu/command_buffer/common/unittest_main.cc b/gpu/command_buffer/common/unittest_main.cc
@@ -2,48 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/at_exit.h"
 #include "base/bind.h"
-#include "base/command_line.h"
-#include "base/message_loop/message_loop.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/test/test_suite.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-#if defined(OS_MACOSX)
-#include "base/mac/scoped_nsautorelease_pool.h"
-#endif
-
-namespace {
-
-class NoAtExitBaseTestSuite : public base::TestSuite {
- public:
-  NoAtExitBaseTestSuite(int argc, char** argv)
-      : base::TestSuite(argc, argv, false) {
-  }
-};
-
-int RunTestSuite(int argc, char** argv) {
-  base::MessageLoop message_loop;
-  return NoAtExitBaseTestSuite(argc, argv).Run();
-}
-
-}  // namespace
 
 int main(int argc, char** argv) {
-  // On Android, AtExitManager is created in
-  // testing/android/native_test_wrapper.cc before main() is called.
-  // The same thing is also done in base/test/test_suite.cc
-#if !defined(OS_ANDROID)
-  base::AtExitManager exit_manager;
-#endif
-  base::CommandLine::Init(argc, argv);
-#if defined(OS_MACOSX)
-  base::mac::ScopedNSAutoreleasePool autorelease_pool;
-#endif
-  testing::InitGoogleMock(&argc, argv);
-  return base::LaunchUnitTests(argc,
-                               argv,
-                               base::Bind(&RunTestSuite, argc, argv));
+  base::TestSuite test_suite(argc, argv);
+  return base::LaunchUnitTests(
+      argc, argv,
+      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
 }
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
@@ -5,6 +5,7 @@
 #ifndef GPU_COMMAND_BUFFER_SERVICE_GLES2_CMD_DECODER_UNITTEST_BASE_H_
 #define GPU_COMMAND_BUFFER_SERVICE_GLES2_CMD_DECODER_UNITTEST_BASE_H_
 
+#include "base/message_loop/message_loop.h"
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
 #include "gpu/command_buffer/service/buffer_manager.h"
@@ -721,6 +722,7 @@ class GLES2DecoderTestBase : public ::testing::TestWithParam<bool> {
   scoped_ptr< ::testing::StrictMock<MockCommandBufferEngine> > engine_;
   scoped_refptr<ContextGroup> group_;
   MockGLStates gl_states_;
+  base::MessageLoop message_loop_;
 };
 
 class GLES2DecoderWithShaderTestBase : public GLES2DecoderTestBase {

diff --git a/gpu/command_buffer/service/gpu_scheduler_unittest.cc b/gpu/command_buffer/service/gpu_scheduler_unittest.cc
@@ -72,6 +72,7 @@ class GpuSchedulerTest : public testing::Test {
   int32* buffer_;
   scoped_ptr<gles2::MockGLES2Decoder> decoder_;
   scoped_ptr<GpuScheduler> scheduler_;
+  base::MessageLoop message_loop_;
 };
 
 TEST_F(GpuSchedulerTest, SchedulerDoesNothingIfRingBufferIsEmpty) {

diff --git a/gpu/command_buffer/service/gpu_service_test.h b/gpu/command_buffer/service/gpu_service_test.h
@@ -7,6 +7,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gl/gl_mock.h"
@@ -36,6 +37,7 @@ class GpuServiceTest : public testing::Test {
   bool ran_setup_;
   bool ran_teardown_;
   scoped_refptr<gfx::GLContextStubWithExtensions> context_;
+  base::MessageLoop message_loop_;
 };
 
 }  // namespace gles2

diff --git a/ipc/run_all_unittests.cc b/ipc/run_all_unittests.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/test/test_suite.h"
@@ -12,29 +11,13 @@
 #include "base/test/test_file_util.h"
 #endif
 
-namespace {
-
-class NoAtExitBaseTestSuite : public base::TestSuite {
- public:
-  NoAtExitBaseTestSuite(int argc, char** argv)
-      : base::TestSuite(argc, argv, false) {
-  }
-};
-
-int RunTestSuite(int argc, char** argv) {
-  return NoAtExitBaseTestSuite(argc, argv).Run();
-}
-
-}  // namespace
-
 int main(int argc, char** argv) {
 #if defined(OS_ANDROID)
   JNIEnv* env = base::android::AttachCurrentThread();
   base::RegisterContentUriTestUtils(env);
-#else
-  base::AtExitManager at_exit;
 #endif
-  return base::LaunchUnitTestsSerially(argc,
-                                       argv,
-                                       base::Bind(&RunTestSuite, argc, argv));
+  base::TestSuite test_suite(argc, argv);
+  return base::LaunchUnitTests(
+      argc, argv,
+      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
 }
diff --git a/net/test/net_test_suite.cc b/net/test/net_test_suite.cc
@@ -24,11 +24,6 @@ NetTestSuite::NetTestSuite(int argc, char** argv)
     : TestSuite(argc, argv) {
 }
 
-NetTestSuite::NetTestSuite(int argc, char** argv,
-                           bool create_at_exit_manager)
-    : TestSuite(argc, argv, create_at_exit_manager) {
-}
-
 NetTestSuite::~NetTestSuite() {}
 
 void NetTestSuite::Initialize() {

diff --git a/net/test/net_test_suite.h b/net/test/net_test_suite.h
@@ -28,11 +28,6 @@ class NetTestSuite : public base::TestSuite {
   void Shutdown() override;
 
  protected:
-  // This constructor is only accessible to specialized net test
-  // implementations which need to control the creation of an AtExitManager
-  // instance for the duration of the test.
-  NetTestSuite(int argc, char** argv, bool create_at_exit_manager);
-
   // Called from within Initialize(), but separate so that derived classes
   // can initialize the NetTestSuite instance only and not
   // TestSuite::Initialize().  TestSuite::Initialize() performs some global

diff --git a/remoting/test/app_remoting_test_driver.cc b/remoting/test/app_remoting_test_driver.cc
@@ -143,26 +143,11 @@ void PrintJsonFileInfo() {
          switches::kRefreshTokenFileSwitchName);
 }
 
-// This class exists so that we can create a test suite which does not create
-// its own AtExitManager.  The problem we are working around occurs when
-// the test suite does not create an AtExitManager (e.g. if no tests are run)
-// and the environment object destroys its MessageLoop, then a crash will occur.
-class NoAtExitBaseTestSuite : public base::TestSuite {
- public:
-  NoAtExitBaseTestSuite(int argc, char** argv)
-      : base::TestSuite(argc, argv, false) {}
-
-  static int RunTestSuite(int argc, char** argv) {
-    return NoAtExitBaseTestSuite(argc, argv).Run();
-  }
-};
-
 }  // namespace
 
 int main(int argc, char** argv) {
-  base::AtExitManager at_exit;
+  base::TestSuite test_suite(argc, argv);
   base::MessageLoopForIO message_loop;
-  testing::InitGoogleTest(&argc, argv);
 
   if (!base::CommandLine::InitializedForCurrentProcess()) {
     if (!base::CommandLine::Init(argc, argv)) {
@@ -197,7 +182,7 @@ int main(int argc, char** argv) {
     PrintAuthCodeInfo();
     return base::LaunchUnitTestsSerially(
         argc, argv,
-        base::Bind(&NoAtExitBaseTestSuite::RunTestSuite, argc, argv));
+        base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
   }
 
   remoting::test::AppRemotingTestDriverEnvironment::EnvironmentOptions options;
@@ -265,5 +250,6 @@ int main(int argc, char** argv) {
   // Because many tests may access the same remoting host(s), we need to run
   // the tests sequentially so they do not interfere with each other.
   return base::LaunchUnitTestsSerially(
-      argc, argv, base::Bind(&NoAtExitBaseTestSuite::RunTestSuite, argc, argv));
+      argc, argv,
+      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
 }