[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





--- Comment #3 from Brady Eidson <beidson at apple.com>  2013-01-23 10:41:19 PST ---
(In reply to comment #2)
> (From update of attachment 184257 [details])
> 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.

I think the cancel can be performed from the authentication-specific method, and resourceHandleStopped can just destroy the handle itself.

> > 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).

This is a structural problem with the authentication APIs we build on and how authentication works in WebCore and the WebProcess... this isn't actually a "did fail with an error" message.  It is a command to actually perform a cancel, which is to replicate current WebProcess behavior.

> > 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.

I agree the naming is confusing and I'll make that better.

> > Source/WebKit2/NetworkProcess/NetworkResourceLoader.h:120
> > +    bool m_inProgress;
> 
> Can this be replaced with checks for m_handle being non-null?

Yes.

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