1097740 - use NS_INLINE_DECL_REFCOUNTING for Fake_MediaStreamTrack in FakeMediaStreams.h

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Nathan Froyd [:froydnj]]

There are a select few places where RefCounted should be used.  This is not one
of them.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: use NS_INLINE_DECL_REFCOUNTING for Fake_MediaStreamTrack in FakeMediaStreams.h

Note that we already use variants of NS_INLINE_DECL*REFCOUNTING elsewhere in
this file.  This change makes us more consistent in addition to other benefits.

Comment 2

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

I backed this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/8dbb9eef253b for cppunit test orange:
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3827385&repo=mozilla-inbound

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Wes Kocher (:KWierso) from comment #2)
> I backed this out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8dbb9eef253b for
> cppunit test orange:
> https://treeherder.mozilla.org/ui/logviewer.
> html#?job_id=3827385&repo=mozilla-inbound

Yeah, I have no idea why swapping out the refcounting here would cause those sorts of crashes.  Ideally it'll reproduce on my machine, though...

Comment 4

[Nathan Froyd [:froydnj]]

Ah, the real bug is this:

13:05:27 INFO - Hit MOZ_CRASH(Fake_MediaStreamTrack not thread-safe) at /builds/slave/m-in-lx-d-00000000000000000000/build/media/webrtc/signaling/test/FakeMediaStreams.h:221

The MFBT refcounting bits weren't checking for thread ownership, whereas the XPCOM ones were.  Maybe this cleanup will even fix some intermittent crashes...?

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: use NS_INLINE_DECL_THREADSAFE_REFCOUNTING for Fake_MediaStreamTrack in FakeMediaStreams.h

As explained in comment 4, it looks like we ought to have been using threadsafe
refcounting with this class...

Comment 6

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

Comment on attachment 8522268 [details] [diff] [review]
use NS_INLINE_DECL_THREADSAFE_REFCOUNTING for Fake_MediaStreamTrack in FakeMediaStreams.h

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

Yes, I imagine it does have to be threadsafe (and perhaps the old RefCounted<> wasn't doing thread checks).

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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