Bug 79236 – [GStreamer] webkitwebsrc: use HTTP referer provided by MediaPlayer

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Philippe Normand]

MediaPlayer::referrer() was recently added. We should use it.

[Philippe Normand]

Created attachment 128205 [details]
Patch

[Philippe Normand]

Created attachment 128207 [details]
Patch

[Martin Robinson]

Comment on attachment 128207 [details]
Patch

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

[Martin Robinson]

(In reply to comment #3)

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

Oh, I see now that ::refererer() is a recent addition.

[Philippe Normand]

(In reply to comment #4)
> (In reply to comment #3)
> 
> > Looks fine. What caused you to make this change? Is it tracked by any tests? r- for the minor fixes below.
> 
> Oh, I see now that ::refererer() is a recent addition.

Yes, we had code to deal with it already but I believe it makes more sense to use this new method of the player :)

[Philippe Normand]

Created attachment 128249 [details]
Patch

[Philippe Normand]

The test that should still pass is http/tests/media/video-referer.html

[Philippe Normand]

Committed r108524: <http://trac.webkit.org/changeset/108524>