[webkit-reviews] review granted: [Bug 184890] Ensure DNT is set for redirections handled in NetworkProcess : [Attachment 338637] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 184890: Ensure DNT is set for redirections handled in NetworkProcess
https://bugs.webkit.org/show_bug.cgi?id=184890

Attachment 338637: Patch

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




--- Comment #15 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 338637
  --> https://bugs.webkit.org/attachment.cgi?id=338637
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338637&action=review

r=me if the following comments are addressed.

> LayoutTests/http/wpt/fetch/dnt-header-after-redirection.html:21
> +    for (var cptr = 0; cptr < 20; cptr++) {

Use let and non-abbreviated variable name such as counter.
Also, 20 should be stored in a variable with a descriptive name such as the
maxPollCount.

> LayoutTests/http/wpt/fetch/dnt-header-after-redirection.html:35
> +    if (!window.testRunner)
> +	   throw "Test requires testRunner API to turn on private browsing";

It appears to me that this test can be run manually if you opened it in a
private browsing mode.
We should add that description in the test instead of error'ing out like this.