[webkit-reviews] review granted: [Bug 126129] [GTK] Test /webkit2/WebKitWebView/authentication-cancel fails with network process enabled : [Attachment 220153] Updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 9 10:17:37 PST 2014


Alexey Proskuryakov <ap at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 126129: [GTK] Test /webkit2/WebKitWebView/authentication-cancel fails with
network process enabled
https://bugs.webkit.org/show_bug.cgi?id=126129

Attachment 220153: Updated patch
https://bugs.webkit.org/attachment.cgi?id=220153&action=review

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


> Source/WebKit2/ChangeLog:3
> +	   [GTK] Test /webkit2/WebKitWebView/authentication-cancel fails with
network process enabled

This patch is cross-platform, please remove "[GTK]" here and from the bug
title.

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:174
> +void NetworkResourceLoader::cancel()

I'm not a fan of this function. It looks like someone tells
NetworkResourceLoader to cancel, but this is not what it is - it implements
what needs to be done in two places where NetworkResourceLoader decides to
cancel the load on its own, based on indirect hints. In other future places
where NetworkResourceLoader decides to cancel, it may need to do something
else.

I would have just put the code inline, not factoring it out into a cancel()
function. But at the very least, the function should be private, not public.


More information about the webkit-reviews mailing list