688569 - Send "LastCrash" (SecondsSinceLastCrash) with crash reports

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: General, enhancement)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Spun off of bug 574094, which was about InstallTime support (but which mentioned LastCrash):

(In reply to bug 574094 comment #1)

> Seems like this is done in 
> http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/crashreporter/
> nsExceptionHandler.cpp#669
> http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/crashreporter/
> nsExceptionHandler.cpp#682 and
> http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/crashreporter/
> nsExceptionHandler.cpp#732
> 
> Or, in human terms, it writes an InstallTime<BUILDID> file containing a Unix
> timestamp on first startup of the app with that buildid, and then at some
> point read back and sent with the crash.  It was originally implemented in
> bug 376720 (and modified later).
> 
> Per bug 488513 comment 8, we also don't have LastCrash date/time support;
> for reference, in Core that was implemented in bug 376721.  That's also been
> surfaced in Socorro since I lasted looked.  (We can spin this off to a
> separate bug if desired when this one gets fixed, but mentioning it in a
> related and open bug is better than it being lost in bug 488513 comment 8 ;)
> )

(In reply to bug 574094 comment #2)

> (I think LastCrash is going to require some changes in Breakpad to support,
> though I didn't dig deep enough to really see; OTOH, Breakpad will have the
> info it needs to write a LastCrash placeholder--buildID and crash time--so
> changes there could well be "trivial", plus another "trivial" file
> read+parameter set here in BreakpadWrapper…)

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Stuart, on the Breakpad side, should writing LastCrash to a file be done in |Inspector::InspectTask()| (assuming Inspector has access to the build ID, which I think it does) or somewhere in crash_report_sender?

It should in theory to be simple to drop the writing code from bug 574094 into wherever in Breakpad it needs to go (in fact, the code should be even simpler since we're always writing for a given build) and then hook up sending a LastCrash parameter from Camino to Breakpad on next launch.

Comment 2

[Stuart Morgan]

Oh hey, I already wrote this code. For a Chromium feature. Check out
http://code.google.com/p/google-breakpad/source/detail?r=748

Once we rev breakpad (still actively working on that, just haven't had much time), we'll have a file of the form:
crashid,epochtime
crashid,epochtime
...
in a known location, so we can just read the second half of the last line of that file in Camino.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

From a first glance, that looks like it's only logging uploaded crashes (so not failures or non-submissions), which isn't exactly what we want.  Maybe it'll be good enough in general; I can't quite visualize that far ahead this morning.

(Also, there's an argument to be made for having LastCrash be a property of a specific build, so you can see on a given crash report whether a user has crashed this build before, or if their previous crash was possibly only relevant to the last version.  Firefox doesn't seem to do that, though, the counter-argument being, I guess, that you want a to be able to tell how crashy your app is overall as people move from version to version, or that in practice it can actually do both?)

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

OTOH, we can certainly start with that, since it will require 0 changes to Breakpad and only new file-reading/param-setting code in BreakpadWrapper, and then decide where to go from there, since some value for LastCrash in 100% of cases is better than no value in 100% of cases, even if the new value is only accurate in 90% of cases.

Comment 5

[Stuart Morgan]

Alternately, we could look for the newest file in the Camino crashes directory, which should get both uploaded and not.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to Stuart Morgan from comment #5)
> Alternately, we could look for the newest file in the Camino crashes
> directory, which should get both uploaded and not.

s/file/.dmp file/, since we're now storing the InstallTime.plist there, too, but that sounds very do-able short-term.

If I need to take a break from website stuff before RC, I'll look at this for 2.1; otherwise, 2.1.1.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Er, I totally mis-remembered how the toolkit exception handler code that backs the "LastCrash" UI field in Socorro worked :(

The parameter name is actually SecondsSinceLastCrash (http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/crashreporter/nsExceptionHandler.cpp#165), which totally means Breakpad surgery, since Breakpad is the only thing that knows the time of the current crash in order to do the math to provide the correct data for the parameter :(  (Hint: if you have a Gecko crash dump in /pending/, you can look at the .extra file to see the parameters they send, sans comments and email and maybe other stuff)

So this probably does need to be implemented 100% inside Breakpad (although we could conceivably have Camino provide Breakpad, via |BreakpadSetKeyValue|, the time of the last crash using the method described in comment 5).

But not definitely not going to be before 2.1 now :(

(In reply to comment #3)
> (Also, there's an argument to be made for having LastCrash be a property of
> a specific build, so you can see on a given crash report whether a user has
> crashed this build before, or if their previous crash was possibly only
> relevant to the last version.  Firefox doesn't seem to do that, though,

But thought they maybe should: bug 376721 comment 8, bug 376721 comment 10

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'd still like to explore doing this.