Call logging::InitLogging. The lack of this was causing some hangs (and possibly crashes) in ObserverListTest.BUG=6286

This CL has expanded to include some cleanup and refactoring of test_suite and related files, so that this logging change (and other improvements) are applied to all unit tests.


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

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

17 files changed, 90 insertions, 135 deletions

diff --git a/base/base_paths.h b/base/base_paths.h
index 3992d62..0f29f56 100644
--- a/base/base_paths.h
+++ b/base/base_paths.h
@@ -16,6 +16,7 @@
 #elif defined(OS_LINUX)
 #include "base/base_paths_linux.h"
 #endif
+#include "base/path_service.h"
 
 namespace base {
 
diff --git a/base/base_unittests.scons b/base/base_unittests.scons
index 3dd851e..f3ed327 100644
--- a/base/base_unittests.scons
+++ b/base/base_unittests.scons
@@ -50,6 +50,7 @@ input_files = ChromeFileList([
   MSVSFilter('support', [
     'multiprocess_test.h',
     'no_windows2000_unittest.h',
+    'perf_test_suite.h',
     'run_all_unittests.cc',
     'test_suite.h',
   ]),
diff --git a/base/build/base_unittests.vcproj b/base/build/base_unittests.vcproj
index eb148d5..945c8dd 100644
--- a/base/build/base_unittests.vcproj
+++ b/base/build/base_unittests.vcproj
@@ -152,6 +152,10 @@
 				>
 			</File>
 			<File
+				RelativePath="..\perf_test_suite.h"
+				>
+			</File>
+			<File
 				RelativePath="..\run_all_unittests.cc"
 				>
 			</File>
diff --git a/base/perf_test_suite.h b/base/perf_test_suite.h
new file mode 100644
index 0000000..bab7904
--- /dev/null
+++ b/base/perf_test_suite.h
@@ -0,0 +1,50 @@
+// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_PERF_TEST_SUITE_H_
+#define BASE_PERF_TEST_SUITE_H_
+
+#include "base/command_line.h"
+#include "base/debug_util.h"
+#include "base/file_path.h"
+#include "base/perftimer.h"
+#include "base/process_util.h"
+#include "base/string_util.h"
+#include "base/test_suite.h"
+
+class PerfTestSuite : public TestSuite {
+ public:
+  PerfTestSuite(int argc, char** argv) : TestSuite(argc, argv) {
+  }
+
+  virtual void Initialize() {
+    // Initialize the perf timer log
+    FilePath log_path;
+    std::wstring log_file = CommandLine().GetSwitchValue(L"log-file");
+    if (log_file.empty()) {
+      FilePath exe;
+      PathService::Get(base::FILE_EXE, &exe);
+      log_path = exe.ReplaceExtension(FILE_PATH_LITERAL("log"));
+      log_path = log_path.InsertBeforeExtension(FILE_PATH_LITERAL("_perf"));
+    } else {
+      log_path = FilePath::FromWStringHack(log_file);
+    }
+    ASSERT_TRUE(InitPerfLog(log_path));
+
+    // Raise to high priority to have more precise measurements. Since we don't
+    // aim at 1% precision, it is not necessary to run at realtime level.
+    if (!DebugUtil::BeingDebugged())
+      base::RaiseProcessToHighPriority();
+
+    TestSuite::Initialize();
+  }
+
+  virtual void Shutdown() {
+    TestSuite::Shutdown();
+
+    FinalizePerfLog();
+  }
+};
+
+#endif  // BASE_PERF_TEST_SUITE_H_
diff --git a/base/perftimer.cc b/base/perftimer.cc
index 9fc0328..4c64c5e 100644
--- a/base/perftimer.cc
+++ b/base/perftimer.cc
@@ -8,19 +8,20 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 
 static FILE* perf_log_file = NULL;
 
-bool InitPerfLog(const char* log_file) {
+bool InitPerfLog(const FilePath& log_file) {
   if (perf_log_file) {
     // trying to initialize twice
     NOTREACHED();
     return false;
   }
 
-  perf_log_file = file_util::OpenFile(std::string(log_file), "w");
+  perf_log_file = file_util::OpenFile(log_file, "w");
   return perf_log_file != NULL;
 }
 
diff --git a/base/perftimer.h b/base/perftimer.h
index 9558697..81009dd 100644
--- a/base/perftimer.h
+++ b/base/perftimer.h
@@ -7,6 +7,7 @@
 
 #include <string>
 #include "base/basictypes.h"
+#include "base/file_path.h"
 #include "base/time.h"
 
 // ----------------------------------------------------------------------
@@ -14,7 +15,7 @@
 // called at the beginning and end (respectively) of running all the
 // performance tests. The init function returns true on success.
 // ----------------------------------------------------------------------
-bool InitPerfLog(const char* log_file);
+bool InitPerfLog(const FilePath& log_path);
 void FinalizePerfLog();
 
 // ----------------------------------------------------------------------
diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index 5723a83..a6eb8a7 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -158,11 +158,13 @@ MULTIPROCESS_TEST_MAIN(ProcessUtilsLeakFDChildProcess) {
     }
   }
 
+  // InitLogging always opens a file at startup.
+  int expected_num_open_fds = 1;
 #if defined(OS_LINUX)
   // On Linux, '/etc/localtime' is opened before the test's main() enters.
-  const int expected_num_open_fds = 1;
-  num_open_files -= expected_num_open_fds;
+  expected_num_open_fds += 1;
 #endif  // defined(OS_LINUX)
+  num_open_files -= expected_num_open_fds;
 
   write(write_pipe, &num_open_files, sizeof(num_open_files));
   close(write_pipe);
diff --git a/base/run_all_perftests.cc b/base/run_all_perftests.cc
index c0b91c1..84ed48f 100644
--- a/base/run_all_perftests.cc
+++ b/base/run_all_perftests.cc
@@ -2,40 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/command_line.h"
-#include "base/debug_util.h"
-#include "base/perftimer.h"
-#include "base/process_util.h"
-#include "base/string_util.h"
-#include "base/test_suite.h"
-
-class PerfTestSuite : public TestSuite {
- public:
-  PerfTestSuite(int argc, char** argv) : TestSuite(argc, argv) {
-  }
-
-  virtual void Initialize() {
-    // Initialize the perf timer log
-    std::string log_file =
-        WideToUTF8(CommandLine().GetSwitchValue(L"log-file"));
-    if (log_file.empty())
-      log_file = "perf_test.log";
-    ASSERT_TRUE(InitPerfLog(log_file.c_str()));
-
-    // Raise to high priority to have more precise measurements. Since we don't
-    // aim at 1% precision, it is not necessary to run at realtime level.
-    if (!DebugUtil::BeingDebugged())
-      base::RaiseProcessToHighPriority();
-
-    TestSuite::Initialize();
-  }
-
-  virtual void Shutdown() {
-    TestSuite::Shutdown();
-
-    FinalizePerfLog();
-  }
-};
+#include "base/perf_test_suite.h"
 
 int main(int argc, char** argv) {
   return PerfTestSuite(argc, argv).Run();
diff --git a/base/run_all_unittests.cc b/base/run_all_unittests.cc
index e7c7b37..d9f6b58 100644
--- a/base/run_all_unittests.cc
+++ b/base/run_all_unittests.cc
@@ -2,10 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/process_util.h"
 #include "base/test_suite.h"
 
 int main(int argc, char** argv) {
-  base::EnableTerminationOnHeapCorruption();
   return TestSuite(argc, argv).Run();
 }
diff --git a/base/test_suite.h b/base/test_suite.h
index 1c0a785..a9f05b2 100644
--- a/base/test_suite.h
+++ b/base/test_suite.h
@@ -10,11 +10,14 @@
 // any gtest based tests that are linked into your executable.
 
 #include "base/at_exit.h"
+#include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/debug_on_start.h"
+#include "base/file_path.h"
 #include "base/icu_util.h"
 #include "base/logging.h"
 #include "base/multiprocess_test.h"
+#include "base/scoped_nsautorelease_pool.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/multiprocess_func_list.h"
 
@@ -27,16 +30,31 @@
 class TestSuite {
  public:
   TestSuite(int argc, char** argv) {
+    base::ScopedNSAutoreleasePool scoped_pool;
+
+    base::EnableTerminationOnHeapCorruption();
     CommandLine::SetArgcArgv(argc, argv);
     testing::InitGoogleTest(&argc, argv);
 #if defined(OS_LINUX)
     gtk_init_check(&argc, &argv);
 #endif
+
+    FilePath exe;
+    PathService::Get(base::FILE_EXE, &exe);
+    FilePath log_filename = exe.ReplaceExtension(FILE_PATH_LITERAL("log"));
+    logging::InitLogging(log_filename.value().c_str(),
+                         logging::LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG,
+                         logging::LOCK_LOG_FILE,
+                         logging::DELETE_OLD_LOG_FILE);
+    // we want process and thread IDs because we may have multiple processes
+    logging::SetLogItems(true, true, false, true);
   }
 
   virtual ~TestSuite() {}
 
   int Run() {
+    base::ScopedNSAutoreleasePool scoped_pool;
+
     Initialize();
     std::wstring client_func = CommandLine().GetSwitchValue(kRunClientProcess);
     // Check to see if we are being run as a client process.
diff --git a/chrome/common/ipc_tests.cc b/chrome/common/ipc_tests.cc
index ffa7109..be3b0d9 100644
--- a/chrome/common/ipc_tests.cc
+++ b/chrome/common/ipc_tests.cc
@@ -16,8 +16,7 @@
 #include "base/command_line.h"
 #include "base/debug_on_start.h"
 #include "base/perftimer.h"
-#include "base/process_util.h"
-#include "base/scoped_nsautorelease_pool.h"
+#include "base/perf_test_suite.h"
 #include "base/test_suite.h"
 #include "base/thread.h"
 #include "chrome/common/chrome_switches.h"
@@ -435,41 +434,11 @@ MULTIPROCESS_TEST_MAIN(RunReflector) {
 
 #endif  // PERFORMANCE_TEST
 
-#if defined(OS_WIN)
-// All fatal log messages (e.g. DCHECK failures) imply unit test failures
-static void IPCTestAssertHandler(const std::string& str) {
-  FAIL() << str;
-}
-
-// Disable crash dialogs so that it doesn't gum up the buildbot
-static void SuppressErrorDialogs() {
-  UINT new_flags = SEM_FAILCRITICALERRORS |
-                   SEM_NOGPFAULTERRORBOX |
-                   SEM_NOOPENFILEERRORBOX;
-
-  // Preserve existing error mode, as discussed at http://t/dmea
-  UINT existing_flags = SetErrorMode(new_flags);
-  SetErrorMode(existing_flags | new_flags);
-}
-#endif  // defined(OS_WIN)
-
 int main(int argc, char** argv) {
-  base::ScopedNSAutoreleasePool scoped_pool;
-  base::EnableTerminationOnHeapCorruption();
-
-#if defined(OS_WIN)
-  // suppress standard crash dialogs and such unless a debugger is present.
-  if (!IsDebuggerPresent()) {
-    SuppressErrorDialogs();
-    logging::SetLogAssertHandler(IPCTestAssertHandler);
-  }
-#endif  // defined(OS_WIN)
-
-  int retval = TestSuite(argc, argv).Run();
-
 #ifdef PERFORMANCE_TEST
-  if (!InitPerfLog("ipc_perf_child.log"))
-    return 1;
+  int retval = PerfTestSuite(argc, argv).Run();
+#else
+  int retval = TestSuite(argc, argv).Run();
 #endif
   return retval;
 }
diff --git a/chrome/installer/util/run_all_unittests.cc b/chrome/installer/util/run_all_unittests.cc
index e7c7b37..d9f6b58 100644
--- a/chrome/installer/util/run_all_unittests.cc
+++ b/chrome/installer/util/run_all_unittests.cc
@@ -2,10 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/process_util.h"
 #include "base/test_suite.h"
 
 int main(int argc, char** argv) {
-  base::EnableTerminationOnHeapCorruption();
   return TestSuite(argc, argv).Run();
 }
diff --git a/chrome/test/perf/perftests.cc b/chrome/test/perf/perftests.cc
index 18d92aa..da37ba3 100644
--- a/chrome/test/perf/perftests.cc
+++ b/chrome/test/perf/perftests.cc
@@ -2,59 +2,14 @@
 // 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/command_line.h"
-#include "base/debug_util.h"
 #include "base/message_loop.h"
-#include "base/perftimer.h"
-#include "base/process_util.h"
+#include "base/perf_test_suite.h"
 #include "chrome/common/chrome_paths.cc"
-#include "testing/gtest/include/gtest/gtest.h"
-
-// TODO(darin): share code with base/run_all_perftests.cc
 
 int main(int argc, char **argv) {
-  base::AtExitManager exit_manager;
-  base::EnableTerminationOnHeapCorruption();
-  CommandLine::SetArgcArgv(argc, argv);
   chrome::RegisterPathProvider();
   MessageLoop main_message_loop;
 
-  testing::InitGoogleTest(&argc, argv);
-
-  const char log_file_switch[] = "-o";
-  const char* log_filename = NULL;
-  for (int i = 1; i < argc; i++)  {
-    if (strcmp(argv[i], log_file_switch) == 0) {
-      // found the switch for the log file, use the next arg
-      if (i >= argc - 1) {
-        fprintf(stderr, "Log file not specified");
-        return 1;
-      }
-      log_filename = argv[i + 1];
-    }
-  }
-  if (!log_filename) {
-    // use the default filename
-    log_filename = "perf_test.log";
-  }
-  printf("Using output file \"%s\" (change with %s <filename>)\n",
-         log_filename, log_file_switch);
-
-  if (!InitPerfLog(log_filename)) {
-    fprintf(stderr, "Unable to open log file\n");
-    return 1;
-  }
-
-  // Raise to high priority to have more precise measurements. Since we don't
-  // aim at 1% precision, it is not necessary to run at realtime level.
-  if (!DebugUtil::BeingDebugged()) {
-    base::RaiseProcessToHighPriority();
-  }
-
-  int result = RUN_ALL_TESTS();
-
-  FinalizePerfLog();
-  return result;
+  return PerfTestSuite(argc, argv).Run();
 }
 
diff --git a/chrome/test/reliability/run_all_unittests.cc b/chrome/test/reliability/run_all_unittests.cc
index 1f7b2a3..2555872 100644
--- a/chrome/test/reliability/run_all_unittests.cc
+++ b/chrome/test/reliability/run_all_unittests.cc
@@ -2,12 +2,9 @@
 // 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/process_util.h"
 #include "chrome/test/reliability/reliability_test_suite.h"
 
 int main(int argc, char **argv) {
-  base::EnableTerminationOnHeapCorruption();
   return ReliabilityTestSuite(argc, argv).Run();
 }
 
diff --git a/chrome/test/ui/run_all_unittests.cc b/chrome/test/ui/run_all_unittests.cc
index 1670f1c..7745bcaf 100644
--- a/chrome/test/ui/run_all_unittests.cc
+++ b/chrome/test/ui/run_all_unittests.cc
@@ -2,13 +2,10 @@
 // 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/platform_thread.h"
-#include "base/process_util.h"
 #include "chrome/test/ui/ui_test_suite.h"
 
 int main(int argc, char **argv) {
-  base::EnableTerminationOnHeapCorruption();
   PlatformThread::SetName("Tests_Main");
   return UITestSuite(argc, argv).Run();
 }
diff --git a/chrome/test/unit/run_all_unittests.cc b/chrome/test/unit/run_all_unittests.cc
index 8c162f1..5534dbe 100644
--- a/chrome/test/unit/run_all_unittests.cc
+++ b/chrome/test/unit/run_all_unittests.cc
@@ -2,10 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/process_util.h"
 #include "chrome/test/unit/chrome_test_suite.h"
 
 int main(int argc, char **argv) {
-  base::EnableTerminationOnHeapCorruption();
   return ChromeTestSuite(argc, argv).Run();
 }
diff --git a/media/base/run_all_unittests.cc b/media/base/run_all_unittests.cc
index e7c7b37..d9f6b58 100644
--- a/media/base/run_all_unittests.cc
+++ b/media/base/run_all_unittests.cc
@@ -2,10 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/process_util.h"
 #include "base/test_suite.h"
 
 int main(int argc, char** argv) {
-  base::EnableTerminationOnHeapCorruption();
   return TestSuite(argc, argv).Run();
 }