[webkit-reviews] review denied: [Bug 129653] [soup] Propagate TLS error information for resource requests : [Attachment 225865] Set-the-TLS-error-information-if-a-TLS-error-occurred (2)

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Ben Boeckel
<mathstuf at gmail.com>'s request for review:
Bug 129653: [soup] Propagate TLS error information for resource requests
https://bugs.webkit.org/show_bug.cgi?id=129653

Attachment 225865: Set-the-TLS-error-information-if-a-TLS-error-occurred (2)
https://bugs.webkit.org/attachment.cgi?id=225865&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=225865&action=review


What does this fix or how does this improve things? Are you fixing a specific
bug?

> Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:57
> +    if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) {

This should be turned into an early return, so reverse the check and return
generic error immediately.

> Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:61
> +	   ResourceError error = transportError(request, message->status_code,
>	       String::fromUTF8(message->reason_phrase));
> -    else
> -	   return genericGError(error, request);
> +	   if (message->status_code == SOUP_STATUS_SSL_FAILED) {
> +	       GTlsCertificate* certificate;

Nit: an empty line before the if will make this more readable I think.