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

chrome/app/chrome_exe_main_win.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <windows.h>
 #include <malloc.h>
 #include <shellscalingapi.h>
 #include <tchar.h>
 
 #include <string>
 
 #include "base/at_exit.h"
 #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"
 #include "chrome/browser/chrome_process_finder_win.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #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 {
 // List of switches that it's safe to rendezvous early with. Fast start should
 // not be done if a command line contains a switch not in this set.
 // Note this is currently stored as a list of two because it's probably faster
 // to iterate over this small array than building a map for constant time
 // lookups.
 const char* const kFastStartSwitches[] = {
   switches::kProfileDirectory,
@@ skipping 78 matching lines @@
   // Only needed on XP but harmless on other Windows flavors.
   auto crt_heap = _get_heap_handle();
   ULONG enable_LFH = 2;
   if (HeapSetInformation(reinterpret_cast<HANDLE>(crt_heap),
                          HeapCompatibilityInformation,
                          &enable_LFH, sizeof(enable_LFH))) {
     VLOG(1) << "Low fragmentation heap enabled.";
   }
 }
 
+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)
 int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE prev, wchar_t*, int) {
 #else
 // The AddressSanitizer build should be a console program as it prints out stuff
 // on stderr.
 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());
 
   // Signal Chrome Elf that Chrome has begun to start.
   SignalChromeElf();
 
   // Initialize the commandline singleton from the environment.
   base::CommandLine::Init(0, NULL);

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

If you moved this above RunAsCrashpadHandler(), it’d kind of justify using
CommandLine a bit more, because you could use FromCurrentProcess(). But this
should most likely not move above most of the other stuff that precedes it, and
that other stuff may be undesirable for crashpad_handler.

Dependencies. Yuck!

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

Yeah, I moved it up, but then moved it back down again. I think it might be OK
to move it up and share initialization from looking at the code that gets called
between here and there.

   // The exit manager is in charge of calling the dtors of singletons.
   base::AtExitManager exit_manager;
 
   // We don't want to set DPI awareness on pre-Win7 because we don't support
   // DirectWrite there. GDI fonts are kerned very badly, so better to leave
   // DPI-unaware and at effective 1.0. See also ShouldUseDirectWrite().
   if (base::win::GetVersion() >= base::win::VERSION_WIN7)
     EnableHighDPISupport();
 
   if (AttemptFastNotify(*base::CommandLine::ForCurrentProcess()))
     return 0;
 
   // 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;
 }