Closed Bug 1012912 Opened 6 years ago Closed 6 years ago
mac: killing plugin-container with SIGABRT doesn't trigger the crash reporter
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?
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.
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.
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.
Iteration: --- → 34.1
Points: --- → 1
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+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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?
Attachment #8463950 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I put this up for upstream review: https://breakpad.appspot.com/7744002
This is going to wait for upstream landing until Chrome switches to their Breakpad replacement, per: https://breakpad.appspot.com/7744002#msg2
You need to log in before you can comment on or make changes to this bug.