546173 - Provide our own module for about:crashes

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: General, defect)

Description

[Christopher Henderson]

Disabling the Mozilla crash reporter in 1.9.[12] builds prevents the module for about:crashes to be built. We already provide our own crashes.xhtml, so going forward it would be best to also provide our own module to load it.

I'd imagine it would look something like Sean's safebrowsing about module, and would load chrome://global/content/crashes.xhtml.

Comment 1

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

After further investigation, I don't think we need to build with --disable-crashreporter by default.

http://mxr.mozilla.org/mozilla1.9.2/search?string=MOZ_CRASHREPORTER&find=&findi=&filter=&hitlimit=&tree=mozilla1.9.2 shows all the places where that ifdef exists.

Since we don't link toolkit/xre (xulapp module), which appears to be what links in the Moz code in toolkit/crashreporter and the Breakpad handler, client, and common code, and we control our own packaging, disabling crashreporter only affects us in docshell/ (this bug), the code that sends exceptions (xpcom/base/nsObjCExceptions.h), and some additional logging in layout/.

(Incidentally, XRE and toolkitcomps probably make libxul a no-go for us.)

It's a bit of a 50-50, really; we save some build time (toolkit/crashreporter) but have to write our own module here.

OTOH, it seems like we could build without disabling crashreporter and build a little stub service that implements part of "@mozilla.org/toolkit/crash-reporter;1" and pick up a way to get the exceptions from Core's code instead of implementing bug 540903.

(Note that we're not disabling crashreporter on 1.9.0, either; it's actually on by default on all supported platforms; we just don't build in toolkit/crashreporter because we don't build in toolkit/ at all except for the bits used by both xpfe and toolkit apps in http://mxr.mozilla.org/seamonkey/source/toolkit/components/Makefile.in#46 and toolkit-tiers.mk, and the things we specifically turned on--url-classifier, some sort of autocomplete header, and the nsIDownload header.)

That said, there's also the 10.6 and 64-bit issues; if the tree copy of Breakpad hasn't been made 64-bit compatible by the time we move to building on 10.6 or for 64-bit, we might have to disable crashreporter.  We may also want to reimplement about:crashes in Cocoa at some point to provide other features, like bug 515861 and localized date formats.

I don't have a real strong preference here; less building is good, but so is getting free stuff from Gecko (if said stuff doesn't come with too many strings attached).

Comment 2

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

Talked to Stuart about this today, and he didn't object to not disabling crashreporter for now.

I'll leave this bug open; in the future we probably will want to do this for one or more of the reasons mentioned in comment 1, but for now not writing our own module right now frees up coding time for fixing stuff that's really broken.

Comment 3

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

This is actually pretty simple to do; as hendy notes in comment 0, we'd take the initial version of murph's SafeBrowsingAboutModule (if we want to keep loading from the jar; if we want to switch to loading from the filesystem where we can do additional Cocoa localization on the file, we can switch to the current version of SBAM), fix up the names and files, and change the load flags (about:crashes needs to run with chrome privileges, so there should not be a nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT flag).  murph's already done the hard work; this is just copying and pasting.

For safety, we should also ifdef component registration on MOZ_CRASHREPORTER being unset, so that we don't try to double-register with docshell's version if someone is building with MOZ_CRASHREPORTER=1.

Comment 4

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

(In reply to Smokey Ardisson (not following bugs - do not email) from comment #1)
> OTOH, it seems like we could build without disabling crashreporter and build
> a little stub service that implements part of
> "@mozilla.org/toolkit/crash-reporter;1" and pick up a way to get the
> exceptions from Core's code instead of implementing bug 540903.

FWIW, this probably would have been pretty easy; I haven't checked completely, but outside of xulapp and tookitcomps, I don't think anyone's calling other than AnnotateCrashReport, AppendObjCExceptionInfoToAppNotes, and maybe AppendAppNotesToCrashReport; the first would be a straight wrapper around BreakpadAddUploadParameter, and the latter two, which might be called periodically to append data, would be a bit fatter wrapper that holds on to the current data, appends the new data, and updates Breakpad with the new value (e.g., a setReportedURL: on steriods).

Wish I'd realized this a year+ ago :(