1041226 - GMPChild should finish IPC setup before calling Crashreporter IPC method

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

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

Attachment: stack

+++ This bug was initially created as a clone of Bug #1039575 +++

Bug 1039575 broke --enable-crashreporter builds (Nightly) due to an assert when registering the crashreporter ID during GMP initialization:

[time:1405803882621308][19039->-1][PGMPChild] Sending Msg_PCrashReporterConstructor([TODO])
[19039] ###!!! ABORT: not on worker thread!: 'mWorkerLoopID == MessageLoop::current()->id()', file ../../dist/include/mozilla/ipc/MessageChannel.h, line 362

I believe the assert is erroneous here, and is caused by incomplete setup before it tries to send the ID.

Comment 1

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

Attachment: disable crashreporter in GMP plugins until it's ready

Comment 2

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

Comment on attachment 8459235 [details] [diff] [review]
disable crashreporter in GMP plugins until it's ready

Tested on m-c with --enable-crashreporter

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8459235 [details] [diff] [review]
disable crashreporter in GMP plugins until it's ready

Sign-off for quick unbustage for jesup, deeper look tomorrow or monday.

Comment 4

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

Landed on m-c and inbound because it was a hidden requirement of the stuff we landed on m-c last night.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9383ccd71289
https://hg.mozilla.org/mozilla-central/rev/1e7f2af8d929

Comment 5

[Maire Reavy [:mreavy]]

Georg -- I believe you said you'd take this since bsmedberg is on PTO

Comment 6

[Georg Fritzsche [:gfritzsche]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5)
> Georg -- I believe you said you'd take this since bsmedberg is on PTO

Correct.

Comment 7

[Georg Fritzsche [:gfritzsche]]

I'm torn for the points without spending time investigating first, so i'll just lean towards over-estimating.

Comment 8

[Marco Mucci [:MarcoM]]

Hi Georg, can you mark this bug as [qa+] or [qa-] for verification.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9383ccd71289

Comment 10

[Georg Fritzsche [:gfritzsche]]

(In reply to Marco Mucci [:MarcoM] from comment #8)
> Hi Georg, can you mark this bug as [qa+] or [qa-] for verification.

qa- as this path will be covered by the other OpenH264/... testing once we can re-enable the crash reporting.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Does this problem only happen with debug builds?

Comment 12

[Steven Michaud [:smichaud] (Retired)]

For what it's worth, this bug doesn't happen on the Mac when I start the sandbox (call sandbox_init()) in GMPChild::OnChannelConnected() -- after IPC is fully set up.  I tested with a debug build.

We may want to consider doing this on all platforms.

Comment 13

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

I believe the proximate cause is an assertion; however I'm not sure if it will work or not if debug assertions are off.

Holding off on stuff like this until IPC is fully up makes sense....

Comment 15

[Georg Fritzsche [:gfritzsche]]

Attachment: Open IPC channel before using IPC methods

Comment 16

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Nicolas, i heard you may be able to give input on IPC things?
It looks like our IPC peer availability is not good right now.

Comment 17

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Checking through mozilla::ipc::Open() et al, i'm fairly sure this should be fine.

Comment 18

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

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

Comment 19

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

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

Comment 20

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

Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Forgot to list this with the other openh264 uplift requests to bring it up to inbound state.

Approval Request Comment
[Feature/regressing bug #]: bsmedberg GMP crashreporter landing

[User impact if declined]: Crashreporter doesn't get inited for GMP plugins

[Describe test coverage new/current, TBPL]: WIll be covered in the general GMP crashreporting coverage under development

[Risks and why]: Very low risk; on central for some time

[String/UUID change made/needed]: none

Comment 21

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/f3cf6e9ddff5