Add AUTHENTICATED session state.

Add AUTHENTICATED session state.

Also removed AUTHENTICATED state from ConnectionToHost - We no longer need it there.

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

[Wez, 2011-12-03 00:10:37]

LGTM, although I question why we need CONNECTING, CONNECTED and AUTHENTICATED
states; why do callers need to know about the first two?

Also, some comment typos!

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h
File remoting/protocol/session.h (right):

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:38: // session-accept.
Combining this description with those of CONNECTED and AUTHENTICATED, it's not
clear to my why the caller cares about the distinction?

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:41: // Session has been accepted, but authentication
is not finished yet.
nit: "... and is pending authentication."

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:66: // CLOSED or FAILED.
nit: "It is not safe to destroy the session from within the handler unless
|state| is CLOSED or FAILED."

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
File remoting/protocol/session_manager.h (right):

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:22: // Implementations of Session and
SessionManager interfaces delegate
nit: ... of the Session ...

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:23: // authentication to the Authenticator
interface. When accepting
nit: ... to an Authenticator implementation ... ?

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:24: // incoming connection authenticators
are created using authenticator
nit: For incoming connections ...
... created using an AuthenticatorFactory ...

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:25: // factory set via
set_authenticator_factory() method. For outgoing
nit: ... via the set_authenticator_factory() method ...

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:26: // sessions authenticator must be passed
to Connect() method. Session
nit: ... sessions the authenticator must be passed to the Connect() method.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:26: // sessions authenticator must be passed
to Connect() method. Session
nit: The Session's state changes to AUTHENTICATED once authentication succeeds.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:31: // The manager must not be closed or
destroyed before all sessions
Which manager? The SessionManager?

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:34: // sessions). SignalStrategy must
outlive SessionManager.
nit: Missing "the"s

[Sergey Ulanov, 2011-12-06 02:33:20]

I agree that CONNECTING may not be necessary - added TODO for this, but
CONNECTED is still useful - it allows channel creation before auth is finished.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h
File remoting/protocol/session.h (right):

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:38: // session-accept.
On 2011/12/03 00:10:37, Wez wrote:
> Combining this description with those of CONNECTED and AUTHENTICATED, it's not
> clear to my why the caller cares about the distinction?

I think we do need to distinguish between CONNECTED and AUTHENTICATED - in some
cases calling code may want to created channels in CONNECTED state to speed up
connection process.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:41: // Session has been accepted, but authentication
is not finished yet.
On 2011/12/03 00:10:37, Wez wrote:
> nit: "... and is pending authentication."

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session.h#...
remoting/protocol/session.h:66: // CLOSED or FAILED.
On 2011/12/03 00:10:37, Wez wrote:
> nit: "It is not safe to destroy the session from within the handler unless
> |state| is CLOSED or FAILED."

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
File remoting/protocol/session_manager.h (right):

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:22: // Implementations of Session and
SessionManager interfaces delegate
On 2011/12/03 00:10:37, Wez wrote:
> nit: ... of the Session ... 

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:23: // authentication to the Authenticator
interface. When accepting
On 2011/12/03 00:10:37, Wez wrote:
> nit: ... to an Authenticator implementation ... ?

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:24: // incoming connection authenticators
are created using authenticator
On 2011/12/03 00:10:37, Wez wrote:
> nit: For incoming connections ...
> ... created using an AuthenticatorFactory ...

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:25: // factory set via
set_authenticator_factory() method. For outgoing
On 2011/12/03 00:10:37, Wez wrote:
> nit: ... via the set_authenticator_factory() method ...

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:26: // sessions authenticator must be passed
to Connect() method. Session
On 2011/12/03 00:10:37, Wez wrote:
> nit: ... sessions the authenticator must be passed to the Connect() method.

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:26: // sessions authenticator must be passed
to Connect() method. Session
On 2011/12/03 00:10:37, Wez wrote:
> nit: The Session's state changes to AUTHENTICATED once authentication
succeeds.

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:31: // The manager must not be closed or
destroyed before all sessions
On 2011/12/03 00:10:37, Wez wrote:
> Which manager? The SessionManager?

Done.

http://codereview.chromium.org/8774017/diff/4001/remoting/protocol/session_ma...
remoting/protocol/session_manager.h:34: // sessions). SignalStrategy must
outlive SessionManager.
On 2011/12/03 00:10:37, Wez wrote:
> nit: Missing "the"s

Done.

[commit-bot: I haz the power, 2011-12-06 02:42:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/8774017/8013

[commit-bot: I haz the power, 2011-12-06 07:30:17]

Change committed as 113146