Made the ChromotingHost class not ref-counted.

remoting/host/plugin/host_script_object.cc

 // Copyright (c) 2012 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/plugin/host_script_object.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop.h"
@@ skipping 124 matching lines @@
   // Called by ReadPolicyAndConnect once policies have been read.
   void FinishConnect(const std::string& uid,
                      const std::string& auth_token,
                      const std::string& auth_service);
 
   // Called when the support host registration completes.
   void OnReceivedSupportID(bool success,
                            const std::string& support_id,
                            const base::TimeDelta& lifetime);
 
-  // Called when ChromotingHost::Shutdown() has completed.
-  void OnShutdownFinished();
+  // Shuts down |host_| on the network thread.

[Wez, 2013/04/02 19:58:33]

nit: ... and posts ShutdownOnUiThread() to shut down UI thread resources.

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

+  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);
 
   // Handlers for NAT traversal and host domain policies.
   void UpdateNatPolicy(bool nat_traversal_enabled);
   void UpdateHostDomainPolicy(const std::string& host_domain);
 
   // Caller supplied fields.
   scoped_ptr<ChromotingHostContext> host_context_;
   scoped_refptr<base::SingleThreadTaskRunner> plugin_task_runner_;
   base::WeakPtr<HostNPScriptObject> script_object_;
   XmppSignalStrategy::XmppServerConfig xmpp_server_config_;
   std::string directory_bot_jid_;
 
   State state_;
 
   scoped_refptr<RsaKeyPair> host_key_pair_;
   scoped_ptr<SignalStrategy> signal_strategy_;
   scoped_ptr<RegisterSupportHostRequest> register_request_;
   scoped_ptr<LogToServer> log_to_server_;
   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_;
 
   // Host the current nat traversal policy setting.
   bool nat_traversal_enabled_;
 
   // The host domain policy setting.
   std::string required_host_domain_;
 
