Closed
Bug 1012912
Opened 10 years ago
Closed 10 years ago
mac: killing plugin-container with SIGABRT doesn't trigger the crash reporter
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
People
(Reporter: benjamin, Assigned: gfritzsche)
Details
(Whiteboard: breakpad-upstream)
Attachments
(2 files)
(deleted),
patch
|
ted
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Killing plugin container with SIGABRT (either internally using abort() or externally by sending it that signal) should trigger our crash reporter. This was fixed for the main Firefox process in bug 717758 but it doesn't appear to work for Flash or Silverlight.
Ted it appears that there is an explicit IsOutOfProcess check which makes this not work: http://hg.mozilla.org/mozilla-central/annotate/41a54c8add09/toolkit/crashreporter/google-breakpad/src/client/mac/handler/exception_handler.cc#l629
Do you know why that's there or if I can safely remove it?
Flags: needinfo?(ted)
Comment 1•10 years ago
|
||
This dates back to the original implementation for iOS:
https://code.google.com/p/google-breakpad/source/detail?r=933&path=/trunk/src/client/mac/handler/exception_handler.cc
I don't know why it's there, you could try removing it and see what happens.
Flags: needinfo?(ted)
Assignee | ||
Comment 2•10 years ago
|
||
Removing it works fine on OS X. Is there any argument against removing the check now, either completely or specifically for OS X?
If we could do that now it would make QA for GMP crashreporting less complicated until we have bug 1044408.
Flags: needinfo?(ted)
Comment 3•10 years ago
|
||
I don't know any reason why it is like it is, so if changing that works then get a patch up. I'll get it upstreamed as well.
Flags: needinfo?(ted)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8463950 -
Flags: review?(ted)
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Iteration: --- → 34.1
Points: --- → 1
Flags: firefox-backlog+
Updated•10 years ago
|
Assignee: nobody → georg.fritzsche
QA Whiteboard: [qa?]
Assignee | ||
Updated•10 years ago
|
QA Whiteboard: [qa?] → [qa-]
Comment 5•10 years ago
|
||
Comment on attachment 8463950 [details] [diff] [review]
Fix
Review of attachment 8463950 [details] [diff] [review]:
-----------------------------------------------------------------
Can you provide a patch against upstream SVN that I can submit there?
Attachment #8463950 -
Flags: review?(ted) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8463950 [details] [diff] [review]
Fix
Approval Request Comment
[Feature/regressing bug #]: GMP crash testing.
[User impact if declined]: QA & dev complicated a lot for GMP crash testing, this allows us to just kill the plugin-container for that.
[Describe test coverage new/current, TBPL]: Fine with local testing, up on m-c now.
[Risks and why]: Low-risk as this just removes a check that prevent the signal handler from being installed on OS X.
[String/UUID change made/needed]: None.
Attachment #8463950 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox33:
--- → affected
status-firefox34:
--- → fixed
Updated•10 years ago
|
Attachment #8463950 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•10 years ago
|
||
Comment 11•10 years ago
|
||
I put this up for upstream review:
https://breakpad.appspot.com/7744002
Comment 12•10 years ago
|
||
This is going to wait for upstream landing until Chrome switches to their Breakpad replacement, per:
https://breakpad.appspot.com/7744002#msg2
Whiteboard: breakpad-upstream
You need to log in
before you can comment on or make changes to this bug.
Description
•