[webkit-reviews] review granted: [Bug 65330] Remove auth-related functions from SubresourceLoaderClient : [Attachment 103316] Remove receivedCancellation() - with better ChangeLog

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Nate Chapin
<japhet at chromium.org>'s request for review:
Bug 65330: Remove auth-related functions from SubresourceLoaderClient
https://bugs.webkit.org/show_bug.cgi?id=65330

Attachment 103316: Remove receivedCancellation() - with better ChangeLog
https://bugs.webkit.org/attachment.cgi?id=103316&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103316&action=review


> Source/WebCore/ChangeLog:9
> +	   Remove receivedCancellation() from SubresourceLoaderClient.
> +	   It's a no-op because it's always immediately followed by a
> +	   canceallation (ResourceLoader::receivedCancellation() calls
> +	   cancel(), which leads to calling
SubresourceLoaderClinet::didFail()).
> +	   For WorkerScriptLoader and Notification, receivedCancellation()
> +	   and didFail() are identical. For XMLHttpRequest,
receivedCancellation()
> +	   sets m_response, which didFail() clears before it can be used.

Please don't put explanations into ChangeLog like this. There is an
automatically generated list of files below. Simply listing modified files and
functions is not very helpful, and the reason for the script to add it is that
contributors will use it as a starting point.

> Source/WebCore/xml/XMLHttpRequest.cpp:-1054
> -void XMLHttpRequest::didReceiveAuthenticationCancellation(unsigned long,
const ResourceResponse& failureResponse)
> -{
> -    m_response = failureResponse;
> -}

This code was added in <http://trac.webkit.org/changeset/24862> without a
regression test. But it does seem fine to remove it now - WebKit (at least
ports that I checked) no longer does a cancellation when the user cancels an
authentication sheet. It now asks WebKit to continue without credentials.