@@ skipping 65 matching lines @@
 void HostNPScriptObject::It2MeImpl::Disconnect() {
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     DCHECK(plugin_task_runner_->BelongsToCurrentThread());
     host_context_->network_task_runner()->PostTask(
         FROM_HERE, base::Bind(&It2MeImpl::Disconnect, this));
     return;
   }
 
   switch (state_) {
     case kDisconnected:
-      OnShutdownFinished();
+      ShutdownOnUiThread();

[Wez, 2013/04/02 19:58:33]

nit: Call ShutdownOnNetworkThread() here and let that post on to
ShutdownOnUiThread(), so that the post-to-UI-thread can go in
ShutdownOnNetworkThread for clarity.

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

       return;
 
     case kStarting:
       SetState(kDisconnecting);
       SetState(kDisconnected);
-      OnShutdownFinished();
+      ShutdownOnUiThread();
       return;
 
     case kDisconnecting:
       return;
 
     default:
       SetState(kDisconnecting);
 
       if (!host_) {
-        OnShutdownFinished();
+        SetState(kDisconnected);
+        ShutdownOnUiThread();
         return;
       }
 
       // ChromotingHost::Shutdown() may destroy SignalStrategy
       // synchronously, but SignalStrategy::Listener handlers are not
       // allowed to destroy SignalStrategy, so post task to call
-      // Shutdown() later.
+      // ChromotingHost::Shutdown() later.

[Wez, 2013/04/02 19:58:33]

nit: Move this comment into ShutdownOnNetworkThread, and do the
deferred-shutdown there, i.e. 

if (host_) {
  network->PostTask(Shutdown);
  network->DeleteSoon(host_.release());
}

[alexeypa (please no reviews), 2013/04/08 23:28:17]

It will not work because HostNPScriptObject::It2MeImpl should destroy
|signal_strategy_| only after the host has been shutdown. See
HostNPScriptObject::It2MeImpl::OnShutdown().

       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;
   }
 }
 
 void HostNPScriptObject::It2MeImpl::RequestNatPolicy() {
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     DCHECK(plugin_task_runner_->BelongsToCurrentThread());
     host_context_->network_task_runner()->PostTask(
         FROM_HERE, base::Bind(&It2MeImpl::RequestNatPolicy, this));
     return;
@@ skipping 67 matching lines @@
   NetworkSettings network_settings(
      nat_traversal_enabled_ ?
      NetworkSettings::NAT_TRAVERSAL_ENABLED :
      NetworkSettings::NAT_TRAVERSAL_DISABLED);
   if (!nat_traversal_enabled_) {
     network_settings.min_port = NetworkSettings::kDefaultMinPort;
     network_settings.max_port = NetworkSettings::kDefaultMaxPort;
   }
 
   // Create the host.
-  host_ = new ChromotingHost(
+  host_.reset(new ChromotingHost(
       signal_strategy_.get(),
       desktop_environment_factory_.get(),
       CreateHostSessionManager(network_settings,
                                host_context_->url_request_context_getter()),
       host_context_->audio_task_runner(),
       host_context_->input_task_runner(),
       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,
                       signal_strategy_.get(), directory_bot_jid_));
 
   // Disable audio by default.
   // TODO(sergeyu): Add UI to enable it.
   scoped_ptr<protocol::CandidateSessionConfig> protocol_config =
       protocol::CandidateSessionConfig::CreateDefault();
   protocol::CandidateSessionConfig::DisableAudioChannel(protocol_config.get());
   host_->set_protocol_config(protocol_config.Pass());
 
   // Create event logger.
   host_event_logger_ =
       HostEventLogger::Create(host_->AsWeakPtr(), kApplicationName);
 
   // Connect signaling and start the host.
   signal_strategy_->Connect();
   host_->Start(uid);
 
   SetState(kRequestedAccessCode);
   return;
 }
 
-void HostNPScriptObject::It2MeImpl::OnShutdownFinished() {
+void HostNPScriptObject::It2MeImpl::ShutdownOnNetworkThread() {
+  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+  DCHECK_EQ(state_, kDisconnecting);
+
+  host_->Shutdown();
+  host_.reset();
+  SetState(kDisconnected);
+
+  ShutdownOnUiThread();
+}
+
+void HostNPScriptObject::It2MeImpl::ShutdownOnUiThread() {
   if (!host_context_->ui_task_runner()->BelongsToCurrentThread()) {

[Wez, 2013/04/02 19:58:33]

Rather than thread-hop here, can you move the post to ShutdownOnNetworkThread()?
 Shutdown should always be starting off from the network thread, so that
shouldn't add any extra thread-hop overhead.

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

     host_context_->ui_task_runner()->PostTask(
-        FROM_HERE, base::Bind(&It2MeImpl::OnShutdownFinished, this));
+        FROM_HERE, base::Bind(&It2MeImpl::ShutdownOnUiThread, this));
     return;
   }
 
-  // Note that OnShutdownFinished() may be called more than once.
+  // Note that ShutdownOnUiThread() may be called more than once.

[Wez, 2013/04/02 19:58:33]

Is this still true?

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

 
   // Destroy the DesktopEnvironmentFactory, to free thread references.
   desktop_environment_factory_.reset();
 
   // Stop listening for policy updates.
   if (policy_watcher_.get()) {
     base::WaitableEvent policy_watcher_stopped_(true, false);
     policy_watcher_->StopWatching(&policy_watcher_stopped_);
     policy_watcher_stopped_.Wait();
     policy_watcher_.reset();
@@ skipping 54 matching lines @@
 }
 
 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());
 
   bool nat_policy;
   if (policies->GetBoolean(policy_hack::PolicyWatcher::kNatPolicyName,
                            &nat_policy)) {
     UpdateNatPolicy(nat_policy);
@@ skipping 1060 matching lines @@
 }
 
 void HostNPScriptObject::SetException(const std::string& exception_string) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   g_npnetscape_funcs->setexception(parent_, exception_string.c_str());
   LOG(INFO) << exception_string;
 }
 
 }  // namespace remoting