[Webkit-unassigned] [Bug 107702] Recursion handling cancelled authentication challenges in NetworkProcess

[bugzilla-daemon at webkit.org]

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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #184257|review?                     |review+
               Flag|                            |




--- Comment #2 from Alexey Proskuryakov <ap at webkit.org>  2013-01-23 10:36:49 PST ---
(From update of attachment 184257)
View in context: https://bugs.webkit.org/attachment.cgi?id=184257&action=review

r=me, because this fixes a bug and is generally an improvement. I suggest addressing the below comments though.

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:141
> +        m_handle->cancel();

I don't know if we want to call this from resourceHandleStopped(). I'm pretty sure that this is super confusing, but I don't know if it's necessary given how the code is structured now.

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:294
> +    send(Messages::WebResourceLoader::Cancel());

Can't we just send a didFail with a cancellation error? Having a cancel message from NetworkProcess to WebProcess makes one wonder who's in charge (it's normally WebKit client code that's in charge of cancellation, not NetworkProcess).

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.h:120
> +    bool m_inProgress;

Can this be replaced with checks for m_handle being non-null?

> Source/WebKit2/WebProcess/Network/WebResourceLoader.messages.in:25
> +    Cancel()

As alluded to above, I'd have no idea what this message means from looking at the IDL.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.