Commit d3b1599c authored by scottmg's avatar scottmg Committed by Commit bot

Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664

Review-Url: https://codereview.chromium.org/2279943002
Cr-Commit-Position: refs/heads/master@{#414861}
parent 0cf5d675
......@@ -24,31 +24,6 @@ namespace {
base::LazyInstance<std::vector<crash_reporter::Report>>::Leaky g_crash_reports =
LAZY_INSTANCE_INITIALIZER;
// Gets the exe name from the full path of the exe.
base::string16 GetExeName() {
wchar_t file_path[MAX_PATH] = {};
if (!::GetModuleFileName(nullptr, file_path, arraysize(file_path))) {
assert(false);
return base::string16();
}
base::string16 file_name_string = file_path;
size_t last_slash_pos = file_name_string.find_last_of(L'\\');
if (last_slash_pos != base::string16::npos) {
file_name_string = file_name_string.substr(
last_slash_pos + 1, file_name_string.length() - last_slash_pos);
}
std::transform(file_name_string.begin(), file_name_string.end(),
file_name_string.begin(), ::tolower);
return file_name_string;
}
void InitializeCrashReportingForProcess() {
// We want to initialize crash reporting only in chrome.exe
if (GetExeName() != L"chrome.exe")
return;
ChromeCrashReporterClient::InitializeCrashReportingForProcess();
}
#if !defined(ADDRESS_SANITIZER)
// chrome_elf loads early in the process and initializes Crashpad. That in turn
// uses the SetUnhandledExceptionFilter API to set a top level exception
......@@ -110,7 +85,7 @@ extern "C" __declspec(dllexport) void SetMetricsClientId(
BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {
if (reason == DLL_PROCESS_ATTACH) {
InitializeCrashReportingForProcess();
ChromeCrashReporterClient::InitializeCrashReportingForProcess();
// CRT on initialization installs an exception filter which calls
// TerminateProcess. We need to hook CRT's attempt to set an exception
// handler and ignore it. Don't do this when ASan is present, or ASan will
......
......@@ -9,12 +9,14 @@
#include <vector>
#include "base/base_paths.h"
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/files/file_path.h"
#include "base/files/memory_mapped_file.h"
#include "base/path_service.h"
#include "base/strings/pattern.h"
#include "base/strings/string_util.h"
#include "base/test/launcher/test_launcher.h"
#include "base/win/pe_image.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -63,6 +65,7 @@ class ELFImportsTest : public testing::Test {
// If you break this test, you may have changed base or the Windows sandbox
// such that more system imports are required to link.
#if defined(NDEBUG) && !defined(COMPONENT_BUILD)
TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
base::FilePath dll;
ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dll));
......@@ -103,7 +106,37 @@ TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
ASSERT_TRUE(match) << "Illegal import in chrome_elf.dll: " << import;
}
}
TEST_F(ELFImportsTest, ChromeElfLoadSanityTest) {
// chrome_elf will try to launch crashpad_handler by reinvoking the current
// binary with --type=crashpad-handler if not already running that way. To
// avoid that, we relaunch and run the real test body manually, adding that
// command line argument, as we're only trying to confirm that user32.dll
// doesn't get loaded by import table when chrome_elf.dll does.
base::CommandLine new_test =
base::CommandLine(base::CommandLine::ForCurrentProcess()->GetProgram());
new_test.AppendSwitchASCII(
base::kGTestFilterFlag,
"ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl");
new_test.AppendSwitchASCII("type", "crashpad-handler");
new_test.AppendSwitch("gtest_also_run_disabled_tests");
new_test.AppendSwitch("single-process-tests");
std::string output;
ASSERT_TRUE(base::GetAppOutput(new_test, &output));
std::string crash_string =
"OK ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl";
if (output.find(crash_string) == std::string::npos) {
GTEST_FAIL() << "Couldn't find\n" << crash_string << "\n in output\n "
<< output;
}
}
// Note: This test is not actually disabled, it's just tagged disabled so that
// the real run (above, in ChromeElfLoadSanityTest) can run it with an argument
// added to the command line.
TEST_F(ELFImportsTest, DISABLED_ChromeElfLoadSanityTestImpl) {
base::FilePath dll;
ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dll));
dll = dll.Append(L"chrome_elf.dll");
......@@ -119,7 +152,8 @@ TEST_F(ELFImportsTest, ChromeElfLoadSanityTest) {
EXPECT_EQ(nullptr, ::GetModuleHandle(L"user32.dll"));
EXPECT_TRUE(!!::FreeLibrary(chrome_elf_module_handle));
}
#endif // NDEBUG
#endif // NDEBUG && !COMPONENT_BUILD
TEST_F(ELFImportsTest, ChromeExeSanityCheck) {
std::vector<std::string> exe_imports;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment