Fix up crash reporting in unit tests and the Chrome Frame helper processes:

chrome_frame/crash_server_init.cc

Index: chrome_frame/crash_server_init.cc
===================================================================
--- chrome_frame/crash_server_init.cc	(revision 88766)
+++ chrome_frame/crash_server_init.cc	(working copy)
@@ -4,6 +4,8 @@
 
 #include "chrome_frame/crash_server_init.h"
 
+#include <sddl.h>
+#include <Shlobj.h>
 #include "version.h"  // NOLINT
 
 const wchar_t kChromePipeName[] = L"\\\\.\\pipe\\ChromeCrashServices";
@@ -17,6 +19,51 @@
     MiniDumpWithUnloadedModules |  // Get unloaded modules when available.
     MiniDumpWithIndirectlyReferencedMemory);  // Get memory referenced by stack.
 
+// Builds a string representation of the user's SID and places it in user_sid.
+bool GetUserSidString(std::wstring* user_sid) {
+  bool success = false;
+  if (user_sid) {
+    const DWORD kSize = sizeof(TOKEN_USER) + SECURITY_MAX_SID_SIZE;

[grt (UTC plus 2), 2011/06/22 14:15:01]

Google style guide says to use sizeof(varname) rather than (typename) wherever
possible.  That aside, consider the technique used in
http://msdn.microsoft.com/en-us/library/ee207402(v=VS.85).aspx since it doesn't
require sizeof computations or a cast.

[robertshield, 2011/06/23 02:22:51]

Good idea, much nicer.

+    BYTE buffer[kSize];
+    TOKEN_USER* user = reinterpret_cast<TOKEN_USER*>(buffer);
+
+    HANDLE token = NULL;
+    if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) {
+      DWORD out_size;
+      if (GetTokenInformation(token, TokenUser, user, kSize, &out_size)) {
+        wchar_t* user_sid_value = NULL;
+        if (user->User.Sid && ConvertSidToStringSid(user->User.Sid,
+                                                    &user_sid_value)) {
+          *user_sid = user_sid_value;
+          LocalFree(user_sid_value);
+          user_sid_value = NULL;
+          success = true;
+        }
+      }
+      CloseHandle(token);
+    }
+  }
+
+  return success;
+}
+
+bool IsRunningSystemInstall() {
+  wchar_t exe_path[MAX_PATH * 2] = {0};
+  GetModuleFileName(NULL, exe_path, MAX_PATH * 2);

[grt (UTC plus 2), 2011/06/22 14:15:01]

Will this not return the path to iexplore.exe when called from the context of
npchrome_frame.dll?  Is this desired?  If not, see base\base_paths_win.cc for
the technique to get the location of the dll.

[grt (UTC plus 2), 2011/06/22 14:15:01]

Rather than repeat MAX_PATH * 2, how about _countof?

[robertshield, 2011/06/23 02:22:51]

This code is only linked into chrome_launcher.exe and chrome_frame_helper.exe
but the __ImageBase thing makes it a bit safer. Changed, thanks.

[robertshield, 2011/06/23 02:22:51]

Done.

+
+  bool is_system = false;
+
+  wchar_t program_files_path[MAX_PATH] = {0};
+  if (SUCCEEDED(SHGetFolderPath(NULL, CSIDL_PROGRAM_FILES, NULL,
+                                SHGFP_TYPE_CURRENT, program_files_path))) {
+    if (wcsstr(exe_path, program_files_path)) {

[grt (UTC plus 2), 2011/06/22 14:15:01]

To be strictly correct, this should be:
  if (wcsstr(...) == exe_path)
although it will still do more work than needed if a match isn't found since all
of exe_path is searched for program_files_path.

[robertshield, 2011/06/23 02:22:51]

Done.

+      is_system = true;
+    }
+  }
+
+  return is_system;
+}
+
 google_breakpad::CustomClientInfo* GetCustomInfo() {
   static google_breakpad::CustomInfoEntry ver_entry(
       L"ver", TEXT(CHROME_VERSION_STRING));
@@ -31,22 +78,31 @@
 }
 
 google_breakpad::ExceptionHandler* InitializeCrashReporting(
-    const wchar_t* cmd_line) {
-  if (cmd_line == NULL) {
-    return NULL;
-  }
-
+    CrashReportingFlags flags) {
   wchar_t temp_path[MAX_PATH + 1] = {0};
   DWORD path_len = ::GetTempPath(MAX_PATH, temp_path);
 
   std::wstring pipe_name;
-  if (wcsstr(cmd_line, kFullMemoryCrashReport) != NULL) {
+  if (flags == HEADLESS) {
+    // This flag is used for testing, connect to the test crash service.
     pipe_name = kChromePipeName;
   } else {
-    // TODO(robertshield): Figure out if we're a per-user install and connect
-    // to the per-user named pipe instead.
+    // Otherwise, build a pipe name corresponding to either user or
+    // system-level Omaha.
     pipe_name = kGoogleUpdatePipeName;
-    pipe_name += kSystemPrincipalSid;
+    if (IsRunningSystemInstall()) {
+      pipe_name += kSystemPrincipalSid;
+    } else {
+      std::wstring user_sid;
+      if (GetUserSidString(&user_sid)) {
+        pipe_name += user_sid;
+      } else {
+        // We don't think we're a system install, but we couldn't get the
+        // user SID. Try connecting to the system-level crash service as a
+        // last ditch effort.
+        pipe_name += kSystemPrincipalSid;
+      }
+    }
   }
 
   google_breakpad::ExceptionHandler* breakpad =