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.
Introduce CrashMetadata, an LLSingleton in llappviewermacosx.cpp, declared in
llappviewermacosx-for-objc.h and accessed by the various
BugsplatStartupManagerDelegate override methods. CrashMetadata is populated by
reading the previous (presumably crashed) run's static_debug_info.log file.
This replaces the previous getOldLogFilePathname(), getFatalMessage() and
getAgentFullname() functions. To extend that suite for additional metadata,
not only would we have to keep adding new free functions, but we'd have to
keep rereading the static_debug_info.log file.
Override the new applicationKeyForBugsplatStartupManager,
defaultUserNameForBugsplatStartupManager,
defaultUserEmailForBugsplatStartupManager methods to extract relevant fields
from CrashMetadata. Change applicationLogForBugsplatStartupManager and
attachmentForBugsplatStartupManager to do the same.
Enhance llviewerregion.cpp to update the static_debug_info.log file every
time we enter a new region.
Direct BugSplat to send crash reports without prompting, on both Windows and
Mac.
Add a mechanism by which code called after LL_ERRS() can retrieve the fatal
log message string. (How did the crash logger extract that for Linden crash
logging?)
Add that fatal message to crash reports on Windows. But as BugsplatMac is
engaged only on the run _after_ the crash, we no longer have that message in
memory.
Also add user name and region location to Windows crash reports. On Mac, (a)
we don't have the information from the previous run and (b) BugsplatMac
doesn't provide an API to attach that information to the crash report.
Add Mac logging to indicate the success or failure of sending the crash
report. Add Windows logging to indicate we're about to send.
Introduce new header file llappviewermacosx-for-objc.h to publish for
llappdelegate-objc.mm and other Objective-C++ consumers the free functions in
llappviewermacosx.cpp they consume. These were never before declared in any
header file. Apparently, to date, we've been trusting to luck that
Objective-C++ will infer the correct signature from calls -- and that the
calls are correct with respect to the function definitions. :-P
This gives us a place to introduce a new getLogFilePathname() function to
query LLDir. (We don't simply #include "lldir.h" because of the pervasive use
of BOOL in viewer headers; BOOL means something very different in Objective-C++.)
These are mostly things that were in fact erroneous, but accepted by older
compilers.
This changeset has not yet been built with Visual Studio 2013 or Linux gcc,
even with -std=c++11.
This changeset has not been built *without* -std=c++11. It should be used in
conjunction with a corresponding change to LL_BUILD_DARWIN_BASE_SWITCHES in
viewer-build-variables/variables.
This is a work in progress. We do not assert that this changeset completes the
work needed to turn on -std=c++11, even on the Mac.
It's not really clear to me why the original coder felt it necessary to cast
the two sigaction::sa_sigaction fields to unsigned int in the first place, but
in a 64-bit clang compile, that discards information.
The LLApp API used to consist of init(), mainLoop(), cleanup() methods. This
makes sense -- but on Mac that structure was being subverted. The method
called mainLoop() was in fact being called once per frame. There was
initialization code in the method, which (on Mac) needed to be skipped with an
already-initialized bool. There was a 'while' loop which (on Mac) needed to be
turned into an 'if' instead so the method would return after every frame.
Rename LLApp::mainLoop() to frame(). Propagate through subclasses LLAppViewer
and LLCrashLogger. Document the fact that frame() returns true to mean "done."
(This was always the case, but had to be inferred from the code.)
Rename the Mac Objective-C function mainLoop to oneFrame. Rename the C++ free
function it calls from runMainLoop() to pumpMainLoop(). Add comments to
llappdelegate-objc.mm explaining (inferred) control flow.
Change the Linux viewer main() and the Windows viewer WINMAIN() from a single
LLAppViewer::mainLoop() call to repeatedly call frame() until it returns true.
Move initialization code from the top of LLAppViewer::frame() to the init()
method, where it more properly belongs. Remove corresponding
mMainLoopInitialized flag (and all references) from LLAppViewer.
Remove 'while (! LLApp::isExiting())' (or on Mac, 'if (! LLApp::isExiting())')
from LLAppViewer::frame() -- thus unindenting the whole body of the 'while'
and causing many lines of apparent change. (Apologies to reviewers.)
There are four LLApp states: APP_STATUS_RUNNING, APP_STATUS_QUITTING,
APP_STATUS_STOPPED and APP_STATUS_ERROR. Change LLAppViewer::frame() return
value from (isExiting()) (QUITTING or ERROR) to (! isRunning()). I do not know
under what circumstances the state might transition to STOPPED during a
frame() call, but I'm quite sure that if it does, we don't want to call
frame() again. We only want a subsequent call if the state is RUNNING.
Also rename mainLoop() method in LLCrashLogger subclasses
LLCrashLoggerWindows, LLCrashLoggerMac, LLCrashLoggerLinux. Of course it's
completely up to the frame() method whether to yield control; none of those in
fact do. Honor protocol by returning true (frame() is done), even though each
one's main() caller ignores the return value.
In fact LLCrashLoggerWindows::mainLoop() wasn't using the return protocol
correctly anyway, returning wParam or 0 or 1 -- possibly because the return
protocol was never explicitly documented. It should always return true: "I'm
done, don't call me again."
Jonathan "Geenz" Goodman