Implement multi-process crash reporting on Mac

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: marcia, Assigned: ted)

Tracking

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta5+)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 438893 [details]
Screenshot of Crash Plugin UI

Seen while running  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100413 Minefield/3.7a5pre. I could not find a dupe of this bug. 

File: Flash Player.plugin
Version: 10.1.53.7
Shockwave Flash 10.1 r53

STR:
1. Load URL.
2. Crash in OOPP with attached screenshot

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsICrashReporter.submitReports]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: chrome://browser/content/browser.js :: anonymous :: line 8449"  data: no]
Asa filed Bug 554451, but I guess I was wondering why there was no crash report available and less about the nature of the message.
The same video does not crash using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.3pre) Gecko/20100405 Firefox/3.6.3plugin1.

Comment 3

9 years ago
I can't find the bug, but plugin-process crash reporting is not implemented on mac yet. cjones, is there a bug for that, or shall we use this one?
The exception, specifically, is from reading the nsICrashReporter.submitReports property, which is unimplemented on OS X (bug 542379). But the effect of the exception is harmless at the moment (it's stale code from the 1st pass of UI where we submitted reports automatically, but will hopefully become unstale in the future if we add an optional autosubmit ability).
Nominating so this gets on the radar. Not sure if we are targeting getting data for the beta.
blocking2.0: --- → ?

Updated

9 years ago
blocking2.0: ? → beta1+
Component: Plug-ins → Breakpad Integration
Product: Core → Toolkit
QA Contact: plugins → breakpad.integration

Updated

9 years ago
Summary: OOPP: You Tube video crashes with "No Report Available" → Implement multi-process crash reporting on Mac
(Reporter)

Updated

9 years ago
Blocks: 567265

Updated

9 years ago
Assignee: nobody → joshmoz
Marcia, don't think we need this for the first beta, but I do think we want to get it for beta2!
blocking2.0: beta1+ → beta2+
(In reply to comment #7)
> Marcia, don't think we need this for the first beta, but I do think we want to
> get it for beta2!

well i think from a QA Services Point i think we would like the Data for Mac as soon as possible to look into possible issues.

Comment 9

8 years ago
what is the value of turning on OOPP for Mac if we don't have any data to analyze and understand if OOPP is improving stability?   we should consider turning on Mac OOPP at the same time that we can get data out of crash-stats.

Updated

8 years ago
Assignee: joshmoz → nobody
Since this bug is currently unowned and targeted for B2, shouldn't we get it assigned to someone?
Probably because Benjamin isn't cc'd: Benjamin/Ted/Josh, can we get this resourced and assigned?
blocking2.0: beta2+ → beta3+
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Attachment #438893 - Attachment is obsolete: true
Keywords: crash
Hey Ted, how's this looking for b3?
This is not going to make b3, it only got assigned to me a few days ago and I've only just barely started.
blocking2.0: beta3+ → beta4+
This punting forward game is NOT sustainable.  This needs to get fixed for beta5, preferably early in the beta cycle.
blocking2.0: beta4+ → beta5+
This is almost done, I'll probably be able to land it in the next couple of days.
First portion is up for upstream review:
http://breakpad.appspot.com/146001/show
Second portion up:
http://breakpad.appspot.com/148001/show

Just need to hack up the Gecko glue to make it work.
Ok, this is still going to require a little more work, and unfortunately I'm not available this weekend to finish it up. It's probably only another couple of hours of work, but I probably won't be able to finish it by Monday night.
Current work is available in my mq repository:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/mq/

The first three patches are:
1) Update Breakpad
2) Makefile changes to match Breakpad update
3) Gecko changes to use OOP crashreporting

