Add crash keys for the installer covering simple InstallerState fields.

chrome/installer/setup/setup_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/installer/setup/setup_main.h"
 
 #include <windows.h>
 #include <msi.h>
 #include <shellapi.h>
 #include <shlobj.h>
@@ skipping 1316 matching lines @@
 
   scoped_ptr<metrics::ClientInfo> client_info =
       GoogleUpdateSettings::LoadMetricsClientInfo();
   if (client_info)
     crash_client->SetCrashReporterClientIdFromGUID(client_info->client_id);
   // TODO(grt): A lack of a client_id at this point generally means that Chrome
   // has yet to have been launched and picked one. Consider creating it and
   // setting it here for Chrome to use.
 }
 
+void SetStartingCrashKeys(const InstallerState& installer_state,

[grt (UTC plus 2), 2015/11/25 16:44:09]

wdyt of moving this and the impl of
InstallerCrashReporterClient::RegisterCrashKeys() out into an
installer_crash_keys.{cc,h} pair? this file can expose helper functions called
at various points of installation to set/clear crash keys as appropriate. this
mirrors chrome/common/crash_keys.cc a bit, so it will also help keep the
installer's handling of crash keys more conceptually similar to Chrome's.

[Joe Mason, 2015/11/25 17:16:34]

Good idea. It will help with the more invasive keys like AP value. I'll do it in
a followup when I implement that.

+                          const InstallationState& original_state)
+{
+  scoped_ptr<base::Version> current_version(

[grt (UTC plus 2), 2015/11/25 16:44:09]

CheckPreInstallConditions mutates |installer_state|, so GetCurrentVersion should
not be pulled from it until after that func is called in InstallProducts.

[Joe Mason, 2015/11/25 20:12:52]

installer_state is also mutated by HandleNonInstallCmdLineOptions and
UninstallMultiChromeFrameIfPresent. Do you think it's worth it to set the
current version key at startup and then update it with each mutation?

[grt (UTC plus 2), 2015/11/25 20:34:30]

I think it would be fine to set this crash key within install.cc's
InstallNewVersion when GetCurrentVersion is ordinarily called. I don't think
it's necessary to repeatedly update this particular key.

+      installer_state.GetCurrentVersion(original_state));
+  if (current_version) {
+    base::debug::SetCrashKeyValue(installer::crash_keys::kCurrentVersion,
+                                  current_version->GetString());
+  }
+
+  switch (installer_state.state_type()) {

[grt (UTC plus 2), 2015/11/25 16:44:09]

i think this is cleaner like so:

const char* DistributionTypeToString(BrowserDistribution::Type type) {
  switch (type) {
    case BrowserDistribution::CHROME_BROWSER:
      return "chrome browser";
    case BrowserDistribution::CHROME_FRAME:
      return "chrome frame";
    case BrowserDistribution::CHROME_BINARIES:
      return "chrome binaries";
    case BrowserDistribution::NUM_TYPES:
      break;
  }
  NOTREACHED();
  return "";
}

void SetStartingCrashKeys(...)
{
  base::debug::SetCrashKeyValue(
      installer::crash_keys::kDistributionType,
      DistributionTypeToString(installer_state.state_type()));
}

[Joe Mason, 2015/11/25 17:16:34]

What do you think of making this a BrowserDistribution method?

[grt (UTC plus 2), 2015/11/25 17:44:01]

I think of this mapping being specific to the crash keys, so I think it belongs
with the crash key impl.

+    case BrowserDistribution::CHROME_BROWSER:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kDistributionType,
+                                    "browser");
+      break;
+    case BrowserDistribution::CHROME_FRAME:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kDistributionType,
+                                    "frame");
+      break;
+    case BrowserDistribution::CHROME_BINARIES:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kDistributionType,
+                                    "binaries");
+      break;
+    case BrowserDistribution::NUM_TYPES:
+      // Not a real enum value.
+      assert(false);

[grt (UTC plus 2), 2015/11/25 16:44:09]

use NOTREACHED() (to crash debug builds) or CHECK(FALSE) (to crash debug and
release builds) rather than assert. CHECK(FALSE) should generally be used
sparingly for things that represent security vulns where continuing execution
would be bad. in this case, the invalid value should never happen in practice,
so NOTREACHED() is the right choice.

+      break;
+  }
+
+  switch (installer_state.operation()) {
+    case InstallerState::SINGLE_INSTALL_OR_UPDATE:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kOperation,
+                                    "single-install-or-update");
+      break;
+    case InstallerState::MULTI_INSTALL:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kOperation,
+                                    "multi-install");
+      break;
+    case InstallerState::MULTI_UPDATE:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kOperation,
+                                    "multi-update");
+      break;
+    case InstallerState::UNINSTALL:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kOperation,
+                                    "uninstall");
+      break;
+    case InstallerState::UNINITIALIZED:
+      // Do nothing.
+      break;
+  }
+
+  switch (installer_state.package_type()) {
+    case InstallerState::SINGLE_PACKAGE:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kPackageType,
+                                    "single");
+      break;
+    case InstallerState::MULTI_PACKAGE:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kPackageType,
+                                    "multi");
+      break;
+    case InstallerState::UNKNOWN_PACKAGE_TYPE:
+      // Do nothing.
+      break;
+  }
+
+  switch (installer_state.level()) {
+    case InstallerState::USER_LEVEL:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kPrivilegeLevel,
+                                    "user");
+      break;
+    case InstallerState::SYSTEM_LEVEL:
+      base::debug::SetCrashKeyValue(installer::crash_keys::kPrivilegeLevel,
+                                    "system");
+      break;
+    case InstallerState::UNKNOWN_LEVEL:
+      // Do nothing.
+      break;
+  }
+
+  const std::wstring state_key = installer_state.state_key();
+  if (!state_key.empty()) {
+    base::debug::SetCrashKeyValue(installer::crash_keys::kStateKey,
+                                  base::UTF16ToUTF8(state_key));
+  }
+}
+
 // Uninstalls multi-install Chrome Frame if the current operation is a
 // multi-install install or update. The operation is performed directly rather
 // than delegated to the existing install since there is no facility in older
 // versions of setup.exe to uninstall GCF without touching the binaries. The
 // binaries will be uninstalled during later processing if they are not in-use
 // (see UninstallBinariesIfUnused). |original_state| and |installer_state| are
 // updated to reflect the state of the world following the operation.
 void UninstallMultiChromeFrameIfPresent(const base::CommandLine& cmd_line,
                                         const MasterPreferences& prefs,
                                         InstallationState* original_state,
@@ skipping 351 matching lines @@
   prefs.GetBool(installer::master_preferences::kSystemLevel, &system_install);
   VLOG(1) << "system install is " << system_install;
 
   InstallationState original_state;
   original_state.Initialize();
 
   InstallerState installer_state;
   installer_state.Initialize(cmd_line, prefs, original_state);
 
   ConfigureCrashReporting(installer_state);
+  SetStartingCrashKeys(installer_state, original_state);
 
   // Make sure the process exits cleanly on unexpected errors.
   base::EnableTerminationOnHeapCorruption();
   base::EnableTerminationOnOutOfMemory();
   base::win::RegisterInvalidParamHandler();
   base::win::SetupCRT(cmd_line);
 
   const bool is_uninstall = cmd_line.HasSwitch(installer::switches::kUninstall);
 
   // Check to make sure current system is WinXP or later. If not, log
@@ skipping 106 matching lines @@
     // Note that we allow the status installer::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is
     // never invoked directly by Google Update.
     return_code = InstallUtil::GetInstallReturnCode(install_status);
   }
 
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;
 }