Crashpad Windows: Use the Crashpad client instead of Breakpad on Windows

chrome/app/chrome_exe_main_win.cc

Index: chrome/app/chrome_exe_main_win.cc
diff --git a/chrome/app/chrome_exe_main_win.cc b/chrome/app/chrome_exe_main_win.cc
index d26676fe0f7fb1d552040185afd4098a3fb01221..defbbea61d651bd8888fe77fc0276137b04fb5ea 100644
--- a/chrome/app/chrome_exe_main_win.cc
+++ b/chrome/app/chrome_exe_main_win.cc
@@ -13,6 +13,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/win/windows_version.h"
 #include "chrome/app/main_dll_loader_win.h"
@@ -23,6 +24,7 @@
 #include "chrome_elf/chrome_elf_main.h"
 #include "components/startup_metric_utils/startup_metric_utils.h"
 #include "content/public/common/result_codes.h"
+#include "third_party/crashpad/crashpad/handler/handler_main.h"
 #include "ui/gfx/win/dpi.h"
 
 namespace {
@@ -121,6 +123,28 @@ void SwitchToLFHeap() {
   }
 }
 
+bool RunAsCrashpadHandler(int* rc) {
+  const base::CommandLine command_line =
+      base::CommandLine::FromString(GetCommandLine());
+  if (command_line.HasSwitch("crashpad_handler")) {

[Mark Mentovai, 2015/11/10 17:21:17]

If you’re using command_line, should this go into chrome_switches or something?

[Mark Mentovai, 2015/11/10 17:21:17]

--type=crashpad_handler would probably be more Chromey.

[Mark Mentovai, 2015/11/10 17:21:17]

I know Carlos asked you to change this, but since you’ve got to run through the
whole command line anyway, I don’t know if base::CommandLine gains much, and it
puts more unexpected stuff in between us and handler startup. You’re doing it up
to three times here, and you don’t need at least one of them.

[scottmg, 2015/11/16 21:48:39]

Went with --type=crashpad-handler (_ seems pretty uncommon in Chrome command
line args).

[scottmg, 2015/11/16 21:48:39]

Done.

[scottmg, 2015/11/16 21:48:39]

Switching to base::CommandLine ended up being less code, so that seems somewhat
better. Looking at the current implementation of CommandLine::FromString() it's
not doing too much different than we'd be doing here.

The only thing I could find is that it has a few more options for treating --,
/, etc. as options, but I can't see how they would affect us here.

I'm not sure what you mean by 3 times, iterating through argv you mean?

[Mark Mentovai, 2015/11/16 22:11:13]

scottmg wrote:
Yeah. But not the end of the world.

+    std::vector<base::string16> argv = command_line.argv();
+    argv.erase(std::remove(argv.begin(), argv.end(), L"--crashpad_handler"),
+               argv.end());
+
+    scoped_ptr<char* []> argv_as_utf8(new char*[argv.size() + 1]);
+    std::vector<std::string> storage;
+    storage.reserve(argv.size());
+    for (size_t i = 0; i < argv.size(); ++i) {
+      storage.push_back(base::UTF16ToUTF8(argv[i]));
+      argv_as_utf8[i] = &storage[i][0];
+    }
+    argv_as_utf8[argv.size()] = nullptr;
+    *rc = crashpad::HandlerMain(argv.size(), argv_as_utf8.get());
+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 #if !defined(ADDRESS_SANITIZER)
@@ -131,6 +155,10 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE prev, wchar_t*, int) {
 int main() {
   HINSTANCE instance = GetModuleHandle(NULL);
 #endif
+  int rc;
+  if (RunAsCrashpadHandler(&rc))

[Mark Mentovai, 2015/11/10 17:21:17]

Carry the wchar_t* argument to RunAsCrashpadHandler() so it doesn’t have to call
GetCommandLine(), but I guess you’d still need to call GetCommandLine()
ADDRESS_SANITIZER case, kind of how |instance| is handled.

[scottmg, 2015/11/16 21:48:39]

wWinMain's "command line" argument doesn't include the program name which
confuses CommandLine. I guess that's an argument for doing the processing
ourselves, but we'd have to add a [fake] program name for argv[0] for
HandlerMain() anyway.

+    return rc;

[Mark Mentovai, 2015/11/10 17:21:17]

It’d be cool to someday put the client init right here.

[scottmg, 2015/11/16 21:48:39]

Yeah, I'm not sure if there's anything stopping us from doing that. I think
maybe a few small things like the Paths initialization order that I fiddled with
a bit.

+
   SwitchToLFHeap();
 
   startup_metric_utils::RecordExeMainEntryPointTime(base::Time::Now());
@@ -155,7 +183,7 @@ int main() {
   // Load and launch the chrome dll. *Everything* happens inside.
   VLOG(1) << "About to load main DLL.";
   MainDllLoader* loader = MakeMainDllLoader();
-  int rc = loader->Launch(instance);
+  rc = loader->Launch(instance);
   loader->RelaunchChromeBrowserWithNewCommandLineIfNeeded();
   delete loader;
   return rc;