1012912 - mac: killing plugin-container with SIGABRT doesn't trigger the crash reporter

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Benjamin Smedberg]

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?

Comment 1

[(not currently active) Ted Mielczarek]

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.

Comment 2

[Georg Fritzsche [:gfritzsche]]

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.

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix

Comment 5

[(not currently active) Ted Mielczarek]

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?

Comment 6

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbeb113185da

Comment 7

[Georg Fritzsche [:gfritzsche]]

Attachment: Upstream patch

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bbeb113185da

Comment 9

[Georg Fritzsche [:gfritzsche]]

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.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/35c982e10670

Comment 11

[(not currently active) Ted Mielczarek]

I put this up for upstream review:
https://breakpad.appspot.com/7744002

Comment 12

[(not currently active) Ted Mielczarek]

This is going to wait for upstream landing until Chrome switches to their Breakpad replacement, per:
https://breakpad.appspot.com/7744002#msg2