994708 - Record in FHR the submission rate for plugin/content crashes

Status: VERIFIED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, enhancement)

Description

[Benjamin Smedberg]

Once bug 983313 is finished, we will be recording plugin and content crashes/hangs in FHR. As a followup step we should also record the submission rate for those crashes.

This bug tracks only the subprocess crashes because they are submitted using a different mechanism than main-process crashes and hangs. Main-process crash submission rate is tracked in bug

Comment 1

[Benjamin Smedberg]

"in bug 994707"

Comment 2

[Biru [:poiru]]

Attachment: Part 1: Cleanup CrashSubmit.jsm

After reading through the code, it seems like CrashSubmit.jsm would be the place to implement this. This patch does some minor cleanups in preparation of future patches.

I will wait for the design of the crash submission events to be finalized in bug 994707 before attaching further patches here. However, here is my current thinking of how this will be stitched together:
- CrashSumbit.jsm will have methods to create submission events. When the submission succeeds or fails within CrashSumbit.jsm, we write an event.
- If the user never attempts to submit the report in the first place, browser-plugin.js will call the relevant method in CrashSubmit to create a failed submission.

Does this sound about right?

Comment 3

[Benjamin Smedberg]

Yes, that sounds right.

Comment 4

[Benjamin Smedberg]

Actually though, the last part isn't right. If the user never attempts to submit, we don't want to record anything. Failed submission is only for the case where the user chose to submit but it failed because of the network or something.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8443515 [details] [diff] [review]
Part 1: Cleanup CrashSubmit.jsm

Review of attachment 8443515 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good. I can't wait for CrashSubmit.jsm to be rewritten in terms of CrashManager.jsm APIs.

Comment 6

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56246d479b89

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/56246d479b89

Comment 8

[Biru [:poiru]]

Attachment: Part 2: Record submission event for plugin crashes

This is based on top of the latest patches in bug 994707.

I made processType optional for CrashManager.submit as I was unsure if all uses of CrashManager.submit[0] should record a submission event. Should we handle of them?

[0]: http://mxr.mozilla.org/mozilla-central/search?string=CrashSubmit.submit

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8445582 [details] [diff] [review]
Part 2: Record submission event for plugin crashes

Review of attachment 8445582 [details] [diff] [review]:
-----------------------------------------------------------------

I'm guessing we probably want to record submissions for everything we can. bsmedberg has a more clear version of the requirements/goals than I do.

::: toolkit/crashreporter/CrashSubmit.jsm
@@ +292,5 @@
> +          let crashTime = new Date();
> +          let manager = Services.crashmanager;
> +          manager.addSubmission(self.processType, manager.CRASH_TYPE_CRASH,
> +                                success, success ? ret.CrashID : "",
> +                                crashTime);

Where's the code for addSubmission? I don't see it in the tree or this bug.

Do we care about recording submission attempts as opposed to successes? It seems weird that we record a failed submission and a completed submission the same way and furthermore the distinguishing manner is by presence of a crash ID.

Comment 10

[Biru [:poiru]]

(In reply to Gregory Szorc [:gps] from comment #9)
> Comment on attachment 8445582 [details] [diff] [review]
> Part 2: Record submission event for plugin crashes
> 
> Review of attachment 8445582 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm guessing we probably want to record submissions for everything we can.
> bsmedberg has a more clear version of the requirements/goals than I do.
> 
> ::: toolkit/crashreporter/CrashSubmit.jsm
> @@ +292,5 @@
> > +          let crashTime = new Date();
> > +          let manager = Services.crashmanager;
> > +          manager.addSubmission(self.processType, manager.CRASH_TYPE_CRASH,
> > +                                success, success ? ret.CrashID : "",
> > +                                crashTime);
> 
> Where's the code for addSubmission? I don't see it in the tree or this bug.

See part 2 of bug 994707.

> Do we care about recording submission attempts as opposed to successes? It
> seems weird that we record a failed submission and a completed submission
> the same way and furthermore the distinguishing manner is by presence of a
> crash ID.

The implementation of addSubmission in the bug above should clarify this.

Comment 11

[Biru [:poiru]]

Attachment: Part 2: Record submission event for plugin crashes

This patch should be good to go. This iteration of the patch only contains cosmetic differences.

I will take care of updating other uses of CrashSubmit.submit in subsequent patches.

Comment 12

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45f3f1fe7b39

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/45f3f1fe7b39

Comment 14

[Biru [:poiru]]

Attachment: Part 3: Record submission event for content crashes

Comment 15

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/531825b43409

Try push: https://tbpl.mozilla.org/?tree=Try&rev=d5795053e886

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/531825b43409

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Assigning to Kamil since he handled verification for bug 994707.

Comment 18

[Kamil Jozwiak [:kjozwiak]]

Is there a tool that can force plugin/content crashes in fx?? The tools below only trigger application crashes, example:

- "main-crash": 6,
- "main-crash-submission-succeeded": 5

- https://developer.mozilla.org/en-US/docs/How_to_Report_a_Hung_Firefox
- http://ted.mielczarek.org/mozilla/crashme.html

I believe there should be entries for plugins/content rather than all the crashes being listed under "main-crash" correct?

Comment 19

[Benjamin Smedberg]

On a modern Firefox (33+) you should be able to use those tools for any process: SIGABRT the plugin/content processes on mac/linux, and use crashfirefox.exe with the PID of a plugin-container on windows.

Comment 20

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following m-c build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-09-02-03-02-02-mozilla-central/

** Plugin Test Cases: **

Prerequisites:
- downloaded the latest flash for each OS (Win, OSX, Linux)
- used crashfirefox.exe on Win to crash plugin_container.exe
- used the SIGABRT method on Linux to crash plugin-container
- used the SIGABRT method on OSX to crash Nightly Plugin Content (Shockwave Flash)
- Updated FHR using: Cu.import("resource://gre/modules/CrashManager.jsm"); CrashManager.Singleton.runMaintenanceTasks();

Test Cases Used:

- ensure that "plugin-crash" is being incremented correctly
- ensured that "plugin-crash-submission-succeeded" incremented once the user selects "Submit a crash report"
- ensured that "plugin-crash-submission-succeeded" is not incremented if the user selects "Reload page"
- ensured that "plugin-crash-submission-succeeded" is not incremented if the user selects "X"
- ensured that "plugin-crash-submission-succeeded" is not incremented if the user selects "Learn more..."
- set "MOZ_CRASHREPORTER_URL=http://localhost:9999/invalid" and ensured "plugin-crash-submission-failed" was incremented when appropriate
- ensured that "plugin-crash-submission-succeeded" is not being incremented when "plugin-crash-submission-failed" is incremented
- ensured that the crashes are appearing under about:crashes

** Content Test Cases: **

Prerequisites:
- used crashfirefox.exe on Win to crash plugin_container.exe (e10s)
- used the SIGABRT method on Linux to Web Content (e10s)
- used the SIGABRT method on OSX to crash NightlyWeb Content (e10s)
- Updated FHR using: Cu.import("resource://gre/modules/CrashManager.jsm"); CrashManager.Singleton.runMaintenanceTasks();

Test Cases Used:

- ensure that "content-crash" is being incremented correctly
- ensured that "content-crash-submission-succeeded" incremented once the user selects "Submit a crash report"
- ensured that "content-crash-submission-succeeded" is not incremented if the user selects "Reload page"
- ensured that "content-crash-submission-succeeded" is not incremented if the user selects "X"
- ensured that "content-crash-submission-succeeded" is not incremented if the user selects "Learn more..."
- set "MOZ_CRASHREPORTER_URL=http://localhost:9999/invalid" and ensured "content-crash-submission-failed" was incremented when appropriate
- ensured that "content-crash-submission-succeeded" is not being incremented when "content-crash-submission-failed" is incremented
- ensured that selecting "Try Again" while "Tell Mozilla about this crash so they can fix it." is un-selected wouldn't increment "content-crash-submission-failed" and "content-crash-submission-succeeded" 
- ensured that the crashes are appearing under about:crashes

Checked on the following OS's:
- Win 8.1 x64 (VM)
- Ubuntu 13.10 x64 (VM)
- OSX 10.9.4

Possible issues found:

Issue #1:

- plugins & content crashes are not creating submission files under "/crashes/events/" when crashes occur. However when an application crash occurs, submission files are created and than cleared (Bug # 994707). I just want to make sure that plugin and content crashes shouldn't be mirroring application crashes. If both plugin and content crashes should be creating files under "/crashes/events/" the same way application crashes do, then there's definitely a bug.

Issue #2:

- plugin & content "Crash report submitted successfully" timestamps are not being added into "submit.txt" under "/Mozilla/Firefox/Crash Reports"
- plugin & content "Crash report submission failed: The operation completed successfully." timestamps are not being added into "submit.txt" under "/Mozilla/Firefox/Crash Reports"
- despite not being added into "submit.txt", they're appearing under the "submitted" folder in "/Mozilla/Firefox/Crash Reports/submitted"

The issue is that submission timestamps are not being added into "submit.txt" for plugins/content crashes but the submissions are correctly appearing under "/Mozilla/Firefox/Crash Reports/submitted/"

Issues #3:

- "LastCrash" is only being updated when there's an application crash. It's not being updated when there's a plugin/content crash.

Issue #4:

- If "Tell Mozilla about this crash so they can fix it." is checked (enabled) when a e10s window/tab crashes, it won't be counted unless the user selected "Try Again", example:

A e10s crashes and the user has "Tell Mozilla about this crash so they can fix it." checked, but closes the window/tab and doesn't select "Try Again", the crash won't be counted. It only counts the crashes when the user selects "Try Again". I just want to double check and make sure this is the desired behaviour.

Should those crashes be counted when a user has "Tell Mozilla about this crash so they can fix it." checked and simply closes the tab/window/fx?

Issue #5:

- When you crash a "flash" plugin that's running under a e10s on Win, you won't receive a toolbar stating that there was a crash with flash. Neither "content" nor "plugin" are counted so there's no data to show that there was a crash. (from what I can see)
- On Linux/OSX, crashing the "flash" plugin that's running under a e10s will crash the e10s and count it as a "content" crash

Benjamin, mind taking a quick look at the possible issues?

Comment 21

[Benjamin Smedberg]

> Issue #1:
> 
> - plugins & content crashes are not creating submission files under
> "/crashes/events/" when crashes occur. However when an application crash

This is correct behavior. Submission files are only used when Firefox is not running.

> Issue #2:
> 
> - plugin & content "Crash report submitted successfully" timestamps are not
> being added into "submit.txt" under "/Mozilla/Firefox/Crash Reports"

Also correct.

> Issues #3:
> 
> - "LastCrash" is only being updated when there's an application crash. It's
> not being updated when there's a plugin/content crash.

Also correct.

> Issue #4:

> A e10s crashes and the user has "Tell Mozilla about this crash so they can
> fix it." checked, but closes the window/tab and doesn't select "Try Again",
> the crash won't be counted. It only counts the crashes when the user selects
> "Try Again". I just want to double check and make sure this is the desired
> behaviour.

Please file a bug on this. It's not a problem in this patch, but a UI issue in e10s that we should probably address.

> Issue #5:
> 
> - When you crash a "flash" plugin that's running under a e10s on Win, you
> won't receive a toolbar stating that there was a crash with flash. Neither
> "content" nor "plugin" are counted so there's no data to show that there was
> a crash. (from what I can see)
> - On Linux/OSX, crashing the "flash" plugin that's running under a e10s will
> crash the e10s and count it as a "content" crash

Not issues with this patch, and should be fixed by bug 874016.

Comment 22

[Kamil Jozwiak [:kjozwiak]]

> Please file a bug on this. It's not a problem in this patch, but a UI issue
> in e10s that we should probably address.

Created Bug # 1062348

> Not issues with this patch, and should be fixed by bug 874016.

Checked Bug # 874016 and found several bugs that address the issues I've seen.

Thanks for going through the possible issues Benjamin, marking this as verified.