Bug 142476

Created attachment 248833 [details] patch

I'll file a bug for EFL's jhbuild

Comment on attachment 248833 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=248833&action=review > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:67 > + DEPRECATED_DEFINE_STATIC_LOCAL(RealtimeMediaSourceCenterOwr, center, ()); Nit: is there any reason to not use NeverDestroyed? > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:80 > +void RealtimeMediaSourceCenterOwr::validateRequestConstraints(PassRefPtr<MediaStreamCreationClient> prpClient, PassRefPtr<MediaConstraints> prpAudioConstraints, PassRefPtr<MediaConstraints> prpVideoConstraints) Nit: it would be good to switch from PassRefPtr/RefPtr to Ref before the WebRTC implementation(s) get any bigger. Not necessarily in this patch, but soon. > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:146 > + GList* item; > + > + for (item = sources; item; item = item->next) { Nit: this isn't needed outside of the loop so you shouldn't need to pre-declare it. Can you use "auto"? > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:159 > + if (mediaType & OWR_MEDIA_TYPE_AUDIO) > + sourceType = RealtimeMediaSource::Audio; > + else if (mediaType & OWR_MEDIA_TYPE_VIDEO) > + sourceType = RealtimeMediaSource::Video; else ASSERT()

(In reply to comment #3) > Comment on attachment 248833 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=248833&action=review > > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:67 > > + DEPRECATED_DEFINE_STATIC_LOCAL(RealtimeMediaSourceCenterOwr, center, ()); > > Nit: is there any reason to not use NeverDestroyed? > Hum, nope :) > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:80 > > +void RealtimeMediaSourceCenterOwr::validateRequestConstraints(PassRefPtr<MediaStreamCreationClient> prpClient, PassRefPtr<MediaConstraints> prpAudioConstraints, PassRefPtr<MediaConstraints> prpVideoConstraints) > > Nit: it would be good to switch from PassRefPtr/RefPtr to Ref before the > WebRTC implementation(s) get any bigger. Not necessarily in this patch, but > soon. > Good idea, I'll file a separate patch for this. > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:146 > > + GList* item; > > + > > + for (item = sources; item; item = item->next) { > > Nit: this isn't needed outside of the loop so you shouldn't need to > pre-declare it. Can you use "auto"? > Sure! > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:159 > > + if (mediaType & OWR_MEDIA_TYPE_AUDIO) > > + sourceType = RealtimeMediaSource::Audio; > > + else if (mediaType & OWR_MEDIA_TYPE_VIDEO) > > + sourceType = RealtimeMediaSource::Video; > > else ASSERT() Ok, I can add an ASSERT_NOT_REACHED() here indeed.

Committed r181893: <http://trac.webkit.org/changeset/181893>