Rename classes for Chromoting:

Rename classes for Chromoting:
  ChromotocolServer -> protocol::SessionManager
  ChromotocolConnection -> protocol::Session

BUG=none
TEST=compiles + make chromoting connection

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

[garykac, 2010-11-02 15:26:06]

Things to note:

We now have protocol::Session and cricket::Session

We now have protocol::SessionManager and a SessionManager in the host directory.
I haven't thought too much about it, but it would be nice to rename the host
SessionManager.

We have JingleChromotocolServer and JingleChromotocolConnection. It seems like
these should be JingleSessionManager and JingleSession for consistency, but
those names seem wrong to me.

In the Jingle code, I renamed method names with ChromotocolConnection to
Session.  There are also many 'Connection's in this code, many of which are
jingle connections, but I changed the ones that I thought were
ChromotocolConnections.

Please double check these.

[awong, 2010-11-02 16:50:58]

The SessionManager name conflict is annoying.

The plan is to rename the host's SessionManager into something else.  Haven't
quite decided yet.

We were also just going to live with the conflict with the cricket code. It's
not supposed to leak out beyond this interface.

About JingleChromotingServer and JinglechromotingConnection, yes, those were
supposed to change to JingleSessionManager and JingleSession.  What worried you
about these?

Get sergey to double check these as well.

http://codereview.chromium.org/4313001/diff/2001/3002
File remoting/client/jingle_host_connection.h (right):

http://codereview.chromium.org/4313001/diff/2001/3002#newcode56
remoting/client/jingle_host_connection.h:56: // Callback for chromoting
SessionManager.
This should be chromotocol SessionManager and the below should be chromotocol
session since these classes are in the protocol layer.

http://codereview.chromium.org/4313001/diff/2001/3009
File remoting/protocol/jingle_chromotocol_connection.cc (right):

http://codereview.chromium.org/4313001/diff/2001/3009#newcode20
remoting/protocol/jingle_chromotocol_connection.cc:20: using cricket::Session;
Remove this using

http://codereview.chromium.org/4313001/diff/2001/3009#newcode227
remoting/protocol/jingle_chromotocol_connection.cc:227: // If we initiate the
connection, we get to specify the content name. When
connection -> session

http://codereview.chromium.org/4313001/diff/2001/3009#newcode271
remoting/protocol/jingle_chromotocol_connection.cc:271: // Set config for
outgoing connections.
How about, set the config when we are initiating the session.

http://codereview.chromium.org/4313001/diff/2001/3010
File remoting/protocol/jingle_chromotocol_connection.h (right):

http://codereview.chromium.org/4313001/diff/2001/3010#newcode38
remoting/protocol/jingle_chromotocol_connection.h:38: // ChromotocolConnection
interface.
This should be Session

http://codereview.chromium.org/4313001/diff/2001/3012
File remoting/protocol/jingle_chromotocol_server.h (right):

http://codereview.chromium.org/4313001/diff/2001/3012#newcode32
remoting/protocol/jingle_chromotocol_server.h:32: // ChromotocolConnection
interface.
ChromotocolSession

http://codereview.chromium.org/4313001/diff/2001/3015
File remoting/protocol/session_manager.h (right):

http://codereview.chromium.org/4313001/diff/2001/3015#newcode71
remoting/protocol/session_manager.h:71: // IncomingConnectionCallback is called
when a new connection is received. If
I think this is actually IncomingSessionCallback.  Or maybe
SessionInitiationCallback?  Ask Sergey.

The IncomingConnectionResponse should be changed in tandem.

http://codereview.chromium.org/4313001/diff/2001/3015#newcode81
remoting/protocol/session_manager.h:81: // Initializes connection to the host
|jid|.  Ownership of the
I think almost all the "connections" in this file should be "sessions."

A session is a configured set of channels, which is what this thing is supposed
to be managing.

[garykac, 2010-11-02 21:37:04]

http://codereview.chromium.org/4313001/diff/2001/3002
File remoting/client/jingle_host_connection.h (right):

http://codereview.chromium.org/4313001/diff/2001/3002#newcode56
remoting/client/jingle_host_connection.h:56: // Callback for chromoting
SessionManager.
On 2010/11/02 16:50:58, awong wrote:
> This should be chromotocol SessionManager and the below should be chromotocol
> session since these classes are in the protocol layer.

Done.

http://codereview.chromium.org/4313001/diff/2001/3009
File remoting/protocol/jingle_chromotocol_connection.cc (right):

http://codereview.chromium.org/4313001/diff/2001/3009#newcode20
remoting/protocol/jingle_chromotocol_connection.cc:20: using cricket::Session;
On 2010/11/02 16:50:58, awong wrote:
> Remove this using

Done.

http://codereview.chromium.org/4313001/diff/2001/3009#newcode227
remoting/protocol/jingle_chromotocol_connection.cc:227: // If we initiate the
connection, we get to specify the content name. When
On 2010/11/02 16:50:58, awong wrote:
> connection -> session
> 

Done.

http://codereview.chromium.org/4313001/diff/2001/3009#newcode271
remoting/protocol/jingle_chromotocol_connection.cc:271: // Set config for
outgoing connections.
On 2010/11/02 16:50:58, awong wrote:
> How about, set the config when we are initiating the session.

"Set the config if we are the one who initiated the session."

http://codereview.chromium.org/4313001/diff/2001/3010
File remoting/protocol/jingle_chromotocol_connection.h (right):

http://codereview.chromium.org/4313001/diff/2001/3010#newcode38
remoting/protocol/jingle_chromotocol_connection.h:38: // ChromotocolConnection
interface.
On 2010/11/02 16:50:58, awong wrote:
> This should be Session

Done.

http://codereview.chromium.org/4313001/diff/2001/3012
File remoting/protocol/jingle_chromotocol_server.h (right):

http://codereview.chromium.org/4313001/diff/2001/3012#newcode32
remoting/protocol/jingle_chromotocol_server.h:32: // ChromotocolConnection
interface.
On 2010/11/02 16:50:58, awong wrote:
> ChromotocolSession

Done.

http://codereview.chromium.org/4313001/diff/2001/3015
File remoting/protocol/session_manager.h (right):

http://codereview.chromium.org/4313001/diff/2001/3015#newcode71
remoting/protocol/session_manager.h:71: // IncomingConnectionCallback is called
when a new connection is received. If
On 2010/11/02 16:50:58, awong wrote:
> I think this is actually IncomingSessionCallback.  Or maybe
> SessionInitiationCallback?  Ask Sergey.
> 
> The IncomingConnectionResponse should be changed in tandem.

Done.

http://codereview.chromium.org/4313001/diff/2001/3015#newcode81
remoting/protocol/session_manager.h:81: // Initializes connection to the host
|jid|.  Ownership of the
On 2010/11/02 16:50:58, awong wrote:
> I think almost all the "connections" in this file should be "sessions."
> 
> A session is a configured set of channels, which is what this thing is
supposed
> to be managing.

Done.

[Sergey Ulanov, 2010-11-02 22:14:10]

LGTM, with some nits.
ChromotocolConfig also needs to be renamed but it can be done in a separate
change.

http://codereview.chromium.org/4313001/diff/13001/14002
File remoting/client/jingle_host_connection.h (right):

http://codereview.chromium.org/4313001/diff/13001/14002#newcode37
remoting/client/jingle_host_connection.h:37: 
JingleThread is from jingle_glue, and it shouldn't be in the protocol namespace.
We can put it in remoting::jingle_glue:: or in jingle_glue:: if necessary.

http://codereview.chromium.org/4313001/diff/13001/14002#newcode60
remoting/client/jingle_host_connection.h:60: 
s/connection/session/

http://codereview.chromium.org/4313001/diff/13001/14016
File remoting/protocol/jingle_session_manager.h (right):

http://codereview.chromium.org/4313001/diff/13001/14016#newcode120
remoting/protocol/jingle_session_manager.h:120: 
I think it would be better if this was called sessions_.

http://codereview.chromium.org/4313001/diff/13001/14020
File remoting/protocol/session_manager.h (right):

http://codereview.chromium.org/4313001/diff/13001/14020#newcode58
remoting/protocol/session_manager.h:58: 
I wonder if we really need this empty line. Putting these two namespace
definitions together would make it more readable, no?

http://codereview.chromium.org/4313001/diff/13001/14020#newcode84
remoting/protocol/session_manager.h:84: CandidateChromotocolConfig*
chromotocol_config,
We will also need to rename ChromotocolConfig. This can be done in a separate
change.

http://codereview.chromium.org/4313001/diff/13001/14022
File remoting/remoting.gyp (right):

http://codereview.chromium.org/4313001/diff/13001/14022#newcode359
remoting/remoting.gyp:359: 
Alphabet order

[awong, 2010-11-03 00:17:33]

LGTM

Thanks wading through all of that!

[Timur Iskhodzhanov, 2010-11-04 13:50:17]

FYI
This patch has made TSan Windows bot on the memory waterfall red for 15+ hours
due to renaming of excluded tests.
Please pay attention to the memory waterfall after commiting - the changes to
remoting has started making it red too often recently...