Stability instrumentation Crashpad integration

chrome/install_static/product_install_details.cc

Index: chrome/install_static/product_install_details.cc
diff --git a/chrome/install_static/product_install_details.cc b/chrome/install_static/product_install_details.cc
index 3e8bec502296492e070c3f1465554634666a72e8..0159125eb0556be461adc49fcaaa97aef16450af 100644
--- a/chrome/install_static/product_install_details.cc
+++ b/chrome/install_static/product_install_details.cc
@@ -11,6 +11,7 @@
 #include "chrome/install_static/install_details.h"
 #include "chrome/install_static/install_modes.h"
 #include "chrome/install_static/install_util.h"
+#include "chrome/install_static/user_data_dir.h"
 #include "chrome_elf/nt_registry/nt_registry.h"
 
 namespace install_static {
@@ -139,6 +140,13 @@ std::unique_ptr<PrimaryInstallDetails> MakeProductDetails(
   details->set_update_ap(update_ap);
   details->set_update_cohort_name(update_cohort_name);
 
+  if (CurrentProcessNeedsProfileDir()) {
+    std::wstring user_data_dir;
+    // DO NOT SUBMIT: how should we deal with a failure here?

[grt (UTC plus 2), 2017/05/19 08:15:50]

based on the comment in GetUserDataDirectoryImpl, we should be sure that there's
a CHECK *after* crash handling is set up.

+scottmg?

[scottmg, 2017/05/19 17:46:52]

Right, ideally we'd CHECK somewhere later so that a crash could actually be
uploaded if this happens. Currently this would be after
SignalInitializeCrashReporting() in WinMain().

[manzagop (departed), 2017/05/19 22:02:07]

Is an empty string an indicator of invalid user-data-dir? I've seen a few
DCHECKs that seem to imply so.

Assuming that's the case, I've placed a CHECK(!user_data_dir.empty()) in
InitializeUserDataDir which is in PreSandboxStartup.

[manzagop (departed), 2017/05/19 22:02:07]

IIUC this is called before crash reporting is set up [1] and we make use of the
result to setup up crash reporting [2]. Do we need to store the success in
product details?

[1] https://cs.chromium.org/chromium/src/chrome_elf/chrome_elf_main.cc?l=49
[2]
https://cs.chromium.org/chromium/src/components/crash/content/app/crashpad_wi...

[scottmg, 2017/05/19 22:28:06]

Wow, this is confusing. I think what you have in ps16 makes sense. Maybe just
add a comment to the CHECK in PreSandboxStartup() that an empty string means
that no fallback/default user-data-dir could be retrieved either, otherwise it
makes it look like the "show a dialog for invalid directory" might not be
called.

(Also, maybe manually test that `--user-data-dir=q:\bad_drive` does actually
work the same as Stable. :)

[manzagop (departed), 2017/05/23 14:59:14]

Done.
Done. I get a dialog and it falls back to default.

+    install_static::GetUserDataDirectory(*mode, &user_data_dir, nullptr);
+    details->set_user_data_dir(user_data_dir);
+  }
+
   return details;
 }