diff options
| author | wez@chromium.org <wez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-12-07 09:06:47 +0000 |
|---|---|---|
| committer | wez@chromium.org <wez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-12-07 09:06:47 +0000 |
| commit | 24a2a9d27860b25af80eacf98407ad03fd78e811 (patch) | |
| tree | 314fd8d27ed9af9c7a4ae3623ede28d99a0495d4 /remoting/host/client_session.cc | |
| parent | b90785fff5deb3cdebaf0be2da39f4942b5203f1 (diff) | |
| download | chromium_src-24a2a9d27860b25af80eacf98407ad03fd78e811.zip chromium_src-24a2a9d27860b25af80eacf98407ad03fd78e811.tar.gz chromium_src-24a2a9d27860b25af80eacf98407ad03fd78e811.tar.bz2 | |
Fix ClientSession to not access ConnectionToClient stubs after Disconnect().
This CL Stop()s the audio and video scheduler components, and invalidates the
client clipboard stub WeakPtr factory to ensure that the corresponding stubs
will not continue to be accessed between Disconnect() and the ClientSession
being Stop()ed.
The ChromotingHost unit-tests are also fixed to correctly notify
ClientSession::OnConnectedClosed() when the ClientSession disconnects the mock
ConnectionToClient, to properly exercise the ChromotingHost's client tear-down.
BUG=163157
Review URL: https://chromiumcodereview.appspot.com/11419292
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171743 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'remoting/host/client_session.cc')
| -rw-r--r-- | remoting/host/client_session.cc | 50 |
1 files changed, 31 insertions, 19 deletions
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc index 1ed7503..c4fdecd 100644 --- a/remoting/host/client_session.cc +++ b/remoting/host/client_session.cc @@ -169,6 +169,7 @@ void ClientSession::OnConnectionClosed( DCHECK(CalledOnValidThread()); DCHECK_EQ(connection_.get(), connection); + // If the client never authenticated then the session failed. if (!auth_input_filter_.enabled()) event_handler_->OnSessionAuthenticationFailed(this); @@ -181,6 +182,19 @@ void ClientSession::OnConnectionClosed( // Ensure that any pressed keys or buttons are released. input_tracker_.ReleaseAll(); + // Stop components access the client, audio or video stubs, which are no + // longer valid once ConnectionToClient calls OnConnectionClosed(). + if (audio_scheduler_.get()) { + audio_scheduler_->Stop(base::Bind(&ClientSession::OnRecorderStopped, this)); + audio_scheduler_ = NULL; + } + if (video_scheduler_.get()) { + video_scheduler_->Stop(base::Bind(&ClientSession::OnRecorderStopped, this)); + video_scheduler_ = NULL; + } + client_clipboard_factory_.InvalidateWeakPtrs(); + + // Notify the ChromotingHost that this client is disconnected. // TODO(sergeyu): Log failure reason? event_handler_->OnSessionClosed(this); } @@ -210,29 +224,26 @@ void ClientSession::Disconnect() { DCHECK(connection_.get()); max_duration_timer_.Stop(); + // This triggers OnConnectionClosed(), and the session may be destroyed // as the result, so this call must be the last in this method. connection_->Disconnect(); } -void ClientSession::Stop(const base::Closure& done_task) { +void ClientSession::Stop(const base::Closure& stopped_task) { DCHECK(CalledOnValidThread()); - DCHECK(done_task_.is_null()); - - done_task_ = done_task; - if (audio_scheduler_.get()) { - audio_scheduler_->Stop(base::Bind(&ClientSession::OnRecorderStopped, this)); - audio_scheduler_ = NULL; - } + DCHECK(stopped_task_.is_null()); + DCHECK(!stopped_task.is_null()); + DCHECK(audio_scheduler_.get() == NULL); + DCHECK(video_scheduler_.get() == NULL); - if (video_scheduler_.get()) { - video_scheduler_->Stop(base::Bind(&ClientSession::OnRecorderStopped, this)); - video_scheduler_ = NULL; - } + stopped_task_ = stopped_task; - if (!active_recorders_) { + if (active_recorders_ == 0) { + // |stopped_task_| may tear down the signalling layer, so tear-down + // |connection_| before invoking it. connection_.reset(); - done_task_.Run(); + stopped_task_.Run(); } } @@ -252,7 +263,7 @@ void ClientSession::SetDisableInputs(bool disable_inputs) { } ClientSession::~ClientSession() { - DCHECK(!active_recorders_); + DCHECK_EQ(active_recorders_, 0); DCHECK(audio_scheduler_.get() == NULL); DCHECK(video_scheduler_.get() == NULL); } @@ -273,14 +284,15 @@ void ClientSession::OnRecorderStopped() { return; } - DCHECK(!done_task_.is_null()); - --active_recorders_; DCHECK_GE(active_recorders_, 0); - if (!active_recorders_) { + DCHECK(!stopped_task_.is_null()); + if (active_recorders_ == 0) { + // |stopped_task_| may result in the signalling layer being torn down, so + // tear down the ConnectionToClient first. connection_.reset(); - done_task_.Run(); + stopped_task_.Run(); } } |