It's close, but still needs a little work.
Okay, I've got the child process exception handling working. I don't have the hang detector dumping hooked up yet, but after talking with bsmedberg I think we'll just leave that disabled on mac for the time being so we can get child process crash reporting working in for beta 4. I'll attach patches in a second. The "sync to Breakpad" patch syncs us up to r647 + two patches that haven't landed yet:
http://breakpad.appspot.com/151001/show (has review, just didn't land it yet)
http://breakpad.appspot.com/156001/show (not yet reviewed, but a very simple patch)
Created attachment 466308 [details] [diff] [review]
Sync to Breakpad r647, Moz build system changes
Attachment #466308 - Flags: review?(benjamin)
Created attachment 466309 [details] [diff] [review]
Hook up OOP crash reporting for plugin processes on OS X
Attachment #466309 - Flags: review?(benjamin)

Updated

8 years ago
Attachment #466308 - Flags: review?(benjamin) → review+

Updated

8 years ago
Attachment #466309 - Flags: review?(benjamin) → review+
I ran the modules/plugin chrome mochitests with these patches and --setpref=dom.ipc.plugins.enabled=true and all the tests passed, so that's comforting. I've also pushed to try as a sanity check.
(In reply to comment #21)
> The "sync to Breakpad" patch syncs us up to r647 + two patches that haven't
> landed yet:
> http://breakpad.appspot.com/151001/show (has review, just didn't land it yet)
> http://breakpad.appspot.com/156001/show (not yet reviewed, but a very simple
> patch)

I just landed these upstream:
http://code.google.com/p/google-breakpad/source/detail?r=650
http://code.google.com/p/google-breakpad/source/detail?r=651
Blocks: 587747
Just a note for QA:
This is going to be kind of a pain to verify. Right now we only run Flash 10.1 and the new Java plugin OOP on OS X:
http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#915

This means that you can't test with the "Test Plugin" without flipping the dom.ipc.plugins.enabled pref to true. In addition, you can't test by using "kill" to kill the plugin-container process like on Linux, since that doesn't trigger Breakpad on Mac.
(In reply to comment #27)
> Just a note for QA:
Thanks for the work, Ted. I will work on trying to verify this bug.

> This is going to be kind of a pain to verify. Right now we only run Flash 10.1
> and the new Java plugin OOP on OS X:
> http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#915
> 
> This means that you can't test with the "Test Plugin" without flipping the
> dom.ipc.plugins.enabled pref to true. In addition, you can't test by using
> "kill" to kill the plugin-container process like on Linux, since that doesn't
> trigger Breakpad on Mac.
I can verify that this is working using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4) Gecko/20100817 Firefox/4.0b4 as I have already been able to crash in OOPP and received this report: http://crash-stats.mozilla.com/report/index/bp-30f0aed3-b49a-45c2-bb9a-4a2492100817.

If I understand Comment 21 correctly for now I will only get one report and not the hang report as well?
Correct, right now hang reporting is not hooked up at all on Mac. I've separated that out to bug 587747. Thanks for verifying!
Comment on attachment 466308 [details] [diff] [review]
Sync to Breakpad r647, Moz build system changes

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

::: toolkit/crashreporter/google-breakpad/src/common/mac/Makefile.in
@@ +77,5 @@
>  FORCE_STATIC_LIB = 1
>  
>  include $(topsrcdir)/config/rules.mk
> +
> +COMPILE_CMFLAGS += -std=c99

You wouldn't happen to remember why you needed that C standard flag when building an ObjC file, would you?
Flags: needinfo?(ted)
Not 6 years later, no, but looking at the diff that patch also added MachIPC.mm, so I presume it was something in that file:
http://hg.mozilla.org/mozilla-central/rev/e93e3a3d3a6c#l3.12
Flags: needinfo?(ted)
Although that's a .mm file, and we don't use CMFLAGS for that (we use CMMFLAGS), so I'm not entirely sure.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #33)
> Although that's a .mm file, and we don't use CMFLAGS for that (we use
> CMMFLAGS), so I'm not entirely sure.

Heh, I didn't even spot that, that's a good one. What made me raise an eyebrow is the -std flag for C99 for an ObjC source...
You need to log in before you can comment on or make changes to this bug.