541446 - sort out details of where OOP minidumps should wind up

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[(not currently active) Ted Mielczarek]

bug 539843 adds an API to submit crash reports. It takes an "id" parameter, which it expects to be the filename (sans extension) of a minidump in the "Crash Reports/pending" directory. It's also, conveniently, the exact data I provide on the PluginCrashed event in my patch in bug 541076. However, currently the OOP crash reporting paths don't move the minidumps from $profile/minidumps to Crash Reports/pending, which means the crash reporting UI has to do it. (The temporary UI in oopcrashdialog.js does this right now.)

We can't simply have the C++ move them to pending always, because that will break the test harnesses' ability to notice crashes and display a stack. The test harnesses currently set MOZ_CRASHREPORTER_NO_REPORT=1 to suppress launching of the crash reporter, which is why the minidumps stay in $profile/minidumps. We can honor that variable in the OOP case, but that will make writing a browser-chrome test for plugin crash reporting difficult, since the minidump won't wind up in pending as expected.

I don't really know what the right solution is here. Perhaps honoring the env var, but also providing a way for individual tests to override that setting?

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: move dumps to pending in minidump callback (in C++), honor MOZ_CRASHREPORTER_NO_REPORT

Ok, this does basically what I described. I included a test that verifies that unsetting MOZ_CRASHREPORTER_NO_REPORT in a mochitest will let a minidump produced in that test be moved to pending, so we can use that to write a browser-chrome test for dolske's UI.

Comment 2

[(not currently active) Ted Mielczarek]

If the diff looks wonky in bugzilla's diff viewer, it's because I "hg cp"ed test_crash_notify.xul to a new file.

Also, I did some gratuitous tab-removal in the Makefile.in.

Comment 3

[(not currently active) Ted Mielczarek]

The test here depends on bug 541076.

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Patch v.2 [Checkin: Comment 5 & 12]

Updated to apply cleanly.

Comment 5

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/748f275c6f1f

Comment 6

[Benjamin Smedberg]

Attachment: Leak quietly, rev. 1

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 426271 [details] [diff] [review]
Leak quietly, rev. 1

We're leaking it either way, so whatever.

Comment 8

[Justin Dolske [:Dolske]]

In no way does this bug depend on bug 203764.

Comment 9

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/45d6586d3505

Comment 10

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/a7a1e08a510b

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed

Comment 12

[Benjamin Smedberg]

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430