Closed Bug 545712 Opened 10 years ago Closed 10 years ago

"A crash report was submitted" even when it wasn't

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a2
Tracking Status
status1.9.2 --- .4-fixed

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

(Whiteboard: [fixed-lorentz])

Attachments

(2 files)

If you have MOZ_CRASHREPORTER_NO_REPORT set in the environment, the plugin-crashed UI will say that a crash report was submitted when it actually wasn't.

I don't know if there are other cases where this might be true: as filed this is pretty inconsequential, but if it shows up in Linux distros which build --enable-crashreporter but disable crash report submission that might not be so good.
I believe distros generally build --disable-crashreporter currently, FWIW.
This also shows up in normal release builds when you kill a mozilla-runtime process.
Attached patch Patch v.1Splinter Review
Like so? Hmm, bah, guess not, I just read and tested comment 2.

Sounds like I'm also missing a check somewhere to see if the child's "crash" was really a crash or not.
the crashreportID coming with the global "plugin-crashed" notification is empty, but the Firefox code is unprepared for that case. Patch in a sec.
Assignee: dolske → benjamin
This patch has two parts. First we detect an empty crashreportID and also check the return value of CrashReport.submit() in order to set the crashreportsubmitted boolean value correctly.

Secondly it suppresses the message in the crashed-plugin UI in the case where the plugin crashed but you don't have crash submission disabled. Without this change, when you kill mozilla-runtime.exe it says 'You have disabled crash report submission.' which is incorrect.
Attachment #427766 - Flags: review?(dolske)
Blocks: 538910
Comment on attachment 427766 [details] [diff] [review]
Deal with empty crashreportID, rev. 1

>-    let submitReports = gCrashReporter.submitReports;
>+    let submitted = gCrashReporter.submitReports && minidumpID.length > 0;

Nit: Omit "> 0"?

>+    if (submittedReport) {
>+      helpClass = "submitLink";
>+      showClass = "msg msgSubmitted";
>+    }
>+    else if (!gCrashReporter.submitReports) {

Nit: Brace and else on same line?

>+    if (helpClass !== undefined) {

Nit: Just "if (helpClass)"?

I think it's worth a brief comment here to note why it is that neither "msgSubmitted" not "msgNotSubmitted" is being used, since that would normally seem like an obvious binary choice.

Or maybe we should just add a 3rd message here for "No crash report was generated". But that doesn't seem all that useful, unless there are other unexpected and likely ways to trigger this path (other than killing a process manually).
Attachment #427766 - Flags: review?(dolske) → review+
Pushed in http://hg.mozilla.org/mozilla-central/rev/8b56adc3241c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
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
You need to log in before you can comment on or make changes to this bug.