Made the ChromotingHost class not ref-counted.

ChromotingHost becomes a non thread-safe class should should live on the network thread. The CL removes ChromotingHost::Shutdown() allowing ChromotingHost to be destroyed synchronously closing all existing connection. Review URL: https://chromiumcodereview.appspot.com/14348042 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197458 0039d316-1c4b-4281-b951-d872f2087c98
author: alexeypa@chromium.org <alexeypa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2013-04-30 20:46:16 +0000
committer: alexeypa@chromium.org <alexeypa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2013-04-30 20:46:16 +0000
commit: 22f3d247091711bf7844c9e865d99dae3ff7dd5a (patch)
tree: 1b9e1653d1eef758eb341870117cd3268d1044d7
parent: 31c36e98a7e1e60a7575457dc5379d05eaf88898 (diff)
download: chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.zip
chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.tar.gz
chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.tar.bz2
5 files changed, 125 insertions, 230 deletions
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 0d3810b..0a365e1 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -76,14 +76,14 @@ ChromotingHost::ChromotingHost(
       network_task_runner_(network_task_runner),
       ui_task_runner_(ui_task_runner),
       signal_strategy_(signal_strategy),
-      state_(kInitial),
+      started_(false),
       protocol_config_(protocol::CandidateSessionConfig::CreateDefault()),
       login_backoff_(&kDefaultBackoffPolicy),
       authenticating_client_(false),
       reject_authenticating_client_(false),
       weak_factory_(this) {
-  DCHECK(signal_strategy);
   DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(signal_strategy);
 
   if (!desktop_environment_factory_->SupportsAudioCapture()) {
     protocol::CandidateSessionConfig::DisableAudioChannel(
@@ -92,74 +92,41 @@ ChromotingHost::ChromotingHost(
 }
 
 ChromotingHost::~ChromotingHost() {
-  DCHECK(clients_.empty());
+  DCHECK(CalledOnValidThread());
+
+  // Disconnect all of the clients.
+  while (!clients_.empty()) {
+    clients_.front()->DisconnectSession();
+  }
+
+  // Destroy the session manager to make sure that |signal_strategy_| does not
+  // have any listeners registered.
+  session_manager_.reset();
+
+  // Notify observers.
+  if (started_)
+    FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnShutdown());
 }
 
 void ChromotingHost::Start(const std::string& xmpp_login) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
+  DCHECK(!started_);
 
   LOG(INFO) << "Starting host";
-
-  // Make sure this object is not started.
-  if (state_ != kInitial)
-    return;
-  state_ = kStarted;
-
-  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
-                    OnStart(xmpp_login));
+  started_ = true;
+  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnStart(xmpp_login));
 
   // Start the SessionManager, supplying this ChromotingHost as the listener.
   session_manager_->Init(signal_strategy_, this);
 }
 
-// This method is called when we need to destroy the host process.
-void ChromotingHost::Shutdown(const base::Closure& shutdown_task) {
-  if (!network_task_runner_->BelongsToCurrentThread()) {
-    network_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::Shutdown, this, shutdown_task));
-    return;
-  }
-
-  switch (state_) {
-    case kInitial:
-    case kStopped:
-      // Nothing to do if we are not started.
-      state_ = kStopped;
-      if (!shutdown_task.is_null())
-        network_task_runner_->PostTask(FROM_HERE, shutdown_task);
-      break;
-
-    case kStopping:
-      // We are already stopping. Just save the task.
-      if (!shutdown_task.is_null())
-        shutdown_tasks_.push_back(shutdown_task);
-      break;
-
-    case kStarted:
-      if (!shutdown_task.is_null())
-        shutdown_tasks_.push_back(shutdown_task);
-      state_ = kStopping;
-
-      // Disconnect all of the clients.
-      while (!clients_.empty()) {
-        clients_.front()->DisconnectSession();
-      }
-
-      // Run the remaining shutdown tasks.
-      if (state_ == kStopping)
-        ShutdownFinish();
-
-      break;
-  }
-}
-
 void ChromotingHost::AddStatusObserver(HostStatusObserver* observer) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   status_observers_.AddObserver(observer);
 }
 
 void ChromotingHost::RemoveStatusObserver(HostStatusObserver* observer) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   status_observers_.RemoveObserver(observer);
 }
 
@@ -170,7 +137,7 @@ void ChromotingHost::RejectAuthenticatingClient() {
 
 void ChromotingHost::SetAuthenticatorFactory(
     scoped_ptr<protocol::AuthenticatorFactory> authenticator_factory) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   session_manager_->set_authenticator_factory(authenticator_factory.Pass());
 }
 
@@ -182,7 +149,7 @@ void ChromotingHost::SetMaximumSessionDuration(
 ////////////////////////////////////////////////////////////////////////////
 // protocol::ClientSession::EventHandler implementation.
 void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   login_backoff_.Reset();
 
@@ -215,7 +182,7 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   // Notify observers.
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
@@ -223,7 +190,7 @@ void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   // Notify observers.
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
@@ -231,7 +198,7 @@ void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionClosed(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client);
   CHECK(it != clients_.end());
@@ -243,28 +210,25 @@ void ChromotingHost::OnSessionClosed(ClientSession* client) {
 
   clients_.erase(it);
   delete client;
-
-  if (state_ == kStopping && clients_.empty())
-    ShutdownFinish();
 }
 
 void ChromotingHost::OnSessionSequenceNumber(ClientSession* session,
                                              int64 sequence_number) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 }
 
 void ChromotingHost::OnSessionRouteChange(
     ClientSession* session,
     const std::string& channel_name,
     const protocol::TransportRoute& route) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
                     OnClientRouteChange(session->client_jid(), channel_name,
                                         route));
 }
 
 void ChromotingHost::OnSessionManagerReady() {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   // Don't need to do anything here, just wait for incoming
   // connections.
 }
@@ -272,9 +236,9 @@ void ChromotingHost::OnSessionManagerReady() {
 void ChromotingHost::OnIncomingSession(
       protocol::Session* session,
       protocol::SessionManager::IncomingSessionResponse* response) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
-  if (state_ != kStarted) {
+  if (!started_) {
     *response = protocol::SessionManager::DECLINE;
     return;
   }
@@ -323,18 +287,14 @@ void ChromotingHost::OnIncomingSession(
 
 void ChromotingHost::set_protocol_config(
     scoped_ptr<protocol::CandidateSessionConfig> config) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   DCHECK(config.get());
-  DCHECK_EQ(state_, kInitial);
+  DCHECK(!started_);
   protocol_config_ = config.Pass();
 }
 
 void ChromotingHost::DisconnectAllClients() {
-  if (!network_task_runner_->BelongsToCurrentThread()) {
-    network_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::DisconnectAllClients, this));
-    return;
-  }
+  DCHECK(CalledOnValidThread());
 
   while (!clients_.empty()) {
     size_t size = clients_.size();
@@ -343,35 +303,4 @@ void ChromotingHost::DisconnectAllClients() {
   }
 }
 
-void ChromotingHost::ShutdownFinish() {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, kStopping);
-
-  state_ = kStopped;
-
-  // Destroy session manager.
-  session_manager_.reset();
-
-  // Clear |desktop_environment_factory_| and |signal_strategy_| to
-  // ensure we don't try to touch them after running shutdown tasks
-  desktop_environment_factory_ = NULL;
-  signal_strategy_ = NULL;
-
-  // Keep reference to |this|, so that we don't get destroyed while
-  // sending notifications.
-  scoped_refptr<ChromotingHost> self(this);
-
-  // Notify observers.
-  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
-                    OnShutdown());
-
-  for (std::vector<base::Closure>::iterator it = shutdown_tasks_.begin();
-       it != shutdown_tasks_.end(); ++it) {
-    it->Run();
-  }
-  shutdown_tasks_.clear();
-
-  weak_factory_.InvalidateWeakPtrs();
-}
-
 }  // namespace remoting
diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h
index b1cd128..35ef373 100644
--- a/remoting/host/chromoting_host.h
+++ b/remoting/host/chromoting_host.h
@@ -12,6 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/threading/non_thread_safe.h"
 #include "base/threading/thread.h"
 #include "net/base/backoff_entry.h"
 #include "remoting/host/client_session.h"
@@ -59,14 +60,13 @@ class DesktopEnvironmentFactory;
 //    all pending tasks to complete. After all of that completed we
 //    return to the idle state. We then go to step (2) if there a new
 //    incoming connection.
-class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
+class ChromotingHost : public base::NonThreadSafe,
                        public ClientSession::EventHandler,
                        public protocol::SessionManager::Listener,
                        public HostStatusMonitor {
  public:
-  // The caller must ensure that |signal_strategy| and
-  // |desktop_environment_factory| remain valid at least until the
-  // |shutdown_task| supplied to Shutdown() has been notified.
+  // Both |signal_strategy| and |desktop_environment_factory| should outlive
+  // this object.
   ChromotingHost(
       SignalStrategy* signal_strategy,
       DesktopEnvironmentFactory* desktop_environment_factory,
@@ -77,8 +77,9 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
       scoped_refptr<base::SingleThreadTaskRunner> video_encode_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+  virtual ~ChromotingHost();
 
-  // Asynchronously start the host process.
+  // Asynchronously starts the host.
   //
   // After this is invoked, the host process will connect to the talk
   // network and start listening for incoming connections.
@@ -86,10 +87,6 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // This method can only be called once during the lifetime of this object.
   void Start(const std::string& xmpp_login);
 
-  // Asynchronously shutdown the host process. |shutdown_task| is
-  // called after shutdown is completed.
-  void Shutdown(const base::Closure& shutdown_task);
-
   // HostStatusMonitor interface.
   virtual void AddStatusObserver(HostStatusObserver* observer) OVERRIDE;
   virtual void RemoveStatusObserver(HostStatusObserver* observer) OVERRIDE;
@@ -134,10 +131,9 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // Sets desired configuration for the protocol. Must be called before Start().
   void set_protocol_config(scoped_ptr<protocol::CandidateSessionConfig> config);
 
-  // Disconnects all active clients. Clients are disconnected
-  // asynchronously when this method is called on a thread other than
-  // the network thread. Potentically this may cause disconnection of
-  // clients that were not connected when this method is called.
+  // Immediately disconnects all active clients. Host-internal components may
+  // shutdown asynchronously, but the caller is guaranteed not to receive
+  // callbacks for disconnected clients after this call returns.
   void DisconnectAllClients();
 
   base::WeakPtr<ChromotingHost> AsWeakPtr() {
@@ -145,23 +141,10 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   }
 
  private:
-  friend class base::RefCountedThreadSafe<ChromotingHost>;
   friend class ChromotingHostTest;
 
   typedef std::list<ClientSession*> ClientList;
 
-  enum State {
-    kInitial,
-    kStarted,
-    kStopping,
-    kStopped,
-  };
-
-  virtual ~ChromotingHost();
-
-  // Called from Shutdown() to finish shutdown.
-  void ShutdownFinish();
-
   // Unless specified otherwise all members of this class must be
   // used on the network thread only.
 
@@ -184,8 +167,8 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // The connections to remote clients.
   ClientList clients_;
 
-  // Tracks the internal state of the host.
-  State state_;
+  // True if the host has been started.
+  bool started_;
 
   // Configuration of the protocol.
   scoped_ptr<protocol::CandidateSessionConfig> protocol_config_;
@@ -197,10 +180,6 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   bool authenticating_client_;
   bool reject_authenticating_client_;
 
-  // Stores list of tasks that should be executed when we finish
-  // shutdown. Used only while |state_| is set to kStopping.
-  std::vector<base::Closure> shutdown_tasks_;
-
   // The maximum duration of any session.
   base::TimeDelta max_session_duration_;
 
diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index 77a41b6..d7fce86 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -68,7 +68,7 @@ class ChromotingHostTest : public testing::Test {
   }
 
   virtual void SetUp() OVERRIDE {
-    ui_task_runner_ = new AutoThreadTaskRunner(
+    task_runner_ = new AutoThreadTaskRunner(
         message_loop_.message_loop_proxy(),
         base::Bind(&ChromotingHostTest::QuitMainMessageLoop,
                    base::Unretained(this)));
@@ -84,16 +84,16 @@ class ChromotingHostTest : public testing::Test {
 
     session_manager_ = new protocol::MockSessionManager();
 
-    host_ = new ChromotingHost(
+    host_.reset(new ChromotingHost(
         &signal_strategy_,
         desktop_environment_factory_.get(),
         scoped_ptr<protocol::SessionManager>(session_manager_),
-        ui_task_runner_,  // Audio
-        ui_task_runner_,  // Input
-        ui_task_runner_,  // Video capture
-        ui_task_runner_,  // Video encode
-        ui_task_runner_,  // Network
-        ui_task_runner_); // UI
+        task_runner_,   // Audio
+        task_runner_,   // Input
+        task_runner_,   // Video capture
+        task_runner_,   // Video encode
+        task_runner_,   // Network
+        task_runner_)); // UI
     host_->AddStatusObserver(&host_status_observer_);
 
     xmpp_login_ = "host@domain";
@@ -183,41 +183,40 @@ class ChromotingHostTest : public testing::Test {
     protocol::ConnectionToClient* connection_ptr = connection.get();
     scoped_ptr<ClientSession> client(new ClientSession(
         host_.get(),
-        ui_task_runner_,  // Audio
-        ui_task_runner_,  // Input
-        ui_task_runner_,  // Video capture
-        ui_task_runner_,  // Video encode
-        ui_task_runner_,  // Network
-        ui_task_runner_,  // UI
+        task_runner_, // Audio
+        task_runner_, // Input
+        task_runner_, // Video capture
+        task_runner_, // Video encode
+        task_runner_, // Network
+        task_runner_, // UI
         connection.Pass(),
         desktop_environment_factory_.get(),
         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_, base::Passed(&client)));
+    connection_ptr->set_host_stub(client.get());
 
     if (authenticate) {
-      ui_task_runner_->PostTask(
-          FROM_HERE, base::Bind(&ClientSession::OnConnectionAuthenticated,
-                                base::Unretained(client_raw), connection_ptr));
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&ClientSession::OnConnectionAuthenticated,
+                     base::Unretained(client.get()), connection_ptr));
       if (!reject) {
-        ui_task_runner_->PostTask(
+        task_runner_->PostTask(
             FROM_HERE,
             base::Bind(&ClientSession::OnConnectionChannelsConnected,
-                       base::Unretained(client_raw), connection_ptr));
+                       base::Unretained(client.get()), connection_ptr));
       }
     } else {
-      ui_task_runner_->PostTask(
+      task_runner_->PostTask(
           FROM_HERE, base::Bind(&ClientSession::OnConnectionClosed,
-                                base::Unretained(client_raw), connection_ptr,
+                                base::Unretained(client.get()), connection_ptr,
                                 protocol::AUTHENTICATION_FAILED));
     }
 
-    get_client(connection_index) = client_raw;
+    get_client(connection_index) = client.get();
+
+    // |host| is responsible for deleting |client| from now on.
+    host_->clients_.push_back(client.release());
   }
 
   virtual void TearDown() OVERRIDE {
@@ -297,23 +296,16 @@ class ChromotingHostTest : public testing::Test {
     }
   }
 
-  static void AddClientToHost(scoped_refptr<ChromotingHost> host,
-                              scoped_ptr<ClientSession> client) {
-    // |host| is responsible for deleting |client| from now on.
-    host->clients_.push_back(client.release());
-  }
-
   void ShutdownHost() {
-    ui_task_runner_->PostTask(
+    task_runner_->PostTask(
         FROM_HERE,
-        base::Bind(&ChromotingHost::Shutdown, host_,
-                   base::Bind(&ChromotingHostTest::ReleaseUiTaskRunner,
-                              base::Unretained(this))));
+        base::Bind(&ChromotingHostTest::StopAndReleaseTaskRunner,
+                   base::Unretained(this)));
   }
 
-  void ReleaseUiTaskRunner() {
-    ui_task_runner_ = NULL;
-    host_ = NULL;
+  void StopAndReleaseTaskRunner() {
+    host_.reset();
+    task_runner_ = NULL;
     desktop_environment_factory_.reset();
   }
 
@@ -399,11 +391,11 @@ class ChromotingHostTest : public testing::Test {
 
  protected:
   base::MessageLoop message_loop_;
-  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> task_runner_;
   MockConnectionToClientEventHandler handler_;
   MockSignalStrategy signal_strategy_;
   scoped_ptr<MockDesktopEnvironmentFactory> desktop_environment_factory_;
-  scoped_refptr<ChromotingHost> host_;
+  scoped_ptr<ChromotingHost> host_;
   MockHostStatusObserver host_status_observer_;
   protocol::MockSessionManager* session_manager_;
   std::string xmpp_login_;
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index e56074b..fa03cee 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -114,7 +114,6 @@ class HostNPScriptObject::It2MeImpl
   virtual void OnAccessDenied(const std::string& jid) OVERRIDE;
   virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE;
   virtual void OnClientDisconnected(const std::string& jid) OVERRIDE;
-  virtual void OnShutdown() OVERRIDE;
 
  private:
   friend class base::RefCountedThreadSafe<It2MeImpl>;
@@ -142,8 +141,13 @@ class HostNPScriptObject::It2MeImpl
                            const std::string& support_id,
                            const base::TimeDelta& lifetime);
 
-  // Called when ChromotingHost::Shutdown() has completed.
-  void OnShutdownFinished();
+  // Shuts down |host_| on the network thread and posts ShutdownOnUiThread()
+  // to shut down UI thread resources.
+  void ShutdownOnNetworkThread();
+
+  // Shuts down |desktop_environment_factory_| and |policy_watcher_| on
+  // the UI thread.
+  void ShutdownOnUiThread();
 
   // Called when initial policies are read, and when they change.
   void OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies);
@@ -168,7 +172,7 @@ class HostNPScriptObject::It2MeImpl
   scoped_ptr<DesktopEnvironmentFactory> desktop_environment_factory_;
   scoped_ptr<HostEventLogger> host_event_logger_;
 
-  scoped_refptr<ChromotingHost> host_;
+  scoped_ptr<ChromotingHost> host_;
   int failed_login_attempts_;
 
   scoped_ptr<policy_hack::PolicyWatcher> policy_watcher_;
@@ -254,13 +258,13 @@ void HostNPScriptObject::It2MeImpl::Disconnect() {
 
   switch (state_) {
     case kDisconnected:
-      OnShutdownFinished();
+      ShutdownOnNetworkThread();
       return;
 
     case kStarting:
       SetState(kDisconnecting);
       SetState(kDisconnected);
-      OnShutdownFinished();
+      ShutdownOnNetworkThread();
       return;
 
     case kDisconnecting:
@@ -270,18 +274,16 @@ void HostNPScriptObject::It2MeImpl::Disconnect() {
       SetState(kDisconnecting);
 
       if (!host_) {
-        OnShutdownFinished();
+        SetState(kDisconnected);
+        ShutdownOnNetworkThread();
         return;
       }
 
-      // ChromotingHost::Shutdown() may destroy SignalStrategy
-      // synchronously, but SignalStrategy::Listener handlers are not
-      // allowed to destroy SignalStrategy, so post task to call
-      // Shutdown() later.
+      // Deleting the host destroys SignalStrategy synchronously, but
+      // SignalStrategy::Listener handlers are not allowed to destroy
+      // SignalStrategy, so post task to destroy the host later.
       host_context_->network_task_runner()->PostTask(
-          FROM_HERE, base::Bind(
-              &ChromotingHost::Shutdown, host_,
-              base::Bind(&It2MeImpl::OnShutdownFinished, this)));
+          FROM_HERE, base::Bind(&It2MeImpl::ShutdownOnNetworkThread, this));
       return;
   }
 }
@@ -369,7 +371,7 @@ void HostNPScriptObject::It2MeImpl::FinishConnect(
   }
 
   // Create the host.
-  host_ = new ChromotingHost(
+  host_.reset(new ChromotingHost(
       signal_strategy_.get(),
       desktop_environment_factory_.get(),
       CreateHostSessionManager(network_settings,
@@ -379,7 +381,7 @@ void HostNPScriptObject::It2MeImpl::FinishConnect(
       host_context_->video_capture_task_runner(),
       host_context_->video_encode_task_runner(),
       host_context_->network_task_runner(),
-      host_context_->ui_task_runner());
+      host_context_->ui_task_runner()));
   host_->AddStatusObserver(this);
   log_to_server_.reset(
       new LogToServer(host_->AsWeakPtr(), ServerLogEntry::IT2ME,
@@ -404,14 +406,27 @@ void HostNPScriptObject::It2MeImpl::FinishConnect(
   return;
 }
 
-void HostNPScriptObject::It2MeImpl::OnShutdownFinished() {
-  if (!host_context_->ui_task_runner()->BelongsToCurrentThread()) {
-    host_context_->ui_task_runner()->PostTask(
-        FROM_HERE, base::Bind(&It2MeImpl::OnShutdownFinished, this));
-    return;
+void HostNPScriptObject::It2MeImpl::ShutdownOnNetworkThread() {
+  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+  DCHECK(state_ == kDisconnecting || state_ == kDisconnected);
+
+  if (state_ == kDisconnecting) {
+    host_event_logger_.reset();
+    host_->RemoveStatusObserver(this);
+    host_.reset();
+
+    register_request_.reset();
+    log_to_server_.reset();
+    signal_strategy_.reset();
+    SetState(kDisconnected);
   }
 
-  // Note that OnShutdownFinished() may be called more than once.
+  host_context_->ui_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&It2MeImpl::ShutdownOnUiThread, this));
+}
+
+void HostNPScriptObject::It2MeImpl::ShutdownOnUiThread() {
+  DCHECK(host_context_->ui_task_runner()->BelongsToCurrentThread());
 
   // Destroy the DesktopEnvironmentFactory, to free thread references.
   desktop_environment_factory_.reset();
@@ -478,21 +493,6 @@ void HostNPScriptObject::It2MeImpl::OnClientDisconnected(
   Disconnect();
 }
 
-void HostNPScriptObject::It2MeImpl::OnShutdown() {
-  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
-
-  register_request_.reset();
-  log_to_server_.reset();
-  signal_strategy_.reset();
-  host_event_logger_.reset();
-  host_->RemoveStatusObserver(this);
-  host_ = NULL;
-
-  if (state_ != kDisconnected) {
-    SetState(kDisconnected);
-  }
-}
-
 void HostNPScriptObject::It2MeImpl::OnPolicyUpdate(
     scoped_ptr<base::DictionaryValue> policies) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index b083e7b..90aa6ad 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -282,7 +282,7 @@ class HostProcess
   scoped_ptr<LogToServer> log_to_server_;
   scoped_ptr<HostEventLogger> host_event_logger_;
 
-  scoped_refptr<ChromotingHost> host_;
+  scoped_ptr<ChromotingHost> host_;
 
   // Used to keep this HostProcess alive until it is shutdown.
   scoped_refptr<HostProcess> self_;
@@ -950,7 +950,7 @@ void HostProcess::StartHost() {
     network_settings.max_port = NetworkSettings::kDefaultMaxPort;
   }
 
-  host_ = new ChromotingHost(
+  host_.reset(new ChromotingHost(
       signal_strategy_.get(),
       desktop_environment_factory_.get(),
       CreateHostSessionManager(network_settings,
@@ -960,7 +960,7 @@ void HostProcess::StartHost() {
       context_->video_capture_task_runner(),
       context_->video_encode_task_runner(),
       context_->network_task_runner(),
-      context_->ui_task_runner());
+      context_->ui_task_runner()));
 
   // TODO(simonmorris): Get the maximum session duration from a policy.
 #if defined(OS_LINUX)
@@ -1041,7 +1041,7 @@ void HostProcess::RestartHost() {
   DCHECK_EQ(state_, HOST_STARTED);
 
   state_ = HOST_STOPPING_TO_RESTART;
-  host_->Shutdown(base::Bind(&HostProcess::ShutdownOnNetworkThread, this));
+  ShutdownOnNetworkThread();
 }
 
 void HostProcess::ShutdownHost(int exit_code) {
@@ -1051,13 +1051,9 @@ void HostProcess::ShutdownHost(int exit_code) {
 
   switch (state_) {
     case HOST_INITIALIZING:
-      state_ = HOST_STOPPING;
-      ShutdownOnNetworkThread();
-      break;
-
     case HOST_STARTED:
       state_ = HOST_STOPPING;
-      host_->Shutdown(base::Bind(&HostProcess::ShutdownOnNetworkThread, this));
+      ShutdownOnNetworkThread();
       break;
 
     case HOST_STOPPING_TO_RESTART:
@@ -1074,7 +1070,7 @@ void HostProcess::ShutdownHost(int exit_code) {
 void HostProcess::ShutdownOnNetworkThread() {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
-  host_ = NULL;
+  host_.reset();
   curtaining_host_observer_.reset();
   host_event_logger_.reset();
   log_to_server_.reset();
@@ -1103,8 +1099,7 @@ void HostProcess::ShutdownOnNetworkThread() {
         FROM_HERE,
         base::Bind(&HostProcess::ShutdownOnUiThread, this));
   } else {
-    // This method is used as a callback for ChromotingHost::Shutdown() which is
-    // called only in STOPPING_TO_RESTART and STOPPING states.
+    // This method is only called in STOPPING_TO_RESTART and STOPPING states.
     NOTREACHED();
   }
 }
author	alexeypa@chromium.org <alexeypa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2013-04-30 20:46:16 +0000
committer	alexeypa@chromium.org <alexeypa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2013-04-30 20:46:16 +0000
commit	22f3d247091711bf7844c9e865d99dae3ff7dd5a (patch)
tree	1b9e1653d1eef758eb341870117cd3268d1044d7
parent	31c36e98a7e1e60a7575457dc5379d05eaf88898 (diff)
download	chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.zip chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.tar.gz chromium_src-22f3d247091711bf7844c9e865d99dae3ff7dd5a.tar.bz2