Don't print the callstack in forked browser test processes. This may allocate memory or try to acquire OS locks which may already be locked from the parent process. This was happening because there's a race condition where the browser process forks a process and shuts it down very soon after, before it calls exec*.

BUG=141302,139209,36678
Review URL: https://chromiumcodereview.appspot.com/10836148

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

5 files changed, 43 insertions, 51 deletions

diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc
index cc70f10..bb0e546 100644
--- a/chrome/test/base/in_process_browser_test.cc
+++ b/chrome/test/base/in_process_browser_test.cc
@@ -7,7 +7,6 @@
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/debug/stack_trace.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
@@ -60,9 +59,6 @@ const char kBrowserTestType[] = "browser";
 
 InProcessBrowserTest::InProcessBrowserTest()
     : browser_(NULL)
-#if defined(OS_POSIX)
-      , handle_sigterm_(true)
-#endif
 #if defined(OS_MACOSX)
       , autorelease_pool_(NULL)
 #endif  // OS_MACOSX
@@ -316,22 +312,7 @@ CommandLine InProcessBrowserTest::GetCommandLineForRelaunch() {
 }
 #endif
 
-#if defined(OS_POSIX)
-// On SIGTERM (sent by the runner on timeouts), dump a stack trace (to make
-// debugging easier) and also exit with a known error code (so that the test
-// framework considers this a failure -- http://crbug.com/57578).
-static void DumpStackTraceSignalHandler(int signal) {
-  base::debug::StackTrace().PrintBacktrace();
-  _exit(128 + signal);
-}
-#endif  // defined(OS_POSIX)
-
 void InProcessBrowserTest::RunTestOnMainThreadLoop() {
-#if defined(OS_POSIX)
-  if (handle_sigterm_)
-    signal(SIGTERM, DumpStackTraceSignalHandler);
-#endif  // defined(OS_POSIX)
-
   // Pump startup related events.
   content::RunAllPendingInMessageLoop();
 
diff --git a/chrome/test/base/in_process_browser_test.h b/chrome/test/base/in_process_browser_test.h
index 6146dab..a404906 100644
--- a/chrome/test/base/in_process_browser_test.h
+++ b/chrome/test/base/in_process_browser_test.h
@@ -173,14 +173,6 @@ class InProcessBrowserTest : public BrowserTestBase {
     return host_resolver_.get();
   }
 
-#if defined(OS_POSIX)
-  // This is only needed by a test that raises SIGTERM to ensure that a specific
-  // codepath is taken.
-  void DisableSIGTERMHandling() {
-    handle_sigterm_ = false;
-  }
-#endif
-
 #if defined(OS_MACOSX)
   // Returns the autorelease pool in use inside RunTestOnMainThreadLoop().
   base::mac::ScopedNSAutoreleasePool* AutoreleasePool() const {
@@ -213,10 +205,6 @@ class InProcessBrowserTest : public BrowserTestBase {
   // specified in the command line.
   ScopedTempDir temp_user_data_dir_;
 
-#if defined(OS_POSIX)
-  bool handle_sigterm_;
-#endif
-
 #if defined(OS_CHROMEOS)
   chromeos::ScopedStubCrosEnabler stub_cros_enabler_;
 #endif  // defined(OS_CHROMEOS)
diff --git a/content/test/browser_test_base.cc b/content/test/browser_test_base.cc
index 5e805b5..2e33fa0 100644
--- a/content/test/browser_test_base.cc
+++ b/content/test/browser_test_base.cc
@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/debug/stack_trace.h"
+#include "base/process_util.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/main_function_params.h"
 #include "sandbox/win/src/dep.h"
@@ -17,11 +19,34 @@
 
 extern int BrowserMain(const content::MainFunctionParams&);
 
+namespace {
+
+#if defined(OS_POSIX)
+// On SIGTERM (sent by the runner on timeouts), dump a stack trace (to make
+// debugging easier) and also exit with a known error code (so that the test
+// framework considers this a failure -- http://crbug.com/57578).
+// Note: We only want to do this in the browser process, and not forked
+// processes. That might lead to hangs because of locks inside tcmalloc or the
+// OS. See http://crbug.com/141302.
+static int g_browser_process_pid;
+static void DumpStackTraceSignalHandler(int signal) {
+  if (g_browser_process_pid == base::GetCurrentProcId())
+    base::debug::StackTrace().PrintBacktrace();
+  _exit(128 + signal);
+}
+#endif  // defined(OS_POSIX)
+
+}  // namespace
+
 BrowserTestBase::BrowserTestBase() {
 #if defined(OS_MACOSX)
   base::mac::SetOverrideAmIBundled(true);
   base::SystemMonitor::AllocateSystemIOPorts();
 #endif
+
+#if defined(OS_POSIX)
+  handle_sigterm_ = true;
+#endif
 }
 
 BrowserTestBase::~BrowserTestBase() {
@@ -49,6 +74,12 @@ void BrowserTestBase::TearDown() {
 }
 
 void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
+#if defined(OS_POSIX)
+  if (handle_sigterm_) {
+    g_browser_process_pid = base::GetCurrentProcId();
+    signal(SIGTERM, DumpStackTraceSignalHandler);
+  }
+#endif  // defined(OS_POSIX)
   RunTestOnMainThreadLoop();
 }
 
diff --git a/content/test/browser_test_base.h b/content/test/browser_test_base.h
index 430cccb..90a639f 100644
--- a/content/test/browser_test_base.h
+++ b/content/test/browser_test_base.h
@@ -60,6 +60,14 @@ class BrowserTestBase : public testing::Test {
   const net::TestServer* test_server() const { return test_server_.get(); }
   net::TestServer* test_server() { return test_server_.get(); }
 
+#if defined(OS_POSIX)
+  // This is only needed by a test that raises SIGTERM to ensure that a specific
+  // codepath is taken.
+  void DisableSIGTERMHandling() {
+    handle_sigterm_ = false;
+  }
+#endif
+
   // This function is meant only for classes that directly derive from this
   // class to construct the test server in their constructor. They might need to
   // call this after setting up the paths. Actual test cases should never call
@@ -73,6 +81,10 @@ class BrowserTestBase : public testing::Test {
 
   // Testing server, started on demand.
   scoped_ptr<net::TestServer> test_server_;
+
+#if defined(OS_POSIX)
+  bool handle_sigterm_;
+#endif
 };
 
 #endif  // CONTENT_TEST_BROWSER_TEST_BASE_H_
diff --git a/content/test/content_browser_test.cc b/content/test/content_browser_test.cc
index 0fab44b..fb484c5 100644
--- a/content/test/content_browser_test.cc
+++ b/content/test/content_browser_test.cc
@@ -5,7 +5,6 @@
 #include "content/test/content_browser_test.h"
 
 #include "base/command_line.h"
-#include "base/debug/stack_trace.h"
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -50,11 +49,6 @@ void ContentBrowserTest::SetUp() {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   command_line->AppendSwitch(switches::kContentBrowserTest);
 
-#if defined(OS_LINUX)
-  // http://crbug.com/139209
-  command_line->AppendSwitch(switches::kDisableGpuProcessPrelaunch);
-#endif
-
   SetUpCommandLine(command_line);
 
   // Single-process mode is not set in BrowserMain, so process it explicitly,
@@ -87,24 +81,10 @@ void ContentBrowserTest::TearDown() {
   shell_main_delegate_.reset();
 }
 
-#if defined(OS_POSIX)
-// On SIGTERM (sent by the runner on timeouts), dump a stack trace (to make
-// debugging easier) and also exit with a known error code (so that the test
-// framework considers this a failure -- http://crbug.com/57578).
-static void DumpStackTraceSignalHandler(int signal) {
-  base::debug::StackTrace().PrintBacktrace();
-  _exit(128 + signal);
-}
-#endif  // defined(OS_POSIX)
-
 void ContentBrowserTest::RunTestOnMainThreadLoop() {
   CHECK_EQ(Shell::windows().size(), 1u);
   shell_ = Shell::windows()[0];
 
-#if defined(OS_POSIX)
-  signal(SIGTERM, DumpStackTraceSignalHandler);
-#endif  // defined(OS_POSIX)
-
 #if defined(OS_MACOSX)
   // On Mac, without the following autorelease pool, code which is directly
   // executed (as opposed to executed inside a message loop) would autorelease