Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(351)

Unified Diff: remoting/host/plugin/host_script_object.cc

Issue 13466014: Made the ChromotingHost class not ref-counted. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: cosmetic Created 7 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: remoting/host/plugin/host_script_object.cc
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index b091fe623eaff29cbba93d580b5ec6cbf1d22972..bb32964706c30f616c6f8957dadcc2ff509d03bf 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -142,8 +142,12 @@ 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.
Wez 2013/04/02 19:58:33 nit: ... and posts ShutdownOnUiThread() to shut do
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);
@@ -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();
+ ShutdownOnUiThread();
Wez 2013/04/02 19:58:33 nit: Call ShutdownOnNetworkThread() here and let t
alexeypa (please no reviews) 2013/04/08 23:28:17 Done.
return;
case kStarting:
SetState(kDisconnecting);
SetState(kDisconnected);
- OnShutdownFinished();
+ ShutdownOnUiThread();
return;
case kDisconnecting:
@@ -270,18 +274,17 @@ void HostNPScriptObject::It2MeImpl::Disconnect() {
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 ShutdownOnNetworkThrea
alexeypa (please no reviews) 2013/04/08 23:28:17 It will not work because HostNPScriptObject::It2Me
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 +372,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 +382,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 +407,25 @@ void HostNPScriptObject::It2MeImpl::FinishConnect(
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
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();
@@ -486,11 +500,6 @@ void HostNPScriptObject::It2MeImpl::OnShutdown() {
signal_strategy_.reset();
host_event_logger_.reset();
host_->RemoveStatusObserver(this);
- host_ = NULL;
-
- if (state_ != kDisconnected) {
- SetState(kDisconnected);
- }
}
void HostNPScriptObject::It2MeImpl::OnPolicyUpdate(

Powered by Google App Engine
This is Rietveld 408576698