When we landed Bug 3652, we didn't put the "X-Moz: prefetch" request header in. It's probably a good idea to emit something so sites can, e.g. 404 on prefetch requests, or not do server-log analytics., and so I propose we emit "X-Purpose: prefetch". This is very similar to what Safari generates now for its preview pages ("X-Purpose: preview"), and isn't strangely browser specific like X-Moz was.
We should also mention that we floated this idea in the HTTPbis working group and folks seemed think it was a reasonable idea.
Created attachment 68773 [details] patch to do this, with test
Comment on attachment 68773 [details] patch to do this, with test View in context: https://bugs.webkit.org/attachment.cgi?id=68773&action=review > LayoutTests/http/tests/misc/prefetch-purpose.html:11 > + setTimeout("finishUp()",50); This is racy, no? Also, we should do this unconditionally in the load event handler. > LayoutTests/http/tests/misc/resources/prefetch-purpose.php:6 > + echo $_COOKIE['X-Purpose']; Do we need to clear the cookie in this response? Otherwise, this state will leak into other tests.
(In reply to comment #3) > (From update of attachment 68773 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68773&action=review > > > LayoutTests/http/tests/misc/prefetch-purpose.html:11 > > + setTimeout("finishUp()",50); > > This is racy, no? > Yes, but there's no good way around this. The finishUp() call must occur after prefetches have loaded, and there's no programmatic way to know about this. I think this came up before when we tested prefetching, and I used the same timeout here: http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/HTMLLinkElement/prefetch.html > Also, we should do this unconditionally in the load event handler. > > > LayoutTests/http/tests/misc/resources/prefetch-purpose.php:6 > > + echo $_COOKIE['X-Purpose']; > > Do we need to clear the cookie in this response? Otherwise, this state will leak into other tests. Yes. Fixed, new upload coming.
Created attachment 68778 [details] now clear the cookie
Comment on attachment 68778 [details] now clear the cookie View in context: https://bugs.webkit.org/attachment.cgi?id=68778&action=review > LayoutTests/http/tests/misc/prefetch-purpose.html:11 > + setTimeout("finishUp()",50); Please move this to the load event and move it outside of the conditional. Moving to the load event won't stop the race, but it will be better. Moving it outside of the conditional is important so folks can run the test outside of the test harness.
Created attachment 68781 [details] and move the timer into onload, making this test work better outside of harness
Comment on attachment 68781 [details] and move the timer into onload, making this test work better outside of harness okiedokes. Hopefully this won't be flaky!
Comment on attachment 68781 [details] and move the timer into onload, making this test work better outside of harness Wait, how is it a good idea to make requests larger?
> Wait, how is it a good idea to make requests larger? Prefetch requests? Because servers might want to treat them differently, for example for statistics.
(In reply to comment #9) > (From update of attachment 68781 [details]) > Wait, how is it a good idea to make requests larger? Alexey, thanks for helping review! This patch does nothing to most requests. To some requests, derived from <link rel="prefetch" href="..."> this patch will add a header marking this request as a prefetch "X-Purpose: prefetch". Everything else is left completely untouched; take a look at the unit test and you'll see it has to be fairly careful about how it arranges things to even generate the header (and trickier still to check it). Right now, there's three implementations of prefetching and prefetching like behaviour. Mozilla, when following Link prefetches, adds an "X-Moz: prefetch" header, which is admittedly four bytes shorter. Safari, when generating its bitmap previews, attaches a header "X-Purpose: preview" (which is one byte shorter). Lots of website operators care about prefetch requests, and want to distinguish them. For instance, IMDB and Wikipedia block incoming requests based on the X-Moz header; they 404 these requests, and then a later user initiated navigation will load the page. We didn't implement X-Moz, since the name is so ugly, but I wanted something that gave web site operators a chance to see this. As well, I have started discussions in the HTTPbis working group to standardize this header name, so it may change to just "Purpose" sometime soon. Google searching on X-Purpose finds lots of folks who mutate their site a bit for the (heavier weight) Safari preview loads. Analytics are typically removed. Another use case of this header is log analysis; website operators may want to distinguish link prefetches from user initiated navigation. If you're trying to make the case that link prefetches shouldn't be signalled to server operators at all, I think you'll find that's not going to be a very popular idea with server operators. I think if browsers are going to do prefetching (right now, I'm aware that Chrome & the Android browser do, I do not believe any others have it enabled), then they should signal to the server that they use it. Does the above address your concerns?
The use cases seem questionable to me. - If servers return something different for prefetch, that will confuse intermediate proxies. Servers will need to at least send "Vary: X-Purpose" with the response, and I'm not confident whether it would suffice in practice. I'm particularly suspicious of changing between 404 and 200 based on the Vary header. - If a site doesn't count prefetched requests, it won't see any requests (because the browser will already have the data, and won't make the "real" request). - Why is it good to let servers return 404 for prefetch? We want prefetch to succeed. If there is a need to opt out, let's find a way that doesn't involve wasting time on sending the request.
Generally speaking, it's very common that browsers never do something for what there is a huge demand from authors. Security and privacy reasons are most common, but other considerations may also make a feature undesirable. It may be best to take this discussion to webkit-dev.
Comment on attachment 68781 [details] and move the timer into onload, making this test work better outside of harness Per contention above, r-. Why would we want to let servers know it's a prefetch request? We don't let them know other things about how we're going to display or not-display their content. For example, we don't tell servers that we're about to display their content in a display:none iframe. Or on what screen size, etc. Proxies also don't necessarily tell servers that they're a proxy and thus not displaying the content... This seems like an invalid bug to me. A request is a request. If servers feel they need to filter out requests based on what was done with that request, it seems they should be gathering that information differently (like via JavaScript?).
A better use case mentioned by Gavin on IRC: servers may return a 500 response to prefetch requests if they are close to being overloaded. Another use case I can imagine is corporate network monitoring systems, which may want to know whether you actually navigated to a site, or just had it in search results. Mozilla FAQ <https://developer.mozilla.org/en/Link_prefetching_FAQ> has an answer to how you're supposed to count "real" requests (just send Cache-control: must-revalidate). Personally, I don't find that completely convincing, heavier pages may not support conditional requests at all. I'm still not convinced that this is a good feature to add. Folks on webkit-dev will certainly have more and better ideas for and against it.
I think we should follow whatever the relevant working group decides. So my comments may be invalid. I can only assume that more knowledgeable folks than I have thought about this issue. But on the surface, exposing this in a header seems at best a very slippery slope and likely just plain wrong. :)
I've started a thread in webkit-dev for this.
Comment on attachment 68773 [details] patch to do this, with test Cleared Adam Barth's review+ from obsolete attachment 68773 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 68778 [details] now clear the cookie Cleared Adam Barth's review+ from obsolete attachment 68778 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Noone is commenting in the thread in webkit dev; has this become less controversial?
I have started the dicussion in webkit-dev on this bug again subject "X-Purpose, again!". I hope we can land this change soon.
Comment on attachment 68781 [details] and move the timer into onload, making this test work better outside of harness Looks like the current thinking from webkit-dev is to go with this for the time being. We might want to reconsider after discussing the topic in the HTTPbis WG.
Comment on attachment 68781 [details] and move the timer into onload, making this test work better outside of harness Clearing flags on attachment: 68781 Committed r69420: <http://trac.webkit.org/changeset/69420>
All reviewed patches have been landed. Closing bug.