DRTVWR-447: Add (some) metadata to Mac crash reports.

This required reordering certain operations during Mac viewer startup. Split
llappviewermacosx.cpp's initViewer() function into constructViewer() (which
instantiates LLAppViewerMacOSX) and initViewer() (which calls
LLAppViewerMacOSX::init()).

llappdelegate-objc.mm's applicationDidFinishLaunching override now calls
[BugsplatStartupManager start] between constructViewer() and initViewer(): we
want constructViewer() to have set up the logging subsystem so we can log the
actions of BugsplatStartupManagerDelegate override methods, but otherwise we
want BugsplatStartupManager in place as early as possible to catch any early
crashes. Besides, initViewer() ends up overwriting the static_debug_info.log
on which we depend for the *previous* run's crash metadata.

Move the code that initializes the pathname of the static_debug_info.log file
from LLAppViewerMacOSX::init() to the LLAppViewerMacOSX() constructor, since
BugsplatStartupManagerDelegate override methods need to read (the previous
run's) file.

Add code to applicationLogForBugsplatStartupManager override to set new
BugsplatMac 1.0.6 properties userName and userEmail.

Don't log empty fields from static_debug_info.log if we couldn't read it.

4 files changed, 69 insertions, 33 deletions

diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm
index 66bcf58961..f55304f30b 100644
--- a/indra/newview/llappdelegate-objc.mm
+++ b/indra/newview/llappdelegate-objc.mm
@@ -54,6 +54,25 @@
 
 - (void) applicationDidFinishLaunching:(NSNotification *)notification
 {
+	// Call constructViewer() first so our logging subsystem is in place. This
+	// risks missing crashes in the LLAppViewerMacOSX constructor, but for
+	// present purposes it's more important to get the startup sequence
+	// properly logged.
+	// Someday I would like to modify the logging system so that calls before
+	// it's initialized are cached in a std::ostringstream and then, once it's
+	// initialized, "played back" into whatever handlers have been set up.
+	constructViewer();
+
+#if defined(LL_BUGSPLAT)
+	// Engage BugsplatStartupManager *before* calling initViewer() to handle
+	// any crashes during initialization.
+	// https://www.bugsplat.com/docs/platforms/os-x#initialization
+	[BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES;
+	[BugsplatStartupManager sharedManager].askUserDetails = NO;
+	[BugsplatStartupManager sharedManager].delegate = self;
+	[[BugsplatStartupManager sharedManager] start];
+#endif
+
 	frameTimer = nil;
 
 	[self languageUpdated];
@@ -71,14 +90,6 @@
 	[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(languageUpdated) name:@"NSTextInputContextKeyboardSelectionDidChangeNotification" object:nil];
 
  //   [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self andSelector:@selector(handleGetURLEvent:withReplyEvent:) forEventClass:kInternetEventClass andEventID:kAEGetURL];
-
-#if defined(LL_BUGSPLAT)
-	// https://www.bugsplat.com/docs/platforms/os-x#initialization
-	[BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES;
-	[BugsplatStartupManager sharedManager].askUserDetails = NO;
-	[BugsplatStartupManager sharedManager].delegate = self;
-	[[BugsplatStartupManager sharedManager] start];
-#endif
 }
 
 - (void) handleGetURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)replyEvent {
@@ -198,11 +209,29 @@
 
 - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager
 {
-    std::string fatalMessage(CrashMetadata_instance().fatalMessage);
-    infos("applicationLogForBugsplatStartupManager -> '" + fatalMessage + "'");
-    // This strangely-named override method contributes the User Description
-    // metadata field.
-    return [NSString stringWithCString:fatalMessage.c_str()
+    CrashMetadata& meta(CrashMetadata_instance());
+    // As of BugsplatMac 1.0.6, userName and userEmail properties are now
+    // exposed by the BugsplatStartupManager. Set them here, since the
+    // defaultUserNameForBugsplatStartupManager and
+    // defaultUserEmailForBugsplatStartupManager methods are called later, for
+    // the *current* run, rather than for the previous crashed run whose crash
+    // report we are about to send.
+    infos("applicationLogForBugsplatStartupManager setting userName = '" +
+          meta.agentFullname + '"');
+    bugsplatStartupManager.userName =
+        [NSString stringWithCString:meta.agentFullname.c_str()
+                           encoding:NSUTF8StringEncoding];
+    // Use the email field for OS version, just as we do on Windows, until
+    // BugSplat provides more metadata fields.
+    infos("applicationLogForBugsplatStartupManager setting userEmail = '" +
+          meta.OSInfo + '"');
+    bugsplatStartupManager.userEmail =
+        [NSString stringWithCString:meta.OSInfo.c_str()
+                           encoding:NSUTF8StringEncoding];
+    // This strangely-named override method's return value contributes the
+    // User Description metadata field.
+    infos("applicationLogForBugsplatStartupManager -> '" + meta.fatalMessage + "'");
+    return [NSString stringWithCString:meta.fatalMessage.c_str()
                               encoding:NSUTF8StringEncoding];
 }
 
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index d324a82bf8..846b937a4e 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -707,6 +707,22 @@ LLAppViewer::LLAppViewer()
 	//
 
 	LLLoginInstance::instance().setPlatformInfo(gPlatform, LLOSInfo::instance().getOSVersionString(), LLOSInfo::instance().getOSStringSimple());
+
+	// Under some circumstances we want to read the static_debug_info.log file
+	// from the previous viewer run between this constructor call and the
+	// init() call, which will overwrite the static_debug_info.log file for
+	// THIS run. So setDebugFileNames() early.
+#if LL_BUGSPLAT
+	// MAINT-8917: don't create a dump directory just for the
+	// static_debug_info.log file
+	std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "");
+#else // ! LL_BUGSPLAT
+	// write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues.
+	std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, "");
+#endif // ! LL_BUGSPLAT
+	mDumpPath = logdir;
+	setMiniDumpDir(logdir);
+	setDebugFileNames(logdir);
 }
 
 LLAppViewer::~LLAppViewer()
@@ -781,19 +797,6 @@ bool LLAppViewer::init()
 	initMaxHeapSize() ;
 	LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize"));
 
-#if LL_BUGSPLAT
-	// MAINT-8917: don't create a dump directory just for the
-	// static_debug_info.log file
-	std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "");
-#else // ! LL_BUGSPLAT
-	// write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues.
-	std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, "");
-#endif // ! LL_BUGSPLAT
-	mDumpPath = logdir;
-	setMiniDumpDir(logdir);
-	logdir += gDirUtilp->getDirDelimiter();
-	setDebugFileNames(logdir);
-
 
 	// Although initLoggingAndGetLastDuration() is the right place to mess with
 	// setFatalFunction(), we can't query gSavedSettings until after
diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h
index 79da453cbe..37e8a3917a 100644
--- a/indra/newview/llappviewermacosx-for-objc.h
+++ b/indra/newview/llappviewermacosx-for-objc.h
@@ -24,6 +24,7 @@
 
 #include <string>
 
+void constructViewer();
 bool initViewer();
 void handleUrl(const char* url_utf8);
 bool pumpMainLoop();
diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp
index 77a16f7307..81f04744f8 100644
--- a/indra/newview/llappviewermacosx.cpp
+++ b/indra/newview/llappviewermacosx.cpp
@@ -86,7 +86,7 @@ static void exceptionTerminateHandler()
 	gOldTerminateHandler(); // call old terminate() handler
 }
 
-bool initViewer()
+void constructViewer()
 {
 	// Set the working dir to <bundle>/Contents/Resources
 	if (chdir(gDirUtilp->getAppRODataDir().c_str()) == -1)
@@ -102,18 +102,20 @@ bool initViewer()
 	gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler);
 
 	gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash);
+}
 
-	
+bool initViewer()
+{
 	bool ok = gViewerAppPtr->init();
 	if(!ok)
 	{
 		LL_WARNS() << "Application init failed." << LL_ENDL;
 	}
-    else if (!gHandleSLURL.empty())
-    {
-        dispatchUrl(gHandleSLURL);
-        gHandleSLURL = "";
-    }
+	else if (!gHandleSLURL.empty())
+	{
+		dispatchUrl(gHandleSLURL);
+		gHandleSLURL = "";
+	}
 	return ok;
 }
 
@@ -194,6 +196,7 @@ CrashMetadataSingleton::CrashMetadataSingleton()
         LL_INFOS() << "Can't parse '" << staticDebugPathname
                    << "'; no metadata about previous run" << LL_ENDL;
     }
+    else
     {
         LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL;
         logFilePathname      = get_metadata(info, "SLLog");