Isolate tests by running AtExit callbacks between them.

For now, this is only for base_unittests. The plan is to enable
it for all unit tests. This should finally fix mysterious
problems cause by Singletons surviving after one test etc.

This change also adapts LazyInstance so that it can be reused
after being destroyed. It is used very frequently, for example
each time a MessageLoop is used. It is also worth noting that
we had some problems in the past related to the MessageLoop
being destroyed and re-instantiated in the same test executable.
This patch should also fix that.

TEST=none
BUG=12710

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

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

8 files changed, 76 insertions, 31 deletions

diff --git a/base/at_exit.h b/base/at_exit.h
index 0bc3466..48ba3d9 100644
--- a/base/at_exit.h
+++ b/base/at_exit.h
@@ -66,6 +66,13 @@ class AtExitManager {
   DISALLOW_COPY_AND_ASSIGN(AtExitManager);
 };
 
+#if defined(UNIT_TEST)
+class ShadowingAtExitManager : public AtExitManager {
+ public:
+  ShadowingAtExitManager() : AtExitManager(true) {}
+};
+#endif  // defined(UNIT_TEST)
+
 }  // namespace base
 
 #endif  // BASE_AT_EXIT_H_
diff --git a/base/at_exit_unittest.cc b/base/at_exit_unittest.cc
index 50dec04..2fbed25 100644
--- a/base/at_exit_unittest.cc
+++ b/base/at_exit_unittest.cc
@@ -8,13 +8,6 @@
 
 namespace {
 
-// Don't test the global AtExitManager, because asking it to process its
-// AtExit callbacks can ruin the global state that other tests may depend on.
-class ShadowingAtExitManager : public base::AtExitManager {
- public:
-  ShadowingAtExitManager() : AtExitManager(true) {}
-};
-
 int g_test_counter_1 = 0;
 int g_test_counter_2 = 0;
 
@@ -45,9 +38,14 @@ void ExpectParamIsCounter(void* param) {
 
 }  // namespace
 
-TEST(AtExitTest, Basic) {
-  ShadowingAtExitManager shadowing_at_exit_manager;
+class AtExitTest : public testing::Test {
+ private:
+  // Don't test the global AtExitManager, because asking it to process its
+  // AtExit callbacks can ruin the global state that other tests may depend on.
+  base::ShadowingAtExitManager exit_manager_;
+};
 
+TEST_F(AtExitTest, Basic) {
   ZeroTestCounters();
   base::AtExitManager::RegisterCallback(&IncrementTestCounter1, NULL);
   base::AtExitManager::RegisterCallback(&IncrementTestCounter2, NULL);
@@ -60,9 +58,7 @@ TEST(AtExitTest, Basic) {
   EXPECT_EQ(1, g_test_counter_2);
 }
 
-TEST(AtExitTest, LIFOOrder) {
-  ShadowingAtExitManager shadowing_at_exit_manager;
-
+TEST_F(AtExitTest, LIFOOrder) {
   ZeroTestCounters();
   base::AtExitManager::RegisterCallback(&IncrementTestCounter1, NULL);
   base::AtExitManager::RegisterCallback(&ExpectCounter1IsZero, NULL);
@@ -75,9 +71,7 @@ TEST(AtExitTest, LIFOOrder) {
   EXPECT_EQ(1, g_test_counter_2);
 }
 
-TEST(AtExitTest, Param) {
-  ShadowingAtExitManager shadowing_at_exit_manager;
-
+TEST_F(AtExitTest, Param) {
   base::AtExitManager::RegisterCallback(&ExpectParamIsNull, NULL);
   base::AtExitManager::RegisterCallback(&ExpectParamIsCounter,
                                         &g_test_counter_1);
diff --git a/base/lazy_instance.cc b/base/lazy_instance.cc
index 388622a..9923253 100644
--- a/base/lazy_instance.cc
+++ b/base/lazy_instance.cc
@@ -27,7 +27,14 @@ void LazyInstanceHelper::EnsureInstance(void* instance,
 
     // Instance is created, go from CREATING to CREATED.
     base::subtle::Release_Store(&state_, STATE_CREATED);
-    // Register the destructor callback with AtExitManager.
+
+    // Allow reusing the LazyInstance (reset it to the initial state). This
+    // makes possible calling all AtExit callbacks between tests. Assumes that
+    // no other threads execute when AtExit callbacks are processed.
+    base::AtExitManager::RegisterCallback(&LazyInstanceHelper::ResetState,
+                                          this);
+
+    // Make sure that the lazily instantiated object will get destroyed at exit.
     base::AtExitManager::RegisterCallback(dtor, instance);
   } else {
     // It's either in the process of being created, or already created.  Spin.
@@ -36,4 +43,9 @@ void LazyInstanceHelper::EnsureInstance(void* instance,
   }
 }
 
+// static
+void LazyInstanceHelper::ResetState(void* helper) {
+  reinterpret_cast<LazyInstanceHelper*>(helper)->state_ = STATE_EMPTY;
+}
+
 }  // namespace base
diff --git a/base/lazy_instance.h b/base/lazy_instance.h
index d0cc109..d7754a7 100644
--- a/base/lazy_instance.h
+++ b/base/lazy_instance.h
@@ -76,6 +76,10 @@ class LazyInstanceHelper {
   base::subtle::Atomic32 state_;
 
  private:
+  // Resets state of |helper| to STATE_EMPTY so that it can be reused.
+  // Not thread safe.
+  static void ResetState(void* helper);
+
   DISALLOW_COPY_AND_ASSIGN(LazyInstanceHelper);
 };
 
diff --git a/base/lazy_instance_unittest.cc b/base/lazy_instance_unittest.cc
index de5ea87..cf17a84 100644
--- a/base/lazy_instance_unittest.cc
+++ b/base/lazy_instance_unittest.cc
@@ -10,11 +10,6 @@
 
 namespace {
 
-class ShadowingAtExitManager : public base::AtExitManager {
- public:
-  ShadowingAtExitManager() : AtExitManager(true) { }
-};
-
 base::AtomicSequenceNumber constructed_seq_(base::LINKER_INITIALIZED);
 base::AtomicSequenceNumber destructed_seq_(base::LINKER_INITIALIZED);
 
@@ -63,7 +58,7 @@ static base::LazyInstance<ConstructAndDestructLogger> lazy_logger(
 
 TEST(LazyInstanceTest, Basic) {
   {
-    ShadowingAtExitManager shadow;
+    base::ShadowingAtExitManager shadow;
 
     EXPECT_EQ(0, constructed_seq_.GetNext());
     EXPECT_EQ(0, destructed_seq_.GetNext());
@@ -84,7 +79,7 @@ static base::LazyInstance<SlowConstructor> lazy_slow(base::LINKER_INITIALIZED);
 
 TEST(LazyInstanceTest, ConstructorThreadSafety) {
   {
-    ShadowingAtExitManager shadow;
+    base::ShadowingAtExitManager shadow;
 
     SlowDelegate delegate(&lazy_slow);
     EXPECT_EQ(0, SlowConstructor::constructed);
diff --git a/base/singleton_unittest.cc b/base/singleton_unittest.cc
index c19fa14..bb46bee 100644
--- a/base/singleton_unittest.cc
+++ b/base/singleton_unittest.cc
@@ -10,11 +10,6 @@
 
 namespace {
 
-class ShadowingAtExitManager : public base::AtExitManager {
- public:
-  ShadowingAtExitManager() : AtExitManager(true) { }
-};
-
 COMPILE_ASSERT(DefaultSingletonTraits<int>::kRegisterAtExit == true, a);
 
 template<typename Type>
@@ -134,7 +129,7 @@ TEST_F(SingletonTest, Basic) {
   CallbackFunc* leaky_singleton;
 
   {
-    ShadowingAtExitManager sem;
+    base::ShadowingAtExitManager sem;
     {
       singleton_int_1 = SingletonInt1();
     }
@@ -193,7 +188,7 @@ TEST_F(SingletonTest, Basic) {
   DefaultSingletonTraits<CallbackFunc>::Delete(leaky_singleton);
 
   {
-    ShadowingAtExitManager sem;
+    base::ShadowingAtExitManager sem;
     // Verifiy that the variables were reset.
     {
       singleton_int_1 = SingletonInt1();
diff --git a/base/test/run_all_unittests.cc b/base/test/run_all_unittests.cc
index 841b353..7b90358 100644
--- a/base/test/run_all_unittests.cc
+++ b/base/test/run_all_unittests.cc
@@ -5,5 +5,7 @@
 #include "base/test/test_suite.h"
 
 int main(int argc, char** argv) {
-  return TestSuite(argc, argv).Run();
+  TestSuite test_suite(argc, argv);
+  test_suite.EnforceTestIsolation();
+  return test_suite.Run();
 }
diff --git a/base/test/test_suite.h b/base/test/test_suite.h
index 42ad027..44e8511 100644
--- a/base/test/test_suite.h
+++ b/base/test/test_suite.h
@@ -14,8 +14,10 @@
 #include "base/debug_on_start.h"
 #include "base/i18n/icu_util.h"
 #include "base/multiprocess_test.h"
+#include "base/nss_init.h"
 #include "base/process_util.h"
 #include "base/scoped_nsautorelease_pool.h"
+#include "base/scoped_ptr.h"
 #include "base/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/multiprocess_func_list.h"
@@ -27,6 +29,25 @@
 // Match function used by the GetTestCount method.
 typedef bool (*TestMatch)(const testing::TestInfo&);
 
+// By setting up a shadow AtExitManager, this test event listener ensures that
+// no state is carried between tests (like singletons, lazy instances, etc).
+// Of course it won't help if the code under test corrupts memory.
+class TestIsolationEnforcer : public testing::EmptyTestEventListener {
+ public:
+  virtual void OnTestStart(const testing::TestInfo& test_info) {
+    ASSERT_FALSE(exit_manager_.get());
+    exit_manager_.reset(new base::ShadowingAtExitManager());
+  }
+
+  virtual void OnTestEnd(const testing::TestInfo& test_info) {
+    ASSERT_TRUE(exit_manager_.get());
+    exit_manager_.reset();
+  }
+
+ private:
+  scoped_ptr<base::ShadowingAtExitManager> exit_manager_;
+};
+
 class TestSuite {
  public:
   TestSuite(int argc, char** argv) {
@@ -77,6 +98,13 @@ class TestSuite {
     return count;
   }
 
+  // TODO(phajdan.jr): Enforce isolation for all tests once it's stable.
+  void EnforceTestIsolation() {
+    testing::TestEventListeners& listeners =
+        testing::UnitTest::GetInstance()->listeners();
+    listeners.Append(new TestIsolationEnforcer);
+  }
+
   // Don't add additional code to this method.  Instead add it to
   // Initialize().  See bug 6436.
   int Run() {
@@ -172,6 +200,14 @@ class TestSuite {
 #endif  // defined(OS_WIN)
 
     icu_util::Initialize();
+
+#if defined(OS_LINUX)
+    // Trying to repeatedly initialize and cleanup NSS and NSPR may result in
+    // a deadlock. Such repeated initialization will happen when using test
+    // isolation. Prevent problems by initializing NSS here, so that the cleanup
+    // will be done only on process exit.
+    base::EnsureNSSInit();
+#endif  // defined(OS_LINUX)
   }
 
   virtual void Shutdown() {