win: Rename CrashpadClient::SetHandler() to SetHandlerIPCPipe()

In https://codereview.chromium.org/1414533006/, I'm adding a few
Mac-specific SetHandler() variants, so it makes sense to name each
SetHandler() variant for what it does.

I'm also making it take a wstring argument, which seems like a more
natural fit for what it does. There should be fewer string conversions
this way.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1406993008 .
This commit is contained in:
Mark Mentovai 2015-11-02 17:00:06 -05:00
parent 740c668e87
commit 7f939285de
9 changed files with 48 additions and 47 deletions

View File

@ -51,9 +51,6 @@ class CrashpadClient {
//! send right corresponding to a receive right held by the handler process. //! send right corresponding to a receive right held by the handler process.
//! The handler process runs an exception server on this port. //! The handler process runs an exception server on this port.
//! //!
//! On Windows, SetHandler() is normally used instead since the handler is
//! started by other means.
//!
//! \param[in] handler The path to a Crashpad handler executable. //! \param[in] handler The path to a Crashpad handler executable.
//! \param[in] database The path to a Crashpad database. The handler will be //! \param[in] database The path to a Crashpad database. The handler will be
//! started with this path as its `--database` argument. //! started with this path as its `--database` argument.
@ -85,10 +82,11 @@ class CrashpadClient {
//! registration is done with a crash handler using the appropriate database //! registration is done with a crash handler using the appropriate database
//! and upload server. //! and upload server.
//! //!
//! \param[in] ipc_port The full name of the crash handler IPC port. //! \param[in] ipc_pipe The full name of the crash handler IPC pipe. This is
//! a string of the form `"\\.\pipe\NAME"`.
//! //!
//! \return `true` on success and `false` on failure. //! \return `true` on success and `false` on failure.
bool SetHandler(const std::string& ipc_port); bool SetHandlerIPCPipe(const std::wstring& ipc_pipe);
//! \brief Requests that the handler capture a dump even though there hasn't //! \brief Requests that the handler capture a dump even though there hasn't
//! been a crash. //! been a crash.
@ -101,7 +99,7 @@ class CrashpadClient {
//! \brief Configures the process to direct its crashes to a Crashpad handler. //! \brief Configures the process to direct its crashes to a Crashpad handler.
//! //!
//! The Crashpad handler must previously have been started by StartHandler() //! The Crashpad handler must previously have been started by StartHandler()
//! or configured by SetHandler(). //! or configured by SetHandlerIPCPipe().
//! //!
//! On Mac OS X, this method sets the tasks exception port for `EXC_CRASH`, //! On Mac OS X, this method sets the tasks exception port for `EXC_CRASH`,
//! `EXC_RESOURCE`, and `EXC_GUARD` exceptions to the Mach send right obtained //! `EXC_RESOURCE`, and `EXC_GUARD` exceptions to the Mach send right obtained
@ -147,7 +145,7 @@ class CrashpadClient {
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
base::mac::ScopedMachSendRight exception_port_; base::mac::ScopedMachSendRight exception_port_;
#elif defined(OS_WIN) #elif defined(OS_WIN)
std::string ipc_port_; std::wstring ipc_pipe_;
#endif #endif
DISALLOW_COPY_AND_ASSIGN(CrashpadClient); DISALLOW_COPY_AND_ASSIGN(CrashpadClient);

View File

@ -115,7 +115,7 @@ std::wstring FormatArgumentString(const std::string& name,
namespace crashpad { namespace crashpad {
CrashpadClient::CrashpadClient() CrashpadClient::CrashpadClient()
: ipc_port_() { : ipc_pipe_() {
} }
CrashpadClient::~CrashpadClient() { CrashpadClient::~CrashpadClient() {
@ -127,13 +127,14 @@ bool CrashpadClient::StartHandler(
const std::string& url, const std::string& url,
const std::map<std::string, std::string>& annotations, const std::map<std::string, std::string>& annotations,
const std::vector<std::string>& arguments) { const std::vector<std::string>& arguments) {
DCHECK(ipc_port_.empty()); DCHECK(ipc_pipe_.empty());
ipc_port_ = std::string ipc_pipe =
base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId()); base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
for (int index = 0; index < 16; ++index) { for (int index = 0; index < 16; ++index) {
ipc_port_.append(1, static_cast<char>(base::RandInt('A', 'Z'))); ipc_pipe.append(1, static_cast<char>(base::RandInt('A', 'Z')));
} }
ipc_pipe_ = base::UTF8ToUTF16(ipc_pipe);
std::wstring command_line; std::wstring command_line;
AppendCommandLineArgument(handler.value(), &command_line); AppendCommandLineArgument(handler.value(), &command_line);
@ -156,8 +157,7 @@ bool CrashpadClient::StartHandler(
base::UTF8ToUTF16(kv.first + '=' + kv.second)), base::UTF8ToUTF16(kv.first + '=' + kv.second)),
&command_line); &command_line);
} }
AppendCommandLineArgument(FormatArgumentString("pipe-name", AppendCommandLineArgument(FormatArgumentString("pipe-name", ipc_pipe_),
base::UTF8ToUTF16(ipc_port_)),
&command_line); &command_line);
STARTUPINFO startup_info = {}; STARTUPINFO startup_info = {};
@ -191,17 +191,17 @@ bool CrashpadClient::StartHandler(
return true; return true;
} }
bool CrashpadClient::SetHandler(const std::string& ipc_port) { bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) {
DCHECK(ipc_port_.empty()); DCHECK(ipc_pipe_.empty());
DCHECK(!ipc_port.empty()); DCHECK(!ipc_pipe.empty());
ipc_port_ = ipc_port; ipc_pipe_ = ipc_pipe;
return true; return true;
} }
bool CrashpadClient::UseHandler() { bool CrashpadClient::UseHandler() {
DCHECK(!ipc_port_.empty()); DCHECK(!ipc_pipe_.empty());
DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE); DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE);
DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE); DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE);
DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE); DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE);
@ -232,8 +232,7 @@ bool CrashpadClient::UseHandler() {
ServerToClientMessage response = {0}; ServerToClientMessage response = {0};
if (!SendToCrashHandlerServer( if (!SendToCrashHandlerServer(ipc_pipe_, message, &response)) {
base::UTF8ToUTF16(ipc_port_), message, &response)) {
return false; return false;
} }

View File

@ -81,7 +81,6 @@
'dependencies': [ 'dependencies': [
'../client/client.gyp:crashpad_client', '../client/client.gyp:crashpad_client',
'../third_party/mini_chromium/mini_chromium.gyp:base', '../third_party/mini_chromium/mini_chromium.gyp:base',
'../tools/tools.gyp:crashpad_tool_support',
'../util/util.gyp:crashpad_util', '../util/util.gyp:crashpad_util',
], ],
'include_dirs': [ 'include_dirs': [
@ -99,7 +98,6 @@
'../compat/compat.gyp:crashpad_compat', '../compat/compat.gyp:crashpad_compat',
'../snapshot/snapshot.gyp:crashpad_snapshot', '../snapshot/snapshot.gyp:crashpad_snapshot',
'../third_party/mini_chromium/mini_chromium.gyp:base', '../third_party/mini_chromium/mini_chromium.gyp:base',
'../tools/tools.gyp:crashpad_tool_support',
'../util/util.gyp:crashpad_util', '../util/util.gyp:crashpad_util',
], ],
'include_dirs': [ 'include_dirs': [
@ -121,7 +119,6 @@
'../client/client.gyp:crashpad_client', '../client/client.gyp:crashpad_client',
'../test/test.gyp:crashpad_test', '../test/test.gyp:crashpad_test',
'../third_party/mini_chromium/mini_chromium.gyp:base', '../third_party/mini_chromium/mini_chromium.gyp:base',
'../tools/tools.gyp:crashpad_tool_support',
], ],
'include_dirs': [ 'include_dirs': [
'..', '..',

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
#include <stdint.h>
#include <stdlib.h> #include <stdlib.h>
#include <windows.h> #include <windows.h>
#include <winternl.h> #include <winternl.h>
@ -28,9 +29,7 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/utf_string_conversions.h"
#include "client/crashpad_client.h" #include "client/crashpad_client.h"
#include "tools/tool_support.h"
#include "util/win/critical_section_with_debug_info.h" #include "util/win/critical_section_with_debug_info.h"
#include "util/win/get_function.h" #include "util/win/get_function.h"
@ -94,17 +93,17 @@ void SomeCrashyFunction() {
*foo = 42; *foo = 42;
} }
int CrashyMain(int argc, char* argv[]) { int CrashyMain(int argc, wchar_t* argv[]) {
CrashpadClient client; CrashpadClient client;
if (argc == 2) { if (argc == 2) {
if (!client.SetHandler(argv[1])) { if (!client.SetHandlerIPCPipe(argv[1])) {
LOG(ERROR) << "SetHandler"; LOG(ERROR) << "SetHandler";
return EXIT_FAILURE; return EXIT_FAILURE;
} }
} else if (argc == 3) { } else if (argc == 3) {
if (!client.StartHandler(base::FilePath(base::UTF8ToUTF16(argv[1])), if (!client.StartHandler(base::FilePath(argv[1]),
base::FilePath(base::UTF8ToUTF16(argv[2])), base::FilePath(argv[2]),
std::string(), std::string(),
std::map<std::string, std::string>(), std::map<std::string, std::string>(),
std::vector<std::string>())) { std::vector<std::string>())) {
@ -138,5 +137,5 @@ int CrashyMain(int argc, char* argv[]) {
} // namespace crashpad } // namespace crashpad
int wmain(int argc, wchar_t* argv[]) { int wmain(int argc, wchar_t* argv[]) {
return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyMain); return crashpad::CrashyMain(argc, argv);
} }

View File

@ -20,7 +20,6 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "client/crashpad_client.h" #include "client/crashpad_client.h"
#include "test/paths.h" #include "test/paths.h"
#include "tools/tool_support.h"
#if !defined(ARCH_CPU_X86) #if !defined(ARCH_CPU_X86)
#error This test is only supported on x86. #error This test is only supported on x86.
@ -29,14 +28,14 @@
namespace crashpad { namespace crashpad {
namespace { namespace {
int CrashyLoadZ7Main(int argc, char* argv[]) { int CrashyLoadZ7Main(int argc, wchar_t* argv[]) {
if (argc != 2) { if (argc != 2) {
fprintf(stderr, "Usage: %s <server_pipe_name>\n", argv[0]); fprintf(stderr, "Usage: %ls <server_pipe_name>\n", argv[0]);
return EXIT_FAILURE; return EXIT_FAILURE;
} }
CrashpadClient client; CrashpadClient client;
if (!client.SetHandler(argv[1])) { if (!client.SetHandlerIPCPipe(argv[1])) {
LOG(ERROR) << "SetHandler"; LOG(ERROR) << "SetHandler";
return EXIT_FAILURE; return EXIT_FAILURE;
} }
@ -69,5 +68,5 @@ int CrashyLoadZ7Main(int argc, char* argv[]) {
} // namespace crashpad } // namespace crashpad
int wmain(int argc, wchar_t* argv[]) { int wmain(int argc, wchar_t* argv[]) {
return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyLoadZ7Main); return crashpad::CrashyLoadZ7Main(argc, argv);
} }

View File

@ -21,7 +21,6 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "client/crashpad_client.h" #include "client/crashpad_client.h"
#include "snapshot/win/process_reader_win.h" #include "snapshot/win/process_reader_win.h"
#include "tools/tool_support.h"
namespace crashpad { namespace crashpad {
namespace { namespace {
@ -65,14 +64,14 @@ bool FreeOwnStackAndBreak() {
return true; return true;
} }
int SelfDestroyingMain(int argc, char* argv[]) { int SelfDestroyingMain(int argc, wchar_t* argv[]) {
if (argc != 2) { if (argc != 2) {
fprintf(stderr, "Usage: %s <server_pipe_name>\n", argv[0]); fprintf(stderr, "Usage: %ls <server_pipe_name>\n", argv[0]);
return EXIT_FAILURE; return EXIT_FAILURE;
} }
CrashpadClient client; CrashpadClient client;
if (!client.SetHandler(argv[1])) { if (!client.SetHandlerIPCPipe(argv[1])) {
LOG(ERROR) << "SetHandler"; LOG(ERROR) << "SetHandler";
return EXIT_FAILURE; return EXIT_FAILURE;
} }
@ -93,5 +92,5 @@ int SelfDestroyingMain(int argc, char* argv[]) {
} // namespace crashpad } // namespace crashpad
int wmain(int argc, wchar_t* argv[]) { int wmain(int argc, wchar_t* argv[]) {
return crashpad::ToolSupport::Wmain(argc, argv, crashpad::SelfDestroyingMain); return crashpad::SelfDestroyingMain(argc, argv);
} }

View File

@ -14,20 +14,25 @@
#include <windows.h> #include <windows.h>
#include "base/files/file_path.h"
#include "base/logging.h" #include "base/logging.h"
#include "client/crashpad_client.h" #include "client/crashpad_client.h"
#include "util/file/file_io.h" #include "util/file/file_io.h"
#include "util/win/address_types.h" #include "util/win/address_types.h"
namespace {
__declspec(noinline) crashpad::WinVMAddress CurrentAddress() { __declspec(noinline) crashpad::WinVMAddress CurrentAddress() {
return reinterpret_cast<crashpad::WinVMAddress>(_ReturnAddress()); return reinterpret_cast<crashpad::WinVMAddress>(_ReturnAddress());
} }
int main(int argc, char* argv[]) { } // namespace
int wmain(int argc, wchar_t* argv[]) {
CHECK_EQ(argc, 2); CHECK_EQ(argc, 2);
crashpad::CrashpadClient client; crashpad::CrashpadClient client;
CHECK(client.SetHandler(argv[1])); CHECK(client.SetHandlerIPCPipe(argv[1]));
CHECK(client.UseHandler()); CHECK(client.UseHandler());
HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);

View File

@ -20,15 +20,19 @@
#include "util/file/file_io.h" #include "util/file/file_io.h"
#include "util/win/address_types.h" #include "util/win/address_types.h"
namespace {
__declspec(noinline) crashpad::WinVMAddress CurrentAddress() { __declspec(noinline) crashpad::WinVMAddress CurrentAddress() {
return reinterpret_cast<crashpad::WinVMAddress>(_ReturnAddress()); return reinterpret_cast<crashpad::WinVMAddress>(_ReturnAddress());
} }
int main(int argc, char* argv[]) { } // namespace
int wmain(int argc, wchar_t* argv[]) {
CHECK_EQ(argc, 2); CHECK_EQ(argc, 2);
crashpad::CrashpadClient client; crashpad::CrashpadClient client;
CHECK(client.SetHandler(argv[1])); CHECK(client.SetHandlerIPCPipe(argv[1]));
CHECK(client.UseHandler()); CHECK(client.UseHandler());
HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);

View File

@ -21,6 +21,7 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "client/crashpad_client.h" #include "client/crashpad_client.h"
#include "gtest/gtest.h" #include "gtest/gtest.h"
#include "test/win/win_child_process.h" #include "test/win/win_child_process.h"
@ -134,7 +135,7 @@ TEST_F(ExceptionHandlerServerTest, StopWhileConnected) {
&server(), &server_thread()); &server(), &server_thread());
ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart()); ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart());
CrashpadClient client; CrashpadClient client;
client.SetHandler(pipe_name()); // Connect to server. client.SetHandlerIPCPipe(base::UTF8ToUTF16(pipe_name()));
// Leaving this scope causes the server to be stopped, while the connection // Leaving this scope causes the server to be stopped, while the connection
// is still open. // is still open.
} }
@ -161,9 +162,9 @@ class TestClient final : public WinChildProcess {
private: private:
int Run() override { int Run() override {
std::string pipe_name = ReadString(ReadPipeHandle()); std::wstring pipe_name = base::UTF8ToUTF16(ReadString(ReadPipeHandle()));
CrashpadClient client; CrashpadClient client;
if (!client.SetHandler(pipe_name)) { if (!client.SetHandlerIPCPipe(pipe_name)) {
ADD_FAILURE(); ADD_FAILURE();
return EXIT_FAILURE; return EXIT_FAILURE;
} }