[webkit-reviews] review granted: [Bug 191599] [GStreamer][MediaStream] Handle track addition and removal : [Attachment 355036] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Thibault Saunier
<tsaunier at gnome.org>'s request for review:
Bug 191599: [GStreamer][MediaStream] Handle track addition and removal
https://bugs.webkit.org/show_bug.cgi?id=191599

Attachment 355036: Patch

https://bugs.webkit.org/attachment.cgi?id=355036&action=review




--- Comment #7 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 355036
  --> https://bugs.webkit.org/attachment.cgi?id=355036
Patch

(In reply to Thibault Saunier from comment #6)
> > >
Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:51
8
> > > +    g_return_val_if_fail(WEBKIT_IS_MEDIA_STREAM_SRC(self), FALSE);
> > 
> > I don't think this is needed here. If you still consider it needed, please
> > turn it into an ASSERT instead of g_return_val_if_fail (which btw, should
be
> > returning false instead of FALSE now that it is a bool instead of a
> > gboolean).
> 
> Well it is external API, better safe guard it?
> ASSERT is not the same thing, `g_return_val_if_fail` leads to a runtime
> warning, not an ASSERT by default.

If it remains inside WebKit without being exposed to C or GTK APIs, then we
should stick to the ASSERT.