Add JingleSession::OnRouteChange(), and pass IP endpoint information to a

Add JingleSession::OnRouteChange(), and pass IP endpoint information to a
callback (registered via new Session interface method).

This is the first of a series of CLs aimed at logging client IP addresses in
Remoting Me2Me Host.

BUG=109682
TEST=Manual


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

[Sergey Ulanov, 2012-01-23 20:33:36]

LGTM with several nits.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/fake_session.cc
File remoting/protocol/fake_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/fake_session....
remoting/protocol/fake_session.cc:247: }
NOTIMPLEMENTED()

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/jingle_sessio...
File remoting/protocol/jingle_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/jingle_sessio...
remoting/protocol/jingle_session.cc:142: DCHECK(!callback.is_null());
I don't think we need this DCHECK. The caller may want to reset the callback.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/pepper_sessio...
File remoting/protocol/pepper_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/pepper_sessio...
remoting/protocol/pepper_session.cc:54: NOTREACHED();
add comments saying that this callback is not used on the client side yet.

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

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:72: typedef base::Callback<void(
maybe add TODO to merge these two callbacks to a single interface. Just to make
this code consistent with everything else.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:85: // Must be called on the jingle thread only.
This comment is not necessary anymore, Session interface is not thread-safe now,
and it is not necessary to state it here.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:88: // Set callback that is called when the route
for a channel is changed.
It's also worth mentioning that the callback must be registered immediately
after JingleSessionManager::Connect() or from OnIncomingSession() callback.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:89: // Must be called on the jingle thread only.
remove this comments

[Lambros, 2012-01-24 19:50:50]

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/fake_session.cc
File remoting/protocol/fake_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/fake_session....
remoting/protocol/fake_session.cc:247: }
On 2012/01/23 20:33:37, sergeyu wrote:
> NOTIMPLEMENTED()

Done.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/jingle_sessio...
File remoting/protocol/jingle_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/jingle_sessio...
remoting/protocol/jingle_session.cc:142: DCHECK(!callback.is_null());
On 2012/01/23 20:33:37, sergeyu wrote:
> I don't think we need this DCHECK. The caller may want to reset the callback.

Done.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/pepper_sessio...
File remoting/protocol/pepper_session.cc (right):

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/pepper_sessio...
remoting/protocol/pepper_session.cc:54: NOTREACHED();
On 2012/01/23 20:33:37, sergeyu wrote:
> add comments saying that this callback is not used on the client side yet.

Done.

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

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:72: typedef base::Callback<void(
On 2012/01/23 20:33:37, sergeyu wrote:
> maybe add TODO to merge these two callbacks to a single interface. Just to
make
> this code consistent with everything else.

Done.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:85: // Must be called on the jingle thread only.
On 2012/01/23 20:33:37, sergeyu wrote:
> This comment is not necessary anymore, Session interface is not thread-safe
now,
> and it is not necessary to state it here.

Done.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:88: // Set callback that is called when the route
for a channel is changed.
On 2012/01/23 20:33:37, sergeyu wrote:
> It's also worth mentioning that the callback must be registered immediately
> after JingleSessionManager::Connect() or from OnIncomingSession() callback.

Done.

http://codereview.chromium.org/9271026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:89: // Must be called on the jingle thread only.
On 2012/01/23 20:33:37, sergeyu wrote:
> remove this comments

Done.

[commit-bot: I haz the power, 2012-01-24 21:44:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/9271026/5001

[commit-bot: I haz the power, 2012-01-25 00:30:57]

Change committed as 118956