Add proxy basic auth support in net/socket_stream.

Add proxy basic auth support in net/socket_stream.

BUG=none
TEST=net_unittests passes

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30176

[tyoshino (SeeGerritForStatus), 2009-10-26 09:06:42]

http://codereview.chromium.org/330016/diff/1/3
File net/socket_stream/socket_stream.cc (right):

http://codereview.chromium.org/330016/diff/1/3#newcode349
Line 349: if (result != ERR_IO_PENDING && result != OK)
This is OK, but can we arrive here with result == OK?

[Yuzo, 2009-10-26 10:09:31]

http://codereview.chromium.org/330016/diff/1/3
File net/socket_stream/socket_stream.cc (right):

http://codereview.chromium.org/330016/diff/1/3#newcode160
Line 160: if (!socket_.get())
When can this be true? Is it worth logging/DCHECK?

http://codereview.chromium.org/330016/diff/1/3#newcode580
Line 580: // Wait RestartWithAuth or Close is called.
Wait until?

[ukai, 2009-10-26 11:23:05]

http://codereview.chromium.org/330016/diff/1/3
File net/socket_stream/socket_stream.cc (right):

http://codereview.chromium.org/330016/diff/1/3#newcode160
Line 160: if (!socket_.get())
On 2009/10/26 10:09:31, Yuzo wrote:
> When can this be true? Is it worth logging/DCHECK?

Done.

http://codereview.chromium.org/330016/diff/1/3#newcode349
Line 349: if (result != ERR_IO_PENDING && result != OK)
On 2009/10/26 09:06:42, tyoshino wrote:
> This is OK, but can we arrive here with result == OK?

Done.

http://codereview.chromium.org/330016/diff/1/3#newcode580
Line 580: // Wait RestartWithAuth or Close is called.
On 2009/10/26 10:09:31, Yuzo wrote:
> Wait until?

Done.

[Yuzo, 2009-10-27 02:00:09]

http://codereview.chromium.org/330016/diff/3001/2004
File net/socket_stream/socket_stream.h (right):

http://codereview.chromium.org/330016/diff/3001/2004#newcode71
Line 71: // If there is credential for |auth_info|, use RestartWithAuth().
To me this comment is somewhat hard to understand.

Do you mean:
  auth_info contains credential -> RestartWithAuth is called
  it does not -> Close is called
?

http://codereview.chromium.org/330016/diff/3001/2004#newcode74
Line 74: AuthChallengeInfo* auth_info) {
You might want to comment something like:
// No credential is available. Give up and close the connection.

[Yuzo, 2009-10-27 04:13:54]

http://codereview.chromium.org/330016/diff/3001/2004
File net/socket_stream/socket_stream.h (right):

http://codereview.chromium.org/330016/diff/3001/2004#newcode36
Line 36: // It provides plain full-duplex stream with proxy and SSL support.
It might help the reader understand the behavior if you
could explain in what order authentication identities are
tried, unless it is common knowledge.

If I read the code correctly, identity is first look up by
path, then by realm.

[ukai, 2009-10-27 04:22:20]

http://codereview.chromium.org/330016/diff/3001/2004
File net/socket_stream/socket_stream.h (right):

http://codereview.chromium.org/330016/diff/3001/2004#newcode36
Line 36: // It provides plain full-duplex stream with proxy and SSL support.
On 2009/10/27 04:13:54, Yuzo wrote:
> It might help the reader understand the behavior if you
> could explain in what order authentication identities are
> tried, unless it is common knowledge.
> 
> If I read the code correctly, identity is first look up by
> path, then by realm.

Done.

http://codereview.chromium.org/330016/diff/3001/2004#newcode71
Line 71: // If there is credential for |auth_info|, use RestartWithAuth().
On 2009/10/27 02:00:09, Yuzo wrote:
> To me this comment is somewhat hard to understand.
> 
> Do you mean:
>   auth_info contains credential -> RestartWithAuth is called
>   it does not -> Close is called
> ?
> 

No. auth_info doesn't contain credential.  See net/base/auth.h for detail about
auth_info.
browser should find credential (or ask user to input credential for http
browsing case, but I think we'll just lookup webdatabase for websocket case),
and if it is found, use RestartWithAuth().

http://codereview.chromium.org/330016/diff/3001/2004#newcode74
Line 74: AuthChallengeInfo* auth_info) {
On 2009/10/27 02:00:09, Yuzo wrote:
> You might want to comment something like:
> // No credential is available. Give up and close the connection.
> 

Done.

[Yuzo, 2009-10-27 04:53:58]

LGTM

[tyoshino (SeeGerritForStatus), 2009-10-27 09:25:33]

LGTM

http://codereview.chromium.org/330016/diff/3003/3005
File net/socket_stream/socket_stream.cc (right):

http://codereview.chromium.org/330016/diff/3003/3005#newcode761
Line 761: return;
remove