[webkit-reviews] review granted: [Bug 96134] <audio> and <video> should send Do Not Track when appropriate : [Attachment 162834] Patch

Fri Sep 7 12:17:30 PDT 2012

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 96134: <audio> and <video> should send Do Not Track when appropriate
https://bugs.webkit.org/show_bug.cgi?id=96134

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=162834&action=review

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:304
>      if ([headerFields.get() count])
>	   [options.get() setObject:headerFields.get()
forKey:@"AVURLAssetHTTPHeaderFieldsKey"];
> +
> +    if
(player()->mediaPlayerClient()->mediaPlayerShouldSendDoNotTrackHTTPHeader())
> +	   [headerFields.get() setObject:@"1" forKey:@"DNT"];

You should set the DNT header field first or AVURLAssetHTTPHeaderFieldsKey
won't be set in the dictionary if there isn't a referrer or a UA header.

>
LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:8
> +
> +

Nit: two blank lines?

>
LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:17

> +
> +    

Ditto.

>
LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:18

> +    error_log(var_export(headers_list(), true));

What does this do?

>
LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:20

> +    
> +

Nit: two blank lines?

> LayoutTests/http/tests/media/video-donottrack.html:11
> +<script src=../../media-resources/video-test.js></script>
> +<script src=../../media-resources/media-file.js></script>
> +<script>
> +    function runTest () {

Nit: is there any reason this isn't in the <head>?