[webkit-reviews] review denied: [Bug 79236] [GStreamer] webkitwebsrc: use HTTP referer provided by MediaPlayer : [Attachment 128207] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 79236: [GStreamer] webkitwebsrc: use HTTP referer provided by MediaPlayer
https://bugs.webkit.org/show_bug.cgi?id=79236

Attachment 128207: Patch
https://bugs.webkit.org/attachment.cgi?id=128207&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=128207&action=review


Looks fine. What caused you to make this change? Is it tracked by any tests? r-
for the minor fixes below.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:358
>	   priv->frame.release();

This should be priv->frame.clear()

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:427
> +	   if (priv->player)
> +	       request.setHTTPReferrer(priv->player->referrer());
> +

It seems this should be outside of the if block now.

I think you can simplify this a bit to be:
FrameLoader* loader = priv->frame ? priv->frame->loader() : 0;
if (loader) {
    ....
}

if (priv->player)
    request.setHTTPReferrer(priv->player->referrer());

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:752
> +

Extra newline here