Access ChromotingHost::clients_ only on network thread.

remoting/host/chromoting_host.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/chromoting_host.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
@@ skipping 36 matching lines @@
 ChromotingHost::ChromotingHost(ChromotingHostContext* context,
                                MutableHostConfig* config,
                                DesktopEnvironment* environment,
                                AccessVerifier* access_verifier,
                                bool allow_nat_traversal)
     : context_(context),
       desktop_environment_(environment),
       config_(config),
       access_verifier_(access_verifier),
       allow_nat_traversal_(allow_nat_traversal),
+      stopping_recorders_(0),
       state_(kInitial),
-      stopping_recorders_(0),
       protocol_config_(protocol::CandidateSessionConfig::CreateDefault()),
       is_curtained_(false),
       is_it2me_(false) {
   DCHECK(desktop_environment_);
   desktop_environment_->set_host(this);
 }
 
 ChromotingHost::~ChromotingHost() {
 }
 
 void ChromotingHost::Start() {
   if (!context_->network_message_loop()->BelongsToCurrentThread()) {
     context_->network_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::Start, this));
     return;
   }
 
   LOG(INFO) << "Starting host";
   DCHECK(!signal_strategy_.get());
   DCHECK(access_verifier_.get());
 
   // Make sure this object is not started.
-  {
-    base::AutoLock auto_lock(lock_);
-    if (state_ != kInitial)
-      return;
-    state_ = kStarted;
-  }
+  if (state_ != kInitial)
+    return;
+  state_ = kStarted;
 
   // Use an XMPP connection to the Talk network for session signalling.
   std::string xmpp_login;
   std::string xmpp_auth_token;
   std::string xmpp_auth_service;
   if (!config_->GetString(kXmppLoginConfigPath, &xmpp_login) ||
       !config_->GetString(kXmppAuthTokenConfigPath, &xmpp_auth_token) ||
       !config_->GetString(kXmppAuthServiceConfigPath, &xmpp_auth_service)) {
     LOG(ERROR) << "XMPP credentials are not defined in the config.";
     return;
   }
 
   signal_strategy_.reset(
       new XmppSignalStrategy(context_->jingle_thread(), xmpp_login,
                              xmpp_auth_token,
                              xmpp_auth_service));
   signal_strategy_->Init(this);
 }
 
 // This method is called when we need to destroy the host process.
 void ChromotingHost::Shutdown(Task* shutdown_task) {
-  if (MessageLoop::current() != context_->main_message_loop()) {
-    context_->main_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&ChromotingHost::Shutdown, this, shutdown_task));
+  if (!context_->network_message_loop()->BelongsToCurrentThread()) {
+    context_->network_message_loop()->PostTask(
+        FROM_HERE, base::Bind(&ChromotingHost::Shutdown, this, shutdown_task));
     return;
   }
 
   // No-op if this object is not started yet.
-  {
-    base::AutoLock auto_lock(lock_);
-    if (state_ == kInitial || state_ == kStopped) {
+  if (state_ == kInitial || state_ == kStopped) {
       // Nothing to do if we are not started.
-      state_ = kStopped;
-      context_->main_message_loop()->PostTask(FROM_HERE, shutdown_task);
-      return;
-    }
-    if (shutdown_task)
-      shutdown_tasks_.push_back(shutdown_task);
-    if (state_ == kStopping)
-      return;
-    state_ = kStopping;
+    state_ = kStopped;
+    context_->network_message_loop()->PostTask(FROM_HERE, shutdown_task);
+    return;
   }
+  if (shutdown_task)
+    shutdown_tasks_.push_back(shutdown_task);
+  if (state_ == kStopping)
+    return;
+  state_ = kStopping;
 
   // Disconnect all of the clients, implicitly stopping the ScreenRecorder.
   while (!clients_.empty()) {
     scoped_refptr<ClientSession> client = clients_.front();
     client->Disconnect();
-    OnClientDisconnected(client);
+    OnSessionClosed(client);
   }
 
-  ShutdownNetwork();
+  // Stop session manager.
+  if (session_manager_.get()) {
+    session_manager_->Close();
+    // It may not be safe to delete |session_manager_| here becase
+    // this method may be invoked in resonse to a libjingle event and

[Wez, 2011/11/09 23:18:45]

typo: resonse

[Sergey Ulanov, 2011/11/10 21:06:14]

Done.

+    // libjingle's sigslot doesn't handle it properly, so postpone
+    // the deletion.
+    context_->network_message_loop()->DeleteSoon(
+        FROM_HERE, session_manager_.release());
+  }
+
+  // Stop XMPP connection synchronously.
+  if (signal_strategy_.get()) {
+    signal_strategy_->Close();
+    signal_strategy_.reset();
+
+    for (StatusObserverList::iterator it = status_observers_.begin();
+         it != status_observers_.end(); ++it) {
+      (*it)->OnSignallingDisconnected();
+    }
+  }
+
+  if (recorder_.get()) {
+    StopScreenRecorder();
+  } else {
+    ShutdownFinish();
+  }
 }
 
 void ChromotingHost::AddStatusObserver(HostStatusObserver* observer) {
   DCHECK_EQ(state_, kInitial);
   status_observers_.push_back(observer);
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // protocol::ClientSession::EventHandler implementation.
 void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
-  protocol::Session* session = client->connection()->session();
-  context_->main_message_loop()->PostTask(
-      FROM_HERE, base::Bind(&ChromotingHost::AddAuthenticatedClient,
-                            this, make_scoped_refptr(client),
-                            session->config(), session->jid()));
+
+  // 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) != client) {
+      (*other_client)->Disconnect();
+      OnSessionClosed(*other_client);
+    }
+  }
+
+  // Those disconnections should have killed the screen recorder.
+  CHECK(recorder_.get() == NULL);
+
+  // Create a new RecordSession if there was none.
+  if (!recorder_.get()) {
+    // Then we create a ScreenRecorder passing the message loops that
+    // it should run on.
+    Encoder* encoder = CreateEncoder(client->connection()->session()->config());
+
+    recorder_ = new ScreenRecorder(context_->main_message_loop(),
+                                   context_->encode_message_loop(),
+                                   context_->network_message_loop(),
+                                   desktop_environment_->capturer(),
+                                   encoder);
+  }
+
+  // Immediately add the connection and start the session.
+  recorder_->AddConnection(client->connection());
+  recorder_->Start();
+
+  // Notify observers that there is at least one authenticated client.
+  const std::string& jid = client->connection()->session()->jid();
+  for (StatusObserverList::iterator it = status_observers_.begin();
+       it != status_observers_.end(); ++it) {
+    (*it)->OnClientAuthenticated(jid);
+  }
+  // TODO(jamiewalch): Tidy up actions to be taken on connect/disconnect,
+  // including closing the connection on failure of a critical operation.
+  EnableCurtainMode(true);
+
+  std::string username = jid.substr(0, jid.find('/'));
+  desktop_environment_->OnConnect(username);
 }
 
 void ChromotingHost::OnSessionClosed(ClientSession* client) {
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
-  VLOG(1) << "Connection to client closed.";
-  context_->main_message_loop()->PostTask(
-      FROM_HERE, base::Bind(&ChromotingHost::OnClientDisconnected, this,
-                            make_scoped_refptr(client)));
+  scoped_refptr<ClientSession> client_ref = client;
+
+  ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client);
+  CHECK(it != clients_.end());
+  clients_.erase(it);
+
+  if (recorder_.get()) {
+    recorder_->RemoveConnection(client->connection());
+  }
+
+  for (StatusObserverList::iterator it = status_observers_.begin();
+       it != status_observers_.end(); ++it) {
+    (*it)->OnClientDisconnected(client->client_jid());
+  }
+
+  if (AuthenticatedClientsCount() == 0) {
+    if (recorder_.get()) {
+      // Stop the recorder if there are no more clients.
+      StopScreenRecorder();
+    }
+
+    // Disable the "curtain" if there are no more active clients.
+    EnableCurtainMode(false);
+    desktop_environment_->OnLastDisconnect();
+  }
 }
 
 void ChromotingHost::OnSessionSequenceNumber(ClientSession* session,
                                              int64 sequence_number) {
-  // Update the sequence number in ScreenRecorder.
-  if (MessageLoop::current() != context_->main_message_loop()) {
-    context_->main_message_loop()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::OnSessionSequenceNumber, this,
-                              make_scoped_refptr(session), sequence_number));
-    return;
-  }
-
+  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
   if (recorder_.get())
     recorder_->UpdateSequenceNumber(sequence_number);
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // SignalStrategy::StatusObserver implementations
 void ChromotingHost::OnStateChange(
     SignalStrategy::StatusObserver::State state) {
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
@@ skipping 39 matching lines @@
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
   // Don't need to do anything here, just wait for incoming
   // connections.
 }
 
 void ChromotingHost::OnIncomingSession(
       protocol::Session* session,
       protocol::SessionManager::IncomingSessionResponse* response) {
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
-  base::AutoLock auto_lock(lock_);
   if (state_ != kStarted) {
     *response = protocol::SessionManager::DECLINE;
     return;
   }
 
   // Check that the client has access to the host.
   if (!access_verifier_->VerifyPermissions(session->jid(),
                                            session->initiator_token())) {
     *response = protocol::SessionManager::DECLINE;
 
@@ skipping 38 matching lines @@
 
   *response = protocol::SessionManager::ACCEPT;
 
   LOG(INFO) << "Client connected: " << session->jid();
 
   // Create a client object.
   scoped_refptr<protocol::ConnectionToClient> connection =
       new protocol::ConnectionToClient(context_->network_message_loop(),
                                        session);
   ClientSession* client = new ClientSession(
-      this, connection,
-      desktop_environment_->event_executor(),
+      this, connection, desktop_environment_->event_executor(),
       desktop_environment_->capturer());
-
   clients_.push_back(client);
 }
 
 void ChromotingHost::set_protocol_config(
     protocol::CandidateSessionConfig* config) {
-  DCHECK(config_.get());
+  DCHECK(config);
   DCHECK_EQ(state_, kInitial);
   protocol_config_.reset(config);
 }
 
 void ChromotingHost::LocalMouseMoved(const SkIPoint& new_pos) {
   if (!context_->network_message_loop()->BelongsToCurrentThread()) {
     context_->network_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::LocalMouseMoved, this, new_pos));
     return;
   }
+
   ClientList::iterator client;
   for (client = clients_.begin(); client != clients_.end(); ++client) {
     client->get()->LocalMouseMoved(new_pos);
   }
 }
 
 void ChromotingHost::PauseSession(bool pause) {
-  if (context_->main_message_loop() != MessageLoop::current()) {
-    context_->main_message_loop()->PostTask(
+  if (!context_->network_message_loop()->BelongsToCurrentThread()) {
+    context_->network_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(this,
                           &ChromotingHost::PauseSession,
                           pause));
     return;
   }
+
   ClientList::iterator client;
   for (client = clients_.begin(); client != clients_.end(); ++client) {
     client->get()->set_awaiting_continue_approval(pause);
   }
   desktop_environment_->OnPause(pause);
 }
 
 void ChromotingHost::SetUiStrings(const UiStrings& ui_strings) {
   DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
   DCHECK_EQ(state_, kInitial);
 
   ui_strings_ = ui_strings;
 }
 
-void ChromotingHost::OnClientDisconnected(ClientSession* client) {
-  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
-
-  scoped_refptr<ClientSession> client_ref = client;
-
-  ClientList::iterator it;
-  for (it = clients_.begin(); it != clients_.end(); ++it) {
-    if (it->get() == client)
-      break;
-  }
-  clients_.erase(it);
-
-  if (recorder_.get()) {
-    recorder_->RemoveConnection(client->connection());
-  }
-
-  for (StatusObserverList::iterator it = status_observers_.begin();
-       it != status_observers_.end(); ++it) {
-    (*it)->OnClientDisconnected(client->client_jid());
-  }
-
-  if (AuthenticatedClientsCount() == 0) {
-    if (recorder_.get()) {
-      // Stop the recorder if there are no more clients.
-      StopScreenRecorder();
-    }
-
-    // Disable the "curtain" if there are no more active clients.
-    EnableCurtainMode(false);
-    desktop_environment_->OnLastDisconnect();
-  }
-}
-
 // TODO(sergeyu): Move this to SessionManager?
 Encoder* ChromotingHost::CreateEncoder(const protocol::SessionConfig& config) {
   const protocol::ChannelConfig& video_config = config.video_config();
 
   if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
     return EncoderRowBased::CreateVerbatimEncoder();
   } else if (video_config.codec == protocol::ChannelConfig::CODEC_ZIP) {
     return EncoderRowBased::CreateZlibEncoder();
   } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) {
     return new remoting::EncoderVp8();
   }
 
   return NULL;
 }
 
 std::string ChromotingHost::GenerateHostAuthToken(
     const std::string& encoded_client_token) {
   // TODO(ajwong): Return the signature of this instead.
   return encoded_client_token;
 }
 
 int ChromotingHost::AuthenticatedClientsCount() const {
+  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
+
   int authenticated_clients = 0;
   for (ClientList::const_iterator it = clients_.begin(); it != clients_.end();
        ++it) {
     if (it->get()->authenticated())
       ++authenticated_clients;
   }
   return authenticated_clients;
 }
 
 void ChromotingHost::EnableCurtainMode(bool enable) {
   // TODO(jamiewalch): This will need to be more sophisticated when we think
   // about proper crash recovery and daemon mode.
   // TODO(wez): CurtainMode shouldn't be driven directly by ChromotingHost.
   if (is_it2me_ || enable == is_curtained_)
     return;
   desktop_environment_->curtain()->EnableCurtainMode(enable);
   is_curtained_ = enable;
 }
 
-void ChromotingHost::AddAuthenticatedClient(
-    ClientSession* client,
-    const protocol::SessionConfig& config,
-    const std::string& jid) {
-  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
-
-  // 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) != client) {
-      (*other_client)->Disconnect();
-      OnClientDisconnected(*other_client);
-    }
-  }
-
-  // Those disconnections should have killed the screen recorder.
-  CHECK(recorder_.get() == NULL);
-
-  // Create a new RecordSession if there was none.
-  if (!recorder_.get()) {
-    // Then we create a ScreenRecorder passing the message loops that
-    // it should run on.
-    Encoder* encoder = CreateEncoder(config);
-
-    recorder_ = new ScreenRecorder(context_->main_message_loop(),
-                                   context_->encode_message_loop(),
-                                   context_->network_message_loop(),
-                                   desktop_environment_->capturer(),
-                                   encoder);
-  }
-
-  // Immediately add the connection and start the session.
-  recorder_->AddConnection(client->connection());
-  recorder_->Start();
-  // Notify observers that there is at least one authenticated client.
-  for (StatusObserverList::iterator it = status_observers_.begin();
-       it != status_observers_.end(); ++it) {
-    (*it)->OnClientAuthenticated(jid);
-  }
-  // TODO(jamiewalch): Tidy up actions to be taken on connect/disconnect,
-  // including closing the connection on failure of a critical operation.
-  EnableCurtainMode(true);
-  if (is_it2me_) {
-    std::string username = jid;
-    size_t pos = username.find('/');
-    if (pos != std::string::npos)
-      username.replace(pos, std::string::npos, "");
-    desktop_environment_->OnConnect(username);
-  }
-}
-
 void ChromotingHost::StopScreenRecorder() {
-  DCHECK(MessageLoop::current() == context_->main_message_loop());
+  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
   DCHECK(recorder_.get());
 
   ++stopping_recorders_;
-  recorder_->Stop(base::Bind(&ChromotingHost::OnScreenRecorderStopped, this));
+  scoped_refptr<ScreenRecorder> recorder = recorder_;
   recorder_ = NULL;
+  recorder->Stop(base::Bind(&ChromotingHost::OnScreenRecorderStopped, this));
 }
 
 void ChromotingHost::OnScreenRecorderStopped() {
-  if (MessageLoop::current() != context_->main_message_loop()) {
-    context_->main_message_loop()->PostTask(
+  if (!context_->network_message_loop()->BelongsToCurrentThread()) {
+    context_->network_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::OnScreenRecorderStopped, this));
     return;
   }
 
   --stopping_recorders_;
   DCHECK_GE(stopping_recorders_, 0);
 
-  bool stopping;
-  {
-    base::AutoLock auto_lock(lock_);
-    stopping = state_ == kStopping;
-  }
-
-  if (!stopping_recorders_ && stopping)
+  if (!stopping_recorders_ && state_ == kStopping)
     ShutdownFinish();
 }
 
-void ChromotingHost::ShutdownNetwork() {
-  if (!context_->network_message_loop()->BelongsToCurrentThread()) {
-    context_->network_message_loop()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::ShutdownNetwork, this));
-    return;
-  }
+void ChromotingHost::ShutdownFinish() {
+  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
-  // Stop chromotocol session manager.
-  if (session_manager_.get()) {
-    session_manager_->Close();
-    session_manager_.reset();
-  }
-
-  // Stop XMPP connection.
-  if (signal_strategy_.get()) {
-    signal_strategy_->Close();
-    signal_strategy_.reset();
-
-    for (StatusObserverList::iterator it = status_observers_.begin();
-         it != status_observers_.end(); ++it) {
-      (*it)->OnSignallingDisconnected();
-    }
-  }
-
-  ShutdownRecorder();
-}
-
-void ChromotingHost::ShutdownRecorder() {
-  if (MessageLoop::current() != context_->main_message_loop()) {
-    context_->main_message_loop()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::ShutdownRecorder, this));
-    return;
-  }
-
-  if (recorder_.get()) {
-    StopScreenRecorder();
-  } else if (!stopping_recorders_) {
-    ShutdownFinish();
-  }
-}
-
-void ChromotingHost::ShutdownFinish() {
-  if (MessageLoop::current() != context_->main_message_loop()) {
-    context_->main_message_loop()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::ShutdownFinish, this));
-    return;
-  }
-
-  {
-    base::AutoLock auto_lock(lock_);
-    state_ = kStopped;
-  }
+  state_ = kStopped;
 
   // Keep reference to |this|, so that we don't get destroyed while
   // sending notifications.
   scoped_refptr<ChromotingHost> self(this);
 
   // Notify observers.
   for (StatusObserverList::iterator it = status_observers_.begin();
        it != status_observers_.end(); ++it) {
     (*it)->OnShutdown();
   }
 
   for (std::vector<Task*>::iterator it = shutdown_tasks_.begin();
        it != shutdown_tasks_.end(); ++it) {
     (*it)->Run();
     delete *it;
   }
   shutdown_tasks_.clear();
 }
 
 }  // namespace remoting