Created attachment 280093 [details] Proposed patch

Eric, I did a clean mac build with --media-stream --web-rtc but it didn't seem to build the stuff under the WEB_RTC flag. The patch has added the new files to the mac build (but not the generated JS-bindings).

Comment on attachment 280093 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=280093&action=review > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:265 > + RefPtr<RealtimeMediaSource> remoteSource = m_mediaEndpoint->createMutedRemoteSource(transceiverMid, sourceType); > + RefPtr<MediaStreamTrackPrivate> remoteTrackPrivate = MediaStreamTrackPrivate::create(WTFMove(remoteSource), trackId); > + Ref<MediaStreamTrack> remoteTrack = MediaStreamTrack::create(*m_client->scriptExecutionContext(), *remoteTrackPrivate); Types should all be auto. > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:164 > + RefPtr<RTCRtpSender> sender = RTCRtpSender::create(WTFMove(track), Vector<String>(), *this); > + RefPtr<RTCRtpReceiver> receiver = m_backend->createReceiver(transceiverMid, trackKind, trackId); > + Ref<RTCRtpTransceiver> transceiver = RTCRtpTransceiver::create(WTFMove(sender), WTFMove(receiver)); Types should just be auto. > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:187 > + RefPtr<RTCRtpSender> sender = RTCRtpSender::create(kind, Vector<String>(), *this); > + RefPtr<RTCRtpReceiver> receiver = m_backend->createReceiver(transceiverMid, kind, trackId); > + Ref<RTCRtpTransceiver> transceiver = RTCRtpTransceiver::create(WTFMove(sender), WTFMove(receiver)); Types should just be auto. > Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:69 > + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(MediaStreamTrack track, optional Dictionary init); > + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(DOMString kind, optional Dictionary init); New code should never use Dictionary. These dictionaries should defined. dictionary RTCRtpTransceiverInit { boolean send = true; boolean receive = true; sequence<MediaStream> streams; sequence<RTCRtpEncodingParameters> sendEncodings; }; If the bindings generator can’t handle this dictionary, we should fix it so it can handle it. Every single time we instead use the deprecated Dictionary, we add code we will need to later delete! > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:98 > +const String& RTCRtpTransceiver::directionString() const > +{ > + switch (m_direction) { > + case Direction::Sendrecv: return sendrecvString(); > + case Direction::Sendonly: return sendonlyString(); > + case Direction::Recvonly: return recvonlyString(); > + case Direction::Inactive: return inactiveString(); > + } > + > + ASSERT_NOT_REACHED(); > + return emptyString(); > +} > + > +static bool parseDirectionString(const String& string, RTCRtpTransceiver::Direction& outDirection) > +{ > + if (string == sendrecvString()) > + outDirection = RTCRtpTransceiver::Direction::Sendrecv; > + else if (string == sendonlyString()) > + outDirection = RTCRtpTransceiver::Direction::Sendonly; > + else if (string == recvonlyString()) > + outDirection = RTCRtpTransceiver::Direction::Recvonly; > + else if (string == inactiveString()) > + outDirection = RTCRtpTransceiver::Direction::Inactive; > + else > + return false; > + > + return true; > +} Since bindings create functions like this for all enumerations, we should avoid writing this code ourselves if possible. What makes this necessary? > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:110 > +bool RTCRtpTransceiver::configureWithDictionary(const Dictionary& dictionary) > +{ > + String direction; > + if (dictionary.get("direction", direction)) { > + if (!parseDirectionString(direction, m_direction)) > + return false; > + } > + > + // FIMXE: fix streams > + return true; > +} Since bindings create functions like this for dictionaries, we should avoid writing this code ourselves if possible. What makes this necessary? The bindings should convert dictionaries into structs and we should pass those around. > Source/WebCore/platform/mediastream/MediaEndpoint.h:82 > + virtual RefPtr<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) = 0; Should return Ref, not RefPtr. Caller can’t handle nullptr without crashing. > Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:52 > RealtimeMediaSource::RealtimeMediaSource(const String& id, Type type, const String& name) > - : m_id(id) > + : m_muted(false) > + , m_id(id) > , m_type(type) > , m_name(name) > , m_stopped(false) > - , m_muted(false) > , m_readonly(false) > , m_remote(false) > , m_fitnessScore(0) Most data members should be initialized in the header, not in the constructor, except for the ones where we are initializing from passed in values. > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:43 > RefPtr<MockRealtimeAudioSource> MockRealtimeAudioSource::create() Return type should be Ref, not RefPtr. > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:48 > +RefPtr<MockRealtimeAudioSource> MockRealtimeAudioSource::createMuted(const String& name) Return type should be Ref, not RefPtr. > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:52 > RefPtr<MockRealtimeVideoSource> MockRealtimeVideoSource::create() Return type should be Ref, not RefPtr. > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:57 > +RefPtr<MockRealtimeVideoSource> MockRealtimeVideoSource::createMuted(const String& name) Return type should be Ref, not RefPtr. > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:59 > + RefPtr<MockRealtimeVideoSource> source = adoptRef(new MockRealtimeVideoSource(name)); Should be Ref, not RefPtr. > Source/WebCore/platform/mock/MockRealtimeVideoSource.h:50 > static RefPtr<MockRealtimeVideoSource> create(); > + static RefPtr<MockRealtimeVideoSource> createMuted(const String& name); Return type should be Ref, not RefPtr.

Created attachment 280140 [details] Updated patch Addressed review comments

(In reply to comment #3) > Comment on attachment 280093 [details] > Proposed patch Thanks for reviewing. See comments inline. > View in context: > https://bugs.webkit.org/attachment.cgi?id=280093&action=review > > > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:265 > > + RefPtr<RealtimeMediaSource> remoteSource = m_mediaEndpoint->createMutedRemoteSource(transceiverMid, sourceType); > > + RefPtr<MediaStreamTrackPrivate> remoteTrackPrivate = MediaStreamTrackPrivate::create(WTFMove(remoteSource), trackId); > > + Ref<MediaStreamTrack> remoteTrack = MediaStreamTrack::create(*m_client->scriptExecutionContext(), *remoteTrackPrivate); > > Types should all be auto. > > > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:164 > > + RefPtr<RTCRtpSender> sender = RTCRtpSender::create(WTFMove(track), Vector<String>(), *this); > > + RefPtr<RTCRtpReceiver> receiver = m_backend->createReceiver(transceiverMid, trackKind, trackId); > > + Ref<RTCRtpTransceiver> transceiver = RTCRtpTransceiver::create(WTFMove(sender), WTFMove(receiver)); > > Types should just be auto. > > > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:187 > > + RefPtr<RTCRtpSender> sender = RTCRtpSender::create(kind, Vector<String>(), *this); > > + RefPtr<RTCRtpReceiver> receiver = m_backend->createReceiver(transceiverMid, kind, trackId); > > + Ref<RTCRtpTransceiver> transceiver = RTCRtpTransceiver::create(WTFMove(sender), WTFMove(receiver)); > > Types should just be auto. Fixed all above. I've been thinking about when to use auto. I guess it's OK when there's obvious redundant information; like the above cases :). > > Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:69 > > + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(MediaStreamTrack track, optional Dictionary init); > > + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(DOMString kind, optional Dictionary init); > > New code should never use Dictionary. These dictionaries should defined. > > dictionary RTCRtpTransceiverInit { > boolean send = true; > boolean receive = true; > sequence<MediaStream> streams; > sequence<RTCRtpEncodingParameters> sendEncodings; > }; > > If the bindings generator can’t handle this dictionary, we should fix it so > it can handle it. Every single time we instead use the deprecated > Dictionary, we add code we will need to later delete! Using dictionary from latest published editor's draft [1]. Not all members are supported yet. Should I add a FIXME+bug# for that? [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#idl-def-rtcrtptransceiverinit > > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:98 > > +const String& RTCRtpTransceiver::directionString() const > > +{ > > + switch (m_direction) { > > + case Direction::Sendrecv: return sendrecvString(); > > + case Direction::Sendonly: return sendonlyString(); > > + case Direction::Recvonly: return recvonlyString(); > > + case Direction::Inactive: return inactiveString(); > > + } > > + > > + ASSERT_NOT_REACHED(); > > + return emptyString(); > > +} > > + > > +static bool parseDirectionString(const String& string, RTCRtpTransceiver::Direction& outDirection) > > +{ > > + if (string == sendrecvString()) > > + outDirection = RTCRtpTransceiver::Direction::Sendrecv; > > + else if (string == sendonlyString()) > > + outDirection = RTCRtpTransceiver::Direction::Sendonly; > > + else if (string == recvonlyString()) > > + outDirection = RTCRtpTransceiver::Direction::Recvonly; > > + else if (string == inactiveString()) > > + outDirection = RTCRtpTransceiver::Direction::Inactive; > > + else > > + return false; > > + > > + return true; > > +} > > Since bindings create functions like this for all enumerations, we should > avoid writing this code ourselves if possible. What makes this necessary? Removed parseDirectionString() as part of moving to properly defined dictionary in idl. directionString() is used when the direction value is added to a PeerMediaDescription (which represents direction as a string). I guess there's no nice way to use the generated version from the binding? > > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:110 > > +bool RTCRtpTransceiver::configureWithDictionary(const Dictionary& dictionary) > > +{ > > + String direction; > > + if (dictionary.get("direction", direction)) { > > + if (!parseDirectionString(direction, m_direction)) > > + return false; > > + } > > + > > + // FIMXE: fix streams > > + return true; > > +} > > Since bindings create functions like this for dictionaries, we should avoid > writing this code ourselves if possible. What makes this necessary? The > bindings should convert dictionaries into structs and we should pass those > around. Replaced by defined dictionary in the idl. > > Source/WebCore/platform/mediastream/MediaEndpoint.h:82 > > + virtual RefPtr<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) = 0; > > Should return Ref, not RefPtr. Caller can’t handle nullptr without crashing. True. Fixed. > > Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:52 > > RealtimeMediaSource::RealtimeMediaSource(const String& id, Type type, const String& name) > > - : m_id(id) > > + : m_muted(false) > > + , m_id(id) > > , m_type(type) > > , m_name(name) > > , m_stopped(false) > > - , m_muted(false) > > , m_readonly(false) > > , m_remote(false) > > , m_fitnessScore(0) > > Most data members should be initialized in the header, not in the > constructor, except for the ones where we are initializing from passed in > values. Fixed. Refactored the rest in this init list as well. > > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:43 > > RefPtr<MockRealtimeAudioSource> MockRealtimeAudioSource::create() > > Return type should be Ref, not RefPtr. > > > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:48 > > +RefPtr<MockRealtimeAudioSource> MockRealtimeAudioSource::createMuted(const String& name) > > Return type should be Ref, not RefPtr. > > > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:52 > > RefPtr<MockRealtimeVideoSource> MockRealtimeVideoSource::create() > > Return type should be Ref, not RefPtr. > > > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:57 > > +RefPtr<MockRealtimeVideoSource> MockRealtimeVideoSource::createMuted(const String& name) > > Return type should be Ref, not RefPtr. > > > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:59 > > + RefPtr<MockRealtimeVideoSource> source = adoptRef(new MockRealtimeVideoSource(name)); > > Should be Ref, not RefPtr. > > > Source/WebCore/platform/mock/MockRealtimeVideoSource.h:50 > > static RefPtr<MockRealtimeVideoSource> create(); > > + static RefPtr<MockRealtimeVideoSource> createMuted(const String& name); > > Return type should be Ref, not RefPtr. Fixed all s/RefPtr/Ref/ above.

Created attachment 280145 [details] Updated patch Fix mac build (caused by s/RefPtr/Ref/ changes).

Comment on attachment 280093 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=280093&action=review Noticed a few more things while re-looking at this that we could refine later. >>> Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:69 >>> + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(DOMString kind, optional Dictionary init); >> >> New code should never use Dictionary. These dictionaries should defined. >> >> dictionary RTCRtpTransceiverInit { >> boolean send = true; >> boolean receive = true; >> sequence<MediaStream> streams; >> sequence<RTCRtpEncodingParameters> sendEncodings; >> }; >> >> If the bindings generator can’t handle this dictionary, we should fix it so it can handle it. Every single time we instead use the deprecated Dictionary, we add code we will need to later delete! > > Using dictionary from latest published editor's draft [1]. Not all members are supported yet. Should I add a FIXME+bug# for that? > > [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#idl-def-rtcrtptransceiverinit I don’t think we need a FIXME about each piece that is missing as we implement something new like this. Eventually it might have some value. but during development it’s probably OK to just do things a bit at a time. > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:58 > + if (track) > + setTrack(WTFMove(track)); Not sure the if here is valuable. > Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:47 > virtual ~RTCRtpSenderReceiverBase() { } It’s probably more elegant to write "= default" here instead of writing out an empty body. > Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:53 > + RTCRtpSenderReceiverBase() > + { } It’s probably more elegant to write "= default" here instead of writing out an empty body. >>> Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:98 >>> +} >> >> Since bindings create functions like this for all enumerations, we should avoid writing this code ourselves if possible. What makes this necessary? > > Removed parseDirectionString() as part of moving to properly defined dictionary in idl. directionString() is used when the direction value is added to a PeerMediaDescription (which represents direction as a string). I guess there's no nice way to use the generated version from the binding? Yes, at the moment there is no way to convert an enumeration into the string from the bindings, but if this pattern keeps recurring, I suppose we can add a way.

Comment on attachment 280145 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=280145&action=review > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:195 > + transceiver->setDirection(static_cast<RTCRtpTransceiver::Direction>(init.direction)); This typecast should not be needed. It’s currently only needed because we are defining the enum twice (see my other comments). > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:67 > Vector<RefPtr<RTCRtpSender>> getSenders() const override { return m_senderSet; } > Vector<RefPtr<RTCRtpReceiver>> getReceivers() const { return m_receiverSet; } It’s expensive to have functions that return an existing Vector by copying it. We should not do that unless we have to. > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:78 > + enum class RtpTransceiverDirection { > + Sendrecv, > + Sendonly, > + Recvonly, > + Inactive > + }; Instead of re-defining this here, we should write this: using RtpTransceiverDirection = RTCRtpTransceiver::Direction; > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:82 > + struct RtpTransceiverInit { > + RtpTransceiverDirection direction; > + }; Eventually, we probably also want to define this in RTCRtpTransceiver and use "using" to pull it in here with the appropriate name. > Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:125 > +enum RTCRtpTransceiverDirection { > + "sendrecv", > + "sendonly", > + "recvonly", > + "inactive" > +}; Eventually we will make the bindings script smarter so we don’t have to repeat this in multiple IDL files. In the mean time we might want to comment so anyone modifying one knows they should modify the other. > Source/WebCore/Modules/mediastream/RTCRtpSender.h:60 > + bool isStopped() const { return m_client == nullptr; } WebKit coding style says we write this !m_client rather than m_client == nullptr. > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:80 > + return emptyString(); Maybe return inactiveString() here instead of emptyString()? > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.h:52 > + enum class Direction { > + Sendrecv, > + Sendonly, > + Recvonly, > + Inactive > + }; I find short enumerations like this really nice when writing out all on one line. > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:50 > + Ref<MockRealtimeAudioSource> source = adoptRef(*new MockRealtimeAudioSource(name)); I think auto works better here than writing the type a second time. I’d also omit the empty line below. > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:59 > + Ref<MockRealtimeVideoSource> source = adoptRef(*new MockRealtimeVideoSource(name)); I think auto works better here than writing the type a second time. I’d also omit the empty line below.

Created attachment 280223 [details] Patch for landing

(In reply to comment #7) > Comment on attachment 280093 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280093&action=review > > Noticed a few more things while re-looking at this that we could refine > later. > > >>> Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:69 > >>> + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(DOMString kind, optional Dictionary init); > >> > >> New code should never use Dictionary. These dictionaries should defined. > >> > >> dictionary RTCRtpTransceiverInit { > >> boolean send = true; > >> boolean receive = true; > >> sequence<MediaStream> streams; > >> sequence<RTCRtpEncodingParameters> sendEncodings; > >> }; > >> > >> If the bindings generator can’t handle this dictionary, we should fix it so it can handle it. Every single time we instead use the deprecated Dictionary, we add code we will need to later delete! > > > > Using dictionary from latest published editor's draft [1]. Not all members are supported yet. Should I add a FIXME+bug# for that? > > > > [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#idl-def-rtcrtptransceiverinit > > I don’t think we need a FIXME about each piece that is missing as we > implement something new like this. Eventually it might have some value. but > during development it’s probably OK to just do things a bit at a time. > > > Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:58 > > + if (track) > > + setTrack(WTFMove(track)); > > Not sure the if here is valuable. Let's let setTrack() handle track being null > > Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:47 > > virtual ~RTCRtpSenderReceiverBase() { } > > It’s probably more elegant to write "= default" here instead of writing out > an empty body. Fixed. > > Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:53 > > + RTCRtpSenderReceiverBase() > > + { } > > It’s probably more elegant to write "= default" here instead of writing out > an empty body. Fixed. > >>> Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:98 > >>> +} > >> > >> Since bindings create functions like this for all enumerations, we should avoid writing this code ourselves if possible. What makes this necessary? > > > > Removed parseDirectionString() as part of moving to properly defined dictionary in idl. directionString() is used when the direction value is added to a PeerMediaDescription (which represents direction as a string). I guess there's no nice way to use the generated version from the binding? > > Yes, at the moment there is no way to convert an enumeration into the string > from the bindings, but if this pattern keeps recurring, I suppose we can add > a way. (In reply to comment #8) > Comment on attachment 280145 [details] > Updated patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280145&action=review > > > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:195 > > + transceiver->setDirection(static_cast<RTCRtpTransceiver::Direction>(init.direction)); > > This typecast should not be needed. It’s currently only needed because we > are defining the enum twice (see my other comments). Agreed. See comments below. > > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:67 > > Vector<RefPtr<RTCRtpSender>> getSenders() const override { return m_senderSet; } > > Vector<RefPtr<RTCRtpReceiver>> getReceivers() const { return m_receiverSet; } > > It’s expensive to have functions that return an existing Vector by copying > it. We should not do that unless we have to. The separate fields for storing senders and receivers will be removed shortly since an RTCRtpTransceiver wraps these. They're kept for now to keep the size of this change smaller. The new versions will look into the m_transceiverSet to create the sender and receiver lists so there might be some optimizations to do there as well. These normally contain only a few elements. Sending and receiving 10 streams is quite a lot. > > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:78 > > + enum class RtpTransceiverDirection { > > + Sendrecv, > > + Sendonly, > > + Recvonly, > > + Inactive > > + }; > > Instead of re-defining this here, we should write this: > > using RtpTransceiverDirection = RTCRtpTransceiver::Direction; This would be much nicer. However, applying this change makes the bindings generator generate duplicate conversion functions (jsStringWithCache, parse, convert) for the RTCRtpTransceiverDirection enum with the same C++ enum. I'll leave as is and file a new bug. > > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:82 > > + struct RtpTransceiverInit { > > + RtpTransceiverDirection direction; > > + }; > > Eventually, we probably also want to define this in RTCRtpTransceiver and > use "using" to pull it in here with the appropriate name. > > > Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:125 > > +enum RTCRtpTransceiverDirection { > > + "sendrecv", > > + "sendonly", > > + "recvonly", > > + "inactive" > > +}; > > Eventually we will make the bindings script smarter so we don’t have to > repeat this in multiple IDL files. In the mean time we might want to comment > so anyone modifying one knows they should modify the other. Agreed. Adding comments. > > Source/WebCore/Modules/mediastream/RTCRtpSender.h:60 > > + bool isStopped() const { return m_client == nullptr; } > > WebKit coding style says we write this !m_client rather than m_client == > nullptr. Fixed. > > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:80 > > + return emptyString(); > > Maybe return inactiveString() here instead of emptyString()? Fixed. > > Source/WebCore/Modules/mediastream/RTCRtpTransceiver.h:52 > > + enum class Direction { > > + Sendrecv, > > + Sendonly, > > + Recvonly, > > + Inactive > > + }; > > I find short enumerations like this really nice when writing out all on one > line. The direction enums will likely never be extended so let's make them a one-liner. > > Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:50 > > + Ref<MockRealtimeAudioSource> source = adoptRef(*new MockRealtimeAudioSource(name)); > > I think auto works better here than writing the type a second time. I’d also > omit the empty line below. Fixed. > > Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:59 > > + Ref<MockRealtimeVideoSource> source = adoptRef(*new MockRealtimeVideoSource(name)); > > I think auto works better here than writing the type a second time. I’d also > omit the empty line below. Fixed.

Bindings generator bug: https://bugs.webkit.org/show_bug.cgi?id=158254

Comment on attachment 280223 [details] Patch for landing Clearing flags on attachment: 280223 Committed r201549: <http://trac.webkit.org/changeset/201549>