Simplify ConnectionToHost interface.

Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.

Committed: https://crrev.com/06fa7c566663ee46f417da43c8de7746b1104ff0
Cr-Commit-Position: refs/heads/master@{#365854}

[Sergey Ulanov, 2015-12-16 21:36:17]

Description was changed from

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test it and this functionality would need to
be duplicated between all implementations. Moved this logic to
ChromotingClient.
==========

to

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.
==========

[Sergey Ulanov, 2015-12-16 21:36:31]

sergeyu@chromium.org changed reviewers:
+ joedow@chromium.org

[joedow, 2015-12-17 16:59:45]

lgtm

https://codereview.chromium.org/1520323007/diff/20001/remoting/client/chromot...
File remoting/client/chromoting_client.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/client/chromot...
remoting/client/chromoting_client.cc:76: signal_strategy_ = signal_strategy;
No one should be calling this twice (perhaps for retry logic?) but would it make
sense to add a check to make sure signal_strategy_ hasn't been assigned? 
Otherwise you would never remove your listener from the old strategy.

https://codereview.chromium.org/1520323007/diff/20001/remoting/protocol/conne...
File remoting/protocol/connection_to_host_impl.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/protocol/conne...
remoting/protocol/connection_to_host_impl.cc:53: session_ = std::move(session);
Why use std::move here instead of passing the scoped_ptr?

https://codereview.chromium.org/1520323007/diff/20001/remoting/test/test_chro...
File remoting/test/test_chromoting_client_unittest.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/test/test_chro...
remoting/test/test_chromoting_client_unittest.cc:35: bool is_connected_to_host_
=false;
add space -> "= false;"

[Sergey Ulanov, 2015-12-17 17:32:33]

https://codereview.chromium.org/1520323007/diff/20001/remoting/client/chromot...
File remoting/client/chromoting_client.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/client/chromot...
remoting/client/chromoting_client.cc:76: signal_strategy_ = signal_strategy;
On 2015/12/17 16:59:45, joedow wrote:
> No one should be calling this twice (perhaps for retry logic?) but would it
make
> sense to add a check to make sure signal_strategy_ hasn't been assigned? 
> Otherwise you would never remove your listener from the old strategy.

Start() is not supposed to be called more than once. Added a DCHECK() for
session_manager_.

https://codereview.chromium.org/1520323007/diff/20001/remoting/protocol/conne...
File remoting/protocol/connection_to_host_impl.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/protocol/conne...
remoting/protocol/connection_to_host_impl.cc:53: session_ = std::move(session);
On 2015/12/17 16:59:45, joedow wrote:
> Why use std::move here instead of passing the scoped_ptr?

No good reason. Changed it back to Pass().
I think at some point we will want to replace Pass() with std::move()
everywhere, but it's better to keep the code consistent.

https://codereview.chromium.org/1520323007/diff/20001/remoting/test/test_chro...
File remoting/test/test_chromoting_client_unittest.cc (right):

https://codereview.chromium.org/1520323007/diff/20001/remoting/test/test_chro...
remoting/test/test_chromoting_client_unittest.cc:35: bool is_connected_to_host_
=false;
On 2015/12/17 16:59:45, joedow wrote:
> add space -> "= false;"

Done.

[Sergey Ulanov, 2015-12-17 17:32:37]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2015-12-17 17:32:37]

The patchset sent to the CQ was uploaded after l-g-t-m from joedow@chromium.org
Link to the patchset: https://codereview.chromium.org/1520323007/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-12-17 17:34:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1520323007/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1520323007/40001

[commit-bot: I haz the power, 2015-12-17 18:17:13]

Description was changed from

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.
==========

to

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.
==========

[commit-bot: I haz the power, 2015-12-17 18:17:13]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-17 18:17:55]

Description was changed from

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.
==========

to

==========
Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.

Committed: https://crrev.com/06fa7c566663ee46f417da43c8de7746b1104ff0
Cr-Commit-Position: refs/heads/master@{#365854}
==========

[commit-bot: I haz the power, 2015-12-17 18:17:56]

Patchset 3 (id:??) landed as
https://crrev.com/06fa7c566663ee46f417da43c8de7746b1104ff0
Cr-Commit-Position: refs/heads/master@{#365854}