[Webkit-unassigned] [Bug 203058] HEAD requests are not cached

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=203058

--- Comment #13 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 382236
  --> https://bugs.webkit.org/attachment.cgi?id=382236
WIP: allow HEADs to use cached GETs

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

> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:214
> +    if (decision == UseDecision::Validate && isHead) {
> +        // Seems silly to validate a HEAD, so just force the real request to go out.
> +        return UseDecision::NoDueToHTTPMethod;
> +    }

I don't fully understand from this comment why you went to the extra trouble to prohibit Validate on HEAD in the cache. Does silly in this context mean that it would be slow, or incorrect, or something else?

If the behavior would be surprising but still correct, it might be best just to let it happen instead of special casing it.

> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:327
> +    bool isHead = request.httpMethod() == "HEAD";
> +    auto retrieveDecision = makeRetrieveDecision(request, isHead);

It's inconsistent to use a boolean to identify HEAD requests and a string comparison to identify GET requests. Let's pick one way and stick with it. If we do, the overall special casing and intrusion of HEAD into the cache's design will be smaller.

I think string comparison may be fine for performance, since it's just three (GET) or four (HEAD) branches. So we could just use that. That said, if we want something better than string comparison, I'd suggest a refactoring patch to add httpMethodForCache() to ResourceRequest, and have it return an enum class { Get, Head, Other }. The ResourceRequest could initialize this member in its constructor, or lazily.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20191105/9c0e5757/attachment.htm>