581335 - Hook up crash reporting for content processes

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[(not currently active) Ted Mielczarek]

We'll need to do a little bit of work to hook up crash reporting for content processes. We should be able to reuse all of the Breakpad work that was done for OOPP, we just need the glue code in place to make content processes use it all. (This might involve some refactoring to share the code between the two child process types.)

Comment 1

[Doug Turner (:dougt)]

we really want this for Fennec.

Comment 2

[Benjamin Smedberg]

Attachment: Patch, WIP

mfinkle, this should be enough for you or somebody to hook up the UI bits. I'm still trying to get the test to pass because in a Firefox tree the private-browsing service is causing a crash in the content process.

Comment 3

[Benjamin Smedberg]

Attachment: mobile-browser changes, WIP

Comment 4

[Benjamin Smedberg]

Attachment: mozilla-central changes, rev. 1

I moved the networking stuff to bug 614229. This passes tryserver with that.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 492636 [details] [diff] [review]
mozilla-central changes,  rev. 1

Tests look good.

I assume the dumpID sent via the observer notification is the one we'll need to pass the CrashSubmit.submit, so that looks nice and easy.

I don't really know how the PrivateNoteIntentionalCrash stuff will work, but I assume you do.  You should get cjones or ted to take a look at the other ContentParent changes.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: mobile-browser rev 1

Adds most of what's needed by front-end. Need to test.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 492765 [details] [diff] [review]
mobile-browser rev 1

I was able to submit this crash report: 
http://crash-stats.mozilla.com/report/index/bp-f907b664-bd7f-4132-9764-9c59a2101123

after I changed:
>+      CrashSubmit.submit(dumpID, Elements.stack, null, null);
to:
this.CrashSubmit.submit(dumpID, Elements.stack, null, null);

there was an error on the console "strings is null" from CrashSubmit.jsm:180

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: mobile-browser rev 2

This patch has Brad's fix for "this." and hides the "[ ] Submit crash report" UI if we have no crash report.

Brad was able to submit a report from Android. I have not been able to get a report on Linux Desktop.

The "strings is null" error is because we don't ship crashreporter.ini ion Android. We should file a followup bug for that.

Comment 9

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/9a7aaa92e4b9

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 493029 [details] [diff] [review]
mobile-browser rev 2

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js
>-    this._waitingToClose = true;
>-    let reload = Services.prompt.confirmEx(window, title, message, buttons, closeText, reloadText, null, submitText, { value: true });
>+    // Only ahow the submit UI if we have a crash report we can submit
>+    if (!aSubject.hasKey("dumpID"))
>+      submitText = null;

Nit: submit UI is confusing to me, could you do a comment more understandable (from a french point of view)
Nit2: ahow == show

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/mobile-browser/rev/051c9709248b

This bug should be finished now