In release mode, trigger a SIGABRT rather than a SIGTRAP for fatal log errors, i.e. CHECK(false). Also enable tests to make sure we assert/crash as expected.

BUG=none
TEST=Chrome in release mode generates crash dumps when CHECK() fails. See UI tests: AssertionTest.*:CheckFalseTest.*:RendererCrashTest.*
Review URL: http://codereview.chromium.org/830003

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

8 files changed, 131 insertions, 31 deletions

diff --git a/base/debug_util_posix.cc b/base/debug_util_posix.cc
index 0cccd9e..110cb20 100644
--- a/base/debug_util_posix.cc
+++ b/base/debug_util_posix.cc
@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "build/build_config.h"
 #include "base/debug_util.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>
 #include <sys/types.h>
@@ -17,13 +17,13 @@
 #include <cxxabi.h>
 #endif
 
-#include <iostream>
-#include <string>
-
 #if defined(OS_MACOSX)
 #include <AvailabilityMacros.h>
 #endif
 
+#include <iostream>
+#include <string>
+
 #include "base/basictypes.h"
 #include "base/compat_execinfo.h"
 #include "base/eintr_wrapper.h"
@@ -172,7 +172,7 @@ bool DebugUtil::BeingDebugged() {
   size_t info_size = sizeof(info);
 
   int sysctl_result = sysctl(mib, arraysize(mib), &info, &info_size, NULL, 0);
-  DCHECK(sysctl_result == 0);
+  DCHECK_EQ(sysctl_result, 0);
   if (sysctl_result != 0) {
     is_set = true;
     being_debugged = false;
@@ -229,15 +229,33 @@ bool DebugUtil::BeingDebugged() {
   return false;
 }
 
+#endif  // defined(OS_FREEBSD)
+
+// We want to break into the debugger in Debug mode, and cause a crash dump in
+// Release mode. Breakpad behaves as follows:
+//
+// +-------+-----------------+-----------------+
+// | OS    | Dump on SIGTRAP | Dump on SIGABRT |
+// +-------+-----------------+-----------------+
+// | Linux |       N         |        Y        |
+// | Mac   |       Y         |        N        |
+// +-------+-----------------+-----------------+
+//
+// Thus we do the following:
+// Linux: Debug mode, send SIGTRAP; Release mode, send SIGABRT.
+// Mac: Always send SIGTRAP.
+
+#if defined(NDEBUG) && !defined(OS_MACOSX)
+#define DEBUG_BREAK() abort()
+#elif defined(ARCH_CPU_ARM_FAMILY)
+#define DEBUG_BREAK() asm("bkpt 0")
+#else
+#define DEBUG_BREAK() asm("int3")
 #endif
 
 // static
 void DebugUtil::BreakDebugger() {
-#if defined(ARCH_CPU_ARM_FAMILY)
-  asm("bkpt 0");
-#else
-  asm("int3");
-#endif
+  DEBUG_BREAK();
 }
 
 StackTrace::StackTrace() {
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index d4a42df..8eeecfc 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -491,6 +491,9 @@ void BrowserRenderProcessHost::PropagateBrowserCommandLineToRenderer(
   // with any associated values) if present in the browser command line.
   static const char* const switch_names[] = {
     switches::kRendererAssertTest,
+#if !defined(OFFICIAL_BUILD)
+    switches::kRendererCheckFalseTest,
+#endif  // !defined(OFFICIAL_BUILD)
     switches::kRendererCrashTest,
     switches::kRendererStartupDialog,
     switches::kNoSandbox,
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 1229104..97a7f32 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -360,6 +360,7 @@
             'browser/sessions/session_restore_uitest.cc',
             # puts up modal dialogs.
             'browser/unload_uitest.cc',
+            'common/logging_chrome_uitest.cc',
             'test/ui/fast_shutdown_uitest.cc',
             'test/ui/layout_plugin_uitest.cc',
             'test/ui/omnibox_uitest.cc',
@@ -401,7 +402,6 @@
             # to LaunchApp and thus have not been tested for success either).
             'browser/process_singleton_win_uitest.cc',
             'browser/views/find_bar_host_uitest.cc',
-            'common/logging_chrome_uitest.cc',
             'test/ui/sandbox_uitests.cc',
           ],
         }],
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 56528c6..631f39f 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -590,6 +590,11 @@ const char kRemoteShellPort[]               = "remote-shell-port";
 // Causes the renderer process to throw an assertion on launch.
 const char kRendererAssertTest[]            = "renderer-assert-test";
 
+#if !defined(OFFICIAL_BUILD)
+// Causes the renderer process to throw an assertion on launch.
+const char kRendererCheckFalseTest[]        = "renderer-check-false-test";
+#endif
+
 // On POSIX only: the contents of this flag are prepended to the renderer
 // command line. Useful values might be "valgrind" or "xterm -e gdb --args".
 const char kRendererCmdPrefix[]             = "renderer-cmd-prefix";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 134df40..edda3d5 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -253,6 +253,10 @@ extern const char kInvalidateSyncLogin[];
 extern const char kInvalidateSyncXmppLogin[];
 #endif
 
+#if !defined(OFFICIAL_BUILD)
+extern const char kRendererCheckFalseTest[];
+#endif
+
 // DON'T ADD RANDOM STUFF HERE. Put it in the main section above in
 // alphabetical order, or in one of the ifdefs (also in order in each section).
 
diff --git a/chrome/common/logging_chrome_uitest.cc b/chrome/common/logging_chrome_uitest.cc
index 1e75c66..ae6d6c8 100644
--- a/chrome/common/logging_chrome_uitest.cc
+++ b/chrome/common/logging_chrome_uitest.cc
@@ -2,11 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <string>
+#include "build/build_config.h"
+
+#if defined(OS_WIN)
 #include <windows.h>
+#endif
+
+#include <string>
 
-#include "base/command_line.h"
 #include "base/basictypes.h"
+#include "base/command_line.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/logging_chrome.h"
@@ -14,6 +19,8 @@
 #include "chrome/test/ui/ui_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_WIN)
+// TODO(port)
 namespace {
   class ChromeLoggingTest : public testing::Test {
    public:
@@ -62,13 +69,22 @@ TEST_F(ChromeLoggingTest, EnvironmentLogFileName) {
 
   RestoreEnvironmentVariable();
 }
+#endif  // defined(OS_WIN)
+
+#if defined(OS_LINUX) && (!defined(NDEBUG) || !defined(USE_LINUX_BREAKPAD))
+// On Linux in Debug mode, Chrome generates a SIGTRAP.
+// we do not catch SIGTRAPs, thus no crash dump.
+// This also does not work if Breakpad is disabled.
+#define EXPECTED_ASSERT_CRASHES 0
+#else
+#define EXPECTED_ASSERT_CRASHES 1
+#endif
 
-#ifndef NDEBUG  // We don't have assertions in release builds.
+#if !defined(NDEBUG)  // We don't have assertions in release builds.
 // Tests whether we correctly fail on browser assertions during tests.
 class AssertionTest : public UITest {
  protected:
- AssertionTest() : UITest()
-  {
+  AssertionTest() : UITest() {
     // Initial loads will never complete due to assertion.
     wait_for_initial_loads_ = false;
 
@@ -80,23 +96,54 @@ class AssertionTest : public UITest {
 };
 
 // Launch the app in assertion test mode, then close the app.
-TEST_F(AssertionTest, DISABLED_Assertion) {
+#if defined(OS_WIN)
+// http://crbug.com/26715
+#define Assertion DISABLED_Assertion
+#endif
+TEST_F(AssertionTest, Assertion) {
+  if (UITest::in_process_renderer()) {
+    // in process mode doesn't do the crashing.
+    expected_errors_ = 0;
+    expected_crashes_ = 0;
+  } else {
+    expected_errors_ = 1;
+    expected_crashes_ = EXPECTED_ASSERT_CRASHES;
+  }
+}
+#endif  // !defined(NDEBUG)
+
+#if !defined(OFFICIAL_BUILD)
+// Only works on Linux in Release mode with CHROME_HEADLESS=1
+class CheckFalseTest : public UITest {
+ protected:
+  CheckFalseTest() : UITest() {
+    // Initial loads will never complete due to assertion.
+    wait_for_initial_loads_ = false;
+
+    // We're testing the renderer rather than the browser assertion here,
+    // because the browser assertion would flunk the test during SetUp()
+    // (since TAU wouldn't be able to find the browser window).
+    launch_arguments_.AppendSwitch(switches::kRendererCheckFalseTest);
+  }
+};
+
+// Launch the app in assertion test mode, then close the app.
+TEST_F(CheckFalseTest, CheckFails) {
   if (UITest::in_process_renderer()) {
     // in process mode doesn't do the crashing.
     expected_errors_ = 0;
     expected_crashes_ = 0;
   } else {
     expected_errors_ = 1;
-    expected_crashes_ = 1;
+    expected_crashes_ = EXPECTED_ASSERT_CRASHES;
   }
 }
-#endif  // NDEBUG
+#endif  // !defined(OFFICIAL_BUILD)
 
 // Tests whether we correctly fail on browser crashes during UI Tests.
 class RendererCrashTest : public UITest {
  protected:
- RendererCrashTest() : UITest()
-  {
+  RendererCrashTest() : UITest() {
     // Initial loads will never complete due to crash.
     wait_for_initial_loads_ = false;
 
@@ -104,6 +151,13 @@ class RendererCrashTest : public UITest {
   }
 };
 
+#if defined(OS_LINUX) && !defined(USE_LINUX_BREAKPAD)
+// On Linux, do not expect a crash dump if Breakpad is disabled.
+#define EXPECTED_CRASH_CRASHES 0
+#else
+#define EXPECTED_CRASH_CRASHES 1
+#endif
+
 #if defined(OS_WIN)
 // http://crbug.com/32048
 #define Crash FLAKY_Crash
@@ -117,6 +171,6 @@ TEST_F(RendererCrashTest, Crash) {
     scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
     ASSERT_TRUE(browser.get());
     ASSERT_TRUE(browser->WaitForTabCountToBecome(1, action_max_timeout_ms()));
-    expected_crashes_ = 1;
+    expected_crashes_ = EXPECTED_CRASH_CRASHES;
   }
 }
diff --git a/chrome/renderer/renderer_main.cc b/chrome/renderer/renderer_main.cc
index 2556ba7..90a25ea 100644
--- a/chrome/renderer/renderer_main.cc
+++ b/chrome/renderer/renderer_main.cc
@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#if defined(OS_MACOSX)
+#include <signal.h>
+#include <unistd.h>
+#endif  // OS_MACOSX
+
 #include "app/hi_res_timer_manager.h"
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
@@ -31,8 +36,6 @@
 #if defined(OS_MACOSX)
 #include "base/eintr_wrapper.h"
 #include "chrome/app/breakpad_mac.h"
-#include <signal.h>
-#include <unistd.h>
 #endif  // OS_MACOSX
 
 #if defined(USE_LINUX_BREAKPAD)
@@ -152,6 +155,15 @@ static void HandleRendererErrorTestParameters(const CommandLine& command_line) {
     DCHECK(false);
   }
 
+
+#if !defined(OFFICIAL_BUILD)
+  // This parameter causes an assertion too.
+  if (command_line.HasSwitch(switches::kRendererCheckFalseTest)) {
+    CHECK(false);
+  }
+#endif  // !defined(OFFICIAL_BUILD)
+
+
   // This parameter causes a null pointer crash (crash reporter trigger).
   if (command_line.HasSwitch(switches::kRendererCrashTest)) {
     int* bad_pointer = NULL;
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 48a2059..6e753b5 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -144,7 +144,6 @@ UITestBase::UITestBase(MessageLoop::Type msg_loop_type)
       terminate_timeout_ms_(kWaitForTerminateMsec) {
   PathService::Get(chrome::DIR_APP, &browser_directory_);
   PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory_);
-
 }
 
 UITestBase::~UITestBase() {
@@ -182,9 +181,14 @@ void UITestBase::TearDown() {
   // Check for crashes during the test
   FilePath crash_dump_path;
   PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dump_path);
-  // Each crash creates two dump files, so we divide by two here.
   int actual_crashes =
-      file_util::CountFilesCreatedAfter(crash_dump_path, test_start_time_) / 2;
+      file_util::CountFilesCreatedAfter(crash_dump_path, test_start_time_);
+
+#if defined(OS_WIN)
+  // Each crash creates two dump files, so we divide by two here.
+  actual_crashes /= 2;
+#endif
+
   std::wstring error_msg =
       L"Encountered an unexpected crash in the program during this test.";
   if (expected_crashes_ > 0 && actual_crashes == 0) {
@@ -1259,8 +1263,8 @@ void UITestBase::PrintIOPerfInfo(const char* test_name) {
     }
 
     // TODO(sgk):  if/when base::ProcessMetrics returns real stats on mac:
-    //scoped_ptr<base::ProcessMetrics> process_metrics(
-    //    base::ProcessMetrics::CreateProcessMetrics(process_handle));
+    // scoped_ptr<base::ProcessMetrics> process_metrics(
+    //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
     scoped_ptr<ChromeTestProcessMetrics> process_metrics(
         ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
     IoCounters io_counters;
@@ -1362,8 +1366,8 @@ void UITestBase::PrintMemoryUsageInfo(const char* test_name) {
     }
 
     // TODO(sgk):  if/when base::ProcessMetrics returns real stats on mac:
-    //scoped_ptr<base::ProcessMetrics> process_metrics(
-    //    base::ProcessMetrics::CreateProcessMetrics(process_handle));
+    // scoped_ptr<base::ProcessMetrics> process_metrics(
+    //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
     scoped_ptr<ChromeTestProcessMetrics> process_metrics(
         ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));