Make Crashpad start asynchronous, and move back to chrome_elf

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 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 "chrome/browser/chrome_browser_main.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 195 matching lines @@
 #include "chrome/browser/downgrade/user_data_downgrade.h"
 #include "chrome/browser/first_run/try_chrome_dialog_view.h"
 #include "chrome/browser/first_run/upgrade_util_win.h"
 #include "chrome/browser/ui/network_profile_bubble.h"
 #include "chrome/browser/win/browser_util.h"
 #include "chrome/browser/win/chrome_select_file_dialog_factory.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/shell_util.h"
+#include "components/crash/content/app/crashpad.h"
 #include "ui/base/l10n/l10n_util_win.h"
 #include "ui/shell_dialogs/select_file_dialog.h"
 #endif  // defined(OS_WIN)
 
 #if defined(OS_MACOSX)
 #include <Security/Security.h>
 
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "chrome/browser/mac/keystone_glue.h"
 #endif  // defined(OS_MACOSX)
@@ skipping 1178 matching lines @@
   // browser windows and tabs, but no progress has been made in loading
   // content as the main message loop hasn't started processing tasks yet.
   // We setup to observe to the initial page load here to defer running
   // task posted via PostAfterStartupTask until its complete.
   AfterStartupTaskUtils::StartMonitoringStartup();
 }
 
 int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::PreMainMessageLoopRunImpl");
 
+#if defined(OS_WIN)
+  HMODULE chrome_elf = GetModuleHandle(chrome::kChromeElfDllName);
+  if (chrome_elf) {
+    auto block_until_handler_started = reinterpret_cast<void (*)()>(
+        GetProcAddress(chrome_elf, "BlockUntilHandlerStartedImpl"));

[robertshield, 2016/12/09 01:56:44]

This isn't in elf's .def file. Should it be?

[scottmg, 2016/12/09 20:23:14]

We seem to have a mix of __declspec(dllexport) and .def export. I was following
the others in the file where BlockUntilHandlerStartedImpl was added, which are
in components/... rather than chrome_elf/

I did confirm that it is indeed in the /exports of chrome_elf.dll though.

[robertshield, 2016/12/09 21:11:47]

No worries, just thought it was a bit weird that we have both ways of declaring
exports. Might be nice to just use one (would probably pick the declspec
approach).

FWIW it's also a little odd that a number of the dll exports are called FooImpl
but maybe that's fine since chrome.exe and chrome_elf.dll are so tightly
coupled. 

Anyway, this comment feels like nits, feel free to ignore entirely if you'd
like. 

[scottmg, 2016/12/13 18:46:59]

Yeah, it's a bit messy. I think "Impl" has been used to just mean "Thunk" or
"Helper" or something. We should clean all those up, but I'm going to go with
deferring that to a later CL for the purposes of this change.

+    if (block_until_handler_started) {
+      SCOPED_UMA_HISTOGRAM_TIMER("Startup.BlockForCrashpadHandlerStartupTime");
+      block_until_handler_started();

[robertshield, 2016/12/09 21:11:47]

This is a WaitForSingleObject(.., INFINITE) at Chrome startup. If something goes
wrong here does Chrome hang forever or will a watchdog step in and crash the
process / send us a minidump? If the former, would it be worth using a
non-INFINITE wait instead?

I can't see an obvious case in which the wait wouldn't return immediately, but
there might be an unobvious case (some 3rd party module's DLL_THREAD_ATTACH
taking forever or some such). 

Maybe this is fine, but figured I'd ask. 

[scottmg, 2016/12/13 18:46:59]

Updated so that it waits a maximum of 5s. There's probably still some case where
the act of WFSO could be bad if the process is really screwed, but at least a
temporary wait is better than an INFINITE wait.

+    }
+  }
+#endif
+
   SCOPED_UMA_HISTOGRAM_LONG_TIMER("Startup.PreMainMessageLoopRunImplLongTime");
   const base::TimeTicks start_time_step1 = base::TimeTicks::Now();
 
   // This must occur at PreMainMessageLoopRun because |SetupMetrics()| uses the
   // blocking pool, which is disabled until the CreateThreads phase of startup.
   SetupMetrics();
 
 #if defined(OS_WIN)
   // Windows parental controls calls can be slow, so we do an early init here
   // that calculates this value off of the UI thread.
@@ skipping 671 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif  // defined(OS_CHROMEOS)
 #endif  // defined(OS_ANDROID)
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }