summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--remoting/host/chromoting_host.cc32
-rw-r--r--remoting/host/chromoting_host.h4
-rw-r--r--remoting/host/chromoting_host_unittest.cc23
-rw-r--r--remoting/host/client_session.cc33
-rw-r--r--remoting/host/client_session.h18
-rw-r--r--remoting/host/client_session_unittest.cc12
6 files changed, 46 insertions, 76 deletions
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index fc642e2..3b66a96 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -4,6 +4,8 @@
#include "remoting/host/chromoting_host.h"
+#include <algorithm>
+
#include "base/bind.h"
#include "base/callback.h"
#include "base/logging.h"
@@ -184,18 +186,17 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
login_backoff_.Reset();
- // Disconnect all other clients.
- // Iterate over a copy of the list of clients, to avoid mutating the list
- // while iterating over it.
- ClientList clients_copy(clients_);
- for (ClientList::const_iterator other_client = clients_copy.begin();
- other_client != clients_copy.end(); ++other_client) {
- if (other_client->get() != client) {
- (*other_client)->Disconnect();
- }
+ // Disconnect all other clients. |it| should be advanced before Disconnect()
+ // is called to avoid it becoming invalid when the client is removed from
+ // the list.
+ ClientList::iterator it = clients_.begin();
+ while (it != clients_.end()) {
+ ClientSession* other_client = *it++;
+ if (other_client != client)
+ other_client->Disconnect();
}
- // Disconnects above must have destroyed all other clients and |recorder_|.
+ // Disconnects above must have destroyed all other clients.
DCHECK_EQ(clients_.size(), 1U);
// Notify observers that there is at least one authenticated client.
@@ -232,12 +233,7 @@ void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
void ChromotingHost::OnSessionClosed(ClientSession* client) {
DCHECK(network_task_runner_->BelongsToCurrentThread());
- ClientList::iterator it = clients_.begin();
- for (; it != clients_.end(); ++it) {
- if (it->get() == client) {
- break;
- }
- }
+ ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client);
CHECK(it != clients_.end());
if (client->is_authenticated()) {
@@ -245,8 +241,8 @@ void ChromotingHost::OnSessionClosed(ClientSession* client) {
OnClientDisconnected(client->client_jid()));
}
- client->Stop();
clients_.erase(it);
+ delete client;
if (state_ == kStopping && clients_.empty())
ShutdownFinish();
@@ -311,7 +307,7 @@ void ChromotingHost::OnIncomingSession(
// Create a client object.
scoped_ptr<protocol::ConnectionToClient> connection(
new protocol::ConnectionToClient(session));
- scoped_refptr<ClientSession> client = new ClientSession(
+ ClientSession* client = new ClientSession(
this,
audio_task_runner_,
input_task_runner_,
diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h
index 9aa9afd..6a574a8 100644
--- a/remoting/host/chromoting_host.h
+++ b/remoting/host/chromoting_host.h
@@ -5,8 +5,8 @@
#ifndef REMOTING_HOST_CHROMOTING_HOST_H_
#define REMOTING_HOST_CHROMOTING_HOST_H_
+#include <list>
#include <string>
-#include <vector>
#include "base/memory/scoped_ptr.h"
#include "base/memory/ref_counted.h"
@@ -157,7 +157,7 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
friend class base::RefCountedThreadSafe<ChromotingHost>;
friend class ChromotingHostTest;
- typedef std::vector<scoped_refptr<ClientSession> > ClientList;
+ typedef std::list<ClientSession*> ClientList;
enum State {
kInitial,
diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index 80b3fa0..f53caf8 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -237,7 +237,7 @@ class ChromotingHostTest : public testing::Test {
((connection_index == 0) ? owned_connection1_ : owned_connection2_).
PassAs<protocol::ConnectionToClient>();
protocol::ConnectionToClient* connection_ptr = connection.get();
- scoped_refptr<ClientSession> client = new ClientSession(
+ scoped_ptr<ClientSession> client(new ClientSession(
host_.get(),
ui_task_runner_, // Audio
ui_task_runner_, // Input
@@ -247,31 +247,33 @@ class ChromotingHostTest : public testing::Test {
ui_task_runner_, // UI
connection.Pass(),
desktop_environment_factory_.get(),
- base::TimeDelta());
- connection_ptr->set_host_stub(client);
+ base::TimeDelta()));
+
+ ClientSession* client_raw = client.get();
+ connection_ptr->set_host_stub(client_raw);
ui_task_runner_->PostTask(
FROM_HERE, base::Bind(&ChromotingHostTest::AddClientToHost,
- host_, client));
+ host_, base::Passed(&client)));
if (authenticate) {
ui_task_runner_->PostTask(
FROM_HERE, base::Bind(&ClientSession::OnConnectionAuthenticated,
- client, connection_ptr));
+ base::Unretained(client_raw), connection_ptr));
if (!reject) {
ui_task_runner_->PostTask(
FROM_HERE,
base::Bind(&ClientSession::OnConnectionChannelsConnected,
- client, connection_ptr));
+ base::Unretained(client_raw), connection_ptr));
}
} else {
ui_task_runner_->PostTask(
FROM_HERE, base::Bind(&ClientSession::OnConnectionClosed,
- client, connection_ptr,
+ base::Unretained(client_raw), connection_ptr,
protocol::AUTHENTICATION_FAILED));
}
- get_client(connection_index) = client;
+ get_client(connection_index) = client_raw;
}
virtual void TearDown() OVERRIDE {
@@ -360,8 +362,9 @@ class ChromotingHostTest : public testing::Test {
}
static void AddClientToHost(scoped_refptr<ChromotingHost> host,
- ClientSession* session) {
- host->clients_.push_back(session);
+ scoped_ptr<ClientSession> client) {
+ // |host| is responsible for deleting |client| from now on.
+ host->clients_.push_back(client.release());
}
void ShutdownHost() {
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index a150066..11a380e 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -89,6 +89,16 @@ ClientSession::ClientSession(
#endif // defined(OS_WIN)
}
+ClientSession::~ClientSession() {
+ DCHECK(CalledOnValidThread());
+ DCHECK(!audio_scheduler_);
+ DCHECK(!event_executor_);
+ DCHECK(!session_controller_);
+ DCHECK(!video_scheduler_);
+
+ connection_.reset();
+}
+
void ClientSession::NotifyClientResolution(
const protocol::ClientResolution& resolution) {
if (resolution.has_dips_width() && resolution.has_dips_height()) {
@@ -267,16 +277,6 @@ void ClientSession::Disconnect() {
connection_->Disconnect();
}
-void ClientSession::Stop() {
- DCHECK(CalledOnValidThread());
- DCHECK(!audio_scheduler_);
- DCHECK(!event_executor_);
- DCHECK(!session_controller_);
- DCHECK(!video_scheduler_);
-
- connection_.reset();
-}
-
void ClientSession::LocalMouseMoved(const SkIPoint& mouse_pos) {
DCHECK(CalledOnValidThread());
remote_input_filter_.LocalMouseMoved(mouse_pos);
@@ -292,14 +292,6 @@ void ClientSession::SetDisableInputs(bool disable_inputs) {
disable_clipboard_filter_.set_enabled(!disable_inputs);
}
-ClientSession::~ClientSession() {
- DCHECK(CalledOnValidThread());
- DCHECK(!audio_scheduler_);
- DCHECK(!event_executor_);
- DCHECK(!session_controller_);
- DCHECK(!video_scheduler_);
-}
-
scoped_ptr<protocol::ClipboardStub> ClientSession::CreateClipboardProxy() {
DCHECK(CalledOnValidThread());
@@ -342,9 +334,4 @@ scoped_ptr<AudioEncoder> ClientSession::CreateAudioEncoder(
return scoped_ptr<AudioEncoder>(NULL);
}
-// static
-void ClientSessionTraits::Destruct(const ClientSession* client) {
- client->network_task_runner_->DeleteSoon(FROM_HERE, client);
-}
-
} // namespace remoting
diff --git a/remoting/host/client_session.h b/remoting/host/client_session.h
index b6d3a6a..f0d1e51 100644
--- a/remoting/host/client_session.h
+++ b/remoting/host/client_session.h
@@ -34,7 +34,6 @@ namespace remoting {
class AudioEncoder;
class AudioScheduler;
-struct ClientSessionTraits;
class DesktopEnvironment;
class DesktopEnvironmentFactory;
class EventExecutor;
@@ -45,8 +44,7 @@ class VideoScheduler;
// A ClientSession keeps a reference to a connection to a client, and maintains
// per-client state.
class ClientSession
- : public base::RefCountedThreadSafe<ClientSession, ClientSessionTraits>,
- public protocol::HostStub,
+ : public protocol::HostStub,
public protocol::ConnectionToClient::EventHandler,
public base::NonThreadSafe {
public:
@@ -96,6 +94,7 @@ class ClientSession
scoped_ptr<protocol::ConnectionToClient> connection,
DesktopEnvironmentFactory* desktop_environment_factory,
const base::TimeDelta& max_duration);
+ virtual ~ClientSession();
// protocol::HostStub interface.
virtual void NotifyClientResolution(
@@ -124,10 +123,6 @@ class ClientSession
// method returns.
void Disconnect();
- // Stops the ClientSession. The caller can safely release its reference to
- // the client session once Stop() returns.
- void Stop();
-
protocol::ConnectionToClient* connection() const {
return connection_.get();
}
@@ -146,10 +141,6 @@ class ClientSession
void SetDisableInputs(bool disable_inputs);
private:
- friend class base::DeleteHelper<ClientSession>;
- friend struct ClientSessionTraits;
- virtual ~ClientSession();
-
// Creates a proxy for sending clipboard events to the client.
scoped_ptr<protocol::ClipboardStub> CreateClipboardProxy();
@@ -230,11 +221,6 @@ class ClientSession
DISALLOW_COPY_AND_ASSIGN(ClientSession);
};
-// Destroys |ClienSession| instances on the network thread.
-struct ClientSessionTraits {
- static void Destruct(const ClientSession* client);
-};
-
} // namespace remoting
#endif // REMOTING_HOST_CLIENT_SESSION_H_
diff --git a/remoting/host/client_session_unittest.cc b/remoting/host/client_session_unittest.cc
index a7893a4..33277f5 100644
--- a/remoting/host/client_session_unittest.cc
+++ b/remoting/host/client_session_unittest.cc
@@ -100,7 +100,7 @@ class ClientSessionTest : public testing::Test {
MessageLoop message_loop_;
// ClientSession instance under test.
- scoped_refptr<ClientSession> client_session_;
+ scoped_ptr<ClientSession> client_session_;
// ClientSession::EventHandler mock for use in tests.
MockClientSessionEventHandler session_event_handler_;
@@ -160,7 +160,7 @@ void ClientSessionTest::SetUp() {
EXPECT_CALL(*connection, Disconnect());
connection_ = connection.get();
- client_session_ = new ClientSession(
+ client_session_.reset(new ClientSession(
&session_event_handler_,
ui_task_runner, // Audio thread.
ui_task_runner, // Input thread.
@@ -170,12 +170,12 @@ void ClientSessionTest::SetUp() {
ui_task_runner, // UI thread.
connection.PassAs<protocol::ConnectionToClient>(),
desktop_environment_factory_.get(),
- base::TimeDelta());
+ base::TimeDelta()));
}
void ClientSessionTest::TearDown() {
// Verify that the client session has been stopped.
- EXPECT_TRUE(client_session_.get() == NULL);
+ EXPECT_TRUE(!client_session_);
}
void ClientSessionTest::DisconnectClientSession() {
@@ -186,9 +186,7 @@ void ClientSessionTest::DisconnectClientSession() {
}
void ClientSessionTest::StopClientSession() {
- // MockClientSessionEventHandler won't trigger Stop, so fake it.
- client_session_->Stop();
- client_session_ = NULL;
+ client_session_.reset();
desktop_environment_factory_.reset();
}