[Webkit-unassigned] [Bug 185105] ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse() has comment that states we should avoid sniffing for HTTP 304, but we no longer do

[bugzilla-daemon at webkit.org]

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

--- Comment #1 from Daniel Bates <dbates at webkit.org> ---
Q. How did this happen?

A. Before <https://trac.webkit.org/changeset/224267/> (bug #160677) the code snippet read:

[[
...

        // Avoid MIME type sniffing if the response comes back as 304 Not Modified.
        auto msg = CFURLResponseGetHTTPResponse(cfResponse.get());
        int statusCode = msg ? CFHTTPMessageGetResponseStatusCode(msg) : 0;

        if (statusCode != 304) {
            bool isMainResourceLoad = handle->firstRequest().requester() == ResourceRequest::Requester::Main;
            adjustMIMETypeIfNecessary(cfResponse.get(), isMainResourceLoad);
        }
...
]]
<https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp?rev=224266#L158>

After <https://trac.webkit.org/changeset/224267/> (bug #160677) the code changed to read:

[[
...
        // Avoid MIME type sniffing if the response comes back as 304 Not Modified.
        auto msg = CFURLResponseGetHTTPResponse(cfResponse.get());
        int statusCode = msg ? CFHTTPMessageGetResponseStatusCode(msg) : 0;

        if (statusCode != 304) {
            bool isMainResourceLoad = handle->firstRequest().requester() == ResourceRequest::Requester::Main;
#if !PLATFORM(WIN)
            adjustMIMETypeIfNecessary(cfResponse.get(), isMainResourceLoad);
#endif
        }
...
]]
<https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp?rev=224267#L193>

I am unclear why the !PLATFORM(WIN)-guard was added at all as this code does not seem specific to Cocoa platforms. Moreover, I am unclear why the !PLATFORM(WIN)-guard was added only around adjustMIMETypeIfNecessary() and not this entire snippet as the locals msg, statusCode, and isMainResourceLoad exist solely so that we can write the control flow logic to conditionally call adjustMIMETypeIfNecessary().

I suspect the existence of the !PLATFORM(WIN)-guard caused confusion and hence we removed the guarded code, but did not remove  all the aforementioned locals, in <http://trac.webkit.org/changeset/224846> (bug #179688) and hence this code snippet became  what it is today, repeating the snippet from comment 0:

[[
...
        // Avoid MIME type sniffing if the response comes back as 304 Not Modified.
        auto msg = CFURLResponseGetHTTPResponse(cfResponse.get());
        int statusCode = msg ? CFHTTPMessageGetResponseStatusCode(msg) : 0;

        if (statusCode != 304) {
            bool isMainResourceLoad = m_handle->firstRequest().requester() == ResourceRequest::Requester::MainFrame;
        }
...
]]
<https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp?rev=224846#L191>

-- 
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/20180428/fd5d516b/attachment.html>