Bug 102592 – NetworkProcess Authentication

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Get basic authentication working in the NetworkProcess

[Brady Eidson]

Created attachment 174836 [details]
Patch v1 - Take care of basic authentication

FIXMEs mark some more advanced outliers like dealing with downloads and handling client certs.

But this gets basic "enter username/password for a subresource" authentication working.

[Build Bot]

Comment on attachment 174836 [details]
Patch v1 - Take care of basic authentication

Attachment 174836 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14875522

New failing tests:
media/media-continues-playing-after-replace-source.html
inspector-protocol/nmi-webaudio.html

[Brady Eidson]

(In reply to comment #2)
> (From update of attachment 174836 [details])
> Attachment 174836 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/14875522
> 
> New failing tests:
> media/media-continues-playing-after-replace-source.html
> inspector-protocol/nmi-webaudio.html

Can't be related to this patch.  The only changes (besides a *relaxed* ASSERT) are in NetworkProcess code.

[Maciej Stachowiak]

Comment on attachment 174836 [details]
Patch v1 - Take care of basic authentication

View in context: https://bugs.webkit.org/attachment.cgi?id=174836&action=review

> Source/WebKit2/ChangeLog:8
> +        This get's basic HTTP authentication working with the WebProcess by dispatching authentication

Did you mean "with the NetworkProcess"?

[Brady Eidson]

(In reply to comment #4)
> (From update of attachment 174836 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174836&action=review
> 
> > Source/WebKit2/ChangeLog:8
> > +        This get's basic HTTP authentication working with the WebProcess by dispatching authentication
> 
> Did you mean "with the NetworkProcess"?

I did mean that, yes.  Will fix.

[Alexey Proskuryakov]

Comment on attachment 174836 [details]
Patch v1 - Take care of basic authentication

View in context: https://bugs.webkit.org/attachment.cgi?id=174836&action=review

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:317
> +    return OSAtomicIncrement64Barrier(&uniqueCanAuthenticateAgainstProtectionSpaceID);

Why not use WTF::atomicIncrement?

> Source/WebKit2/WebProcess/Network/WebResourceLoader.h:94
>      RefPtr<WebCore::ResourceLoader> m_coreLoader;
> +    WebCore::AuthenticationChallenge m_currentAuthenticationChallenge;

What is the reason to not use a pointer? Not all requests even have authentication.

[Brady Eidson]

(In reply to comment #6)
> (From update of attachment 174836 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174836&action=review
> 
> > Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:317
> > +    return OSAtomicIncrement64Barrier(&uniqueCanAuthenticateAgainstProtectionSpaceID);
> 
> Why not use WTF::atomicIncrement?

Great question.  I didn't know about it until now.

That said, there's no 64 bit specialization of it yet, and the Darwin specific version is already scattered about.

I'll replace them all in a single followup.

> > Source/WebKit2/WebProcess/Network/WebResourceLoader.h:94
> >      RefPtr<WebCore::ResourceLoader> m_coreLoader;
> > +    WebCore::AuthenticationChallenge m_currentAuthenticationChallenge;
> 
> What is the reason to not use a pointer? Not all requests even have authentication.

This makes sense to do.  Will change.

[Brady Eidson]

http://trac.webkit.org/changeset/135179

[Alexey Proskuryakov]

Comment on attachment 174836 [details]
Patch v1 - Take care of basic authentication

View in context: https://bugs.webkit.org/attachment.cgi?id=174836&action=review

> Source/WebKit2/WebProcess/Network/WebResourceLoader.h:34
> +#include <WebCore/AuthenticationChallenge.h>

I think that you could get rid of this include now.

[Brady Eidson]

http://trac.webkit.org/changeset/135198 as that followup