Closed Bug 842415 Opened 7 years ago Closed 7 years ago

PR_Locks should be replaced with MutexAutoLocks in Signaling Code

Categories

(Core :: WebRTC: Signaling, defect, P3, minor)

defect

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: ehugg, Assigned: ehugg)

Details

(Whiteboard: [WebRTC], [blocking-webrtc-] [qa-])

Attachments

(1 file)

The Signaling code still has a couple places where we call PR_Lock directly instead of using a MutexAutoLock.  In particular PeerConnectionMedia::mLocalSourceStreamsLock
Assignee: nobody → ethanhugg
Severity: normal → minor
Priority: -- → P3
Whiteboard: [WebRTC], [blocking-webrtc-]
Comment on attachment 715542 [details] [diff] [review]
Signaling code - replace PR_Lock calls with MutexAutoLock

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

::: media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -349,4 @@
>    nsTArray<nsRefPtr<LocalSourceStreamInfo> > mLocalSourceStreams;
>  
>    // A list of streams provided by the other side
> -  PRLock *mRemoteSourceStreamsLock;

Found this unused data member.  Going to assume this is unneeded since we don't access this list in add/removestream.
Attachment #715542 - Flags: review?(rjesup)
Comment on attachment 715542 [details] [diff] [review]
Signaling code - replace PR_Lock calls with MutexAutoLock

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

::: media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ +237,5 @@
>  class PeerConnectionMedia : public sigslot::has_slots<> {
>   public:
>    PeerConnectionMedia(PeerConnectionImpl *parent)
>        : mParent(parent),
> +      mLocalSourceStreamsLock("LocalSourceStreams"),

"PeerConnectionMedia.mLocalSourceStreamsLock"
Not required, but seems to be semi-standard (chfind 'Lock("')
Attachment #715542 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/dc4ad3318146
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-] [qa-]
You need to log in before you can comment on or make changes to this bug.