Minor cleanups in JingleSession and JingleSessionManager.

remoting/protocol/session_manager.h

Index: remoting/protocol/session_manager.h
diff --git a/remoting/protocol/session_manager.h b/remoting/protocol/session_manager.h
index 376559d6808b1996f1dd32474f3355a7e10976c3..ac1618bac1dbb7dcd4f709f99ead18176dbc1a73 100644
--- a/remoting/protocol/session_manager.h
+++ b/remoting/protocol/session_manager.h
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// The purprose of SessionManager is to facilitate creation of chromotocol
+// The purpose of SessionManager is to facilitate creation of chromotocol
 // sessions. Both host and client use it to establish chromotocol
 // sessions. JingleChromotocolServer implements this inteface using
 // libjingle.
@@ -19,12 +19,11 @@
 // rejected.
 //
 // SESSION OWNERSHIP AND SHUTDOWN
-// SessionManager owns all Chromotocol Session it creates. The server
-// must not be closed while sessions created by the server are still in use.
-// When shutting down the Close() method for the sessionion and the server
-// objects must be called in the following order: Session,
-// SessionManager, JingleClient. The same order must be followed in the case
-// of rejected and failed sessions.
+// SessionManager owns all Sessions it creates. The manager must not
+// be closed while sessions created by the server are still in
+// use. Caller owns Sessions created by a SessionManager (except

[Wez, 2011/07/06 23:26:55]

The comment about closing makes the presence of a Close() call that "closes all
current sessions" confusing; are you trying to make the distinction that the
caller mustn't continue to use any Session after Close() on the manager?  If so
then better to document that requirement on Close() itself.

[Sergey Ulanov, 2011/07/07 00:50:30]

Done.

+// rejected sessions). Sessions must outlive SessionManager, and
+// SessionManager must outlive SignalStrategy.

[Wez, 2011/07/06 23:26:55]

The last comment is exactly the opposite of what's required, isn't it?

[Sergey Ulanov, 2011/07/07 00:50:30]

Done.

 //
 // PROTOCOL VERSION NEGOTIATION
 // When client connects to a host it sends a session-initiate stanza with list
@@ -70,6 +69,9 @@ class SignalStrategy;
 namespace protocol {
 
 // Generic interface for Chromoting session manager.
+//
+// TODO(sergeyu): Split this into two separate interfaces: one for the
+// client side and one for the host side.
 class SessionManager : public base::NonThreadSafe {
  public:
   SessionManager() { }
@@ -88,18 +90,16 @@ class SessionManager : public base::NonThreadSafe {
   // has incompatible configuration, and cannot be accepted.  If the
   // callback accepts session then it must also set configuration for
   // the new session using Session::set_config(). The callback must
-  // take ownership of the session if it accepts connection.
+  // take ownership of the session if it ACCEPTs it.
   typedef Callback2<Session*, IncomingSessionResponse*>::Type
       IncomingSessionCallback;
 
-  // Initializes the session client. Doesn't accept ownership of the
-  // |signal_strategy|. Close() must be called _before_ the |session_manager|
-  // is destroyed.
-  // If this object is used in server mode, then |private_key| and
-  // |certificate| are used to establish a secured communication with the
-  // client. It will also take ownership of these objects.
-  // In case this is used in client mode, pass in NULL for both private key and
-  // certificate.
+  // Initializes the session client. Caller retains ownership of the
+  // |signal_strategy|. If this object is used in server mode, then
+  // |private_key| and |certificate| are used to establish a secured
+  // communication with the client. It will also take ownership of
+  // these objects. In case this is used in client mode, pass in NULL
+  // for both private key and certificate.
   virtual void Init(const std::string& local_jid,
                     SignalStrategy* signal_strategy,
                     IncomingSessionCallback* incoming_session_callback,
@@ -114,9 +114,6 @@ class SessionManager : public base::NonThreadSafe {
   // |config| contains the session configurations that the client supports.
   // |state_change_callback| is called when the connection state changes.
   //
-  // This function may be called from any thread. The |state_change_callback|
-  // is invoked on the network thread.
-  //
   // Ownership of the |config| is passed to the new session.
   virtual Session* Connect(
       const std::string& host_jid,