Create OS windows on the UI thread, since that's where they're destroyed.

BUG=153946


Review URL: https://chromiumcodereview.appspot.com/11145008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162838 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 84 insertions, 75 deletions

diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index 6438a1b..36164913 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -66,6 +66,49 @@ void DoNothing() {
 
 }  // namespace
 
+class MockIt2MeHostUserInterface : public It2MeHostUserInterface {
+ public:
+  MockIt2MeHostUserInterface(
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+
+  void InitFrom(
+      scoped_ptr<DisconnectWindow> disconnect_window,
+      scoped_ptr<ContinueWindow> continue_window,
+      scoped_ptr<LocalInputMonitor> local_input_monitor);
+
+  // A test-only version of Start that does not register a HostStatusObserver.
+  // TODO(rmsousa): Make the unit tests work with the regular Start().
+  virtual void Start(ChromotingHost* host,
+                     const base::Closure& disconnect_callback) OVERRIDE;
+};
+
+MockIt2MeHostUserInterface::MockIt2MeHostUserInterface(
+    scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
+    : It2MeHostUserInterface(network_task_runner, ui_task_runner) {
+}
+
+void MockIt2MeHostUserInterface::InitFrom(
+    scoped_ptr<DisconnectWindow> disconnect_window,
+    scoped_ptr<ContinueWindow> continue_window,
+    scoped_ptr<LocalInputMonitor> local_input_monitor) {
+  DCHECK(ui_task_runner()->BelongsToCurrentThread());
+
+  disconnect_window_ = disconnect_window.Pass();
+  continue_window_ = continue_window.Pass();
+  local_input_monitor_ = local_input_monitor.Pass();
+}
+
+void MockIt2MeHostUserInterface::Start(
+    ChromotingHost* host, const base::Closure& disconnect_callback) {
+  DCHECK(network_task_runner()->BelongsToCurrentThread());
+  DCHECK(host_ == NULL);
+
+  host_ = host;
+  disconnect_callback_ = disconnect_callback;
+}
+
 class ChromotingHostTest : public testing::Test {
  public:
   ChromotingHostTest() {
@@ -111,15 +154,16 @@ class ChromotingHostTest : public testing::Test {
     continue_window_ = new MockContinueWindow();
     local_input_monitor_ = new MockLocalInputMonitor();
     it2me_host_user_interface_.reset(
-        new It2MeHostUserInterface(context_.network_task_runner(),
-                                   context_.ui_task_runner()));
-    it2me_host_user_interface_->StartForTest(
-        host_,
-        base::Bind(&ChromotingHost::Shutdown, host_, base::Closure()),
+        new MockIt2MeHostUserInterface(context_.network_task_runner(),
+                                       context_.ui_task_runner()));
+    it2me_host_user_interface_->InitFrom(
         scoped_ptr<DisconnectWindow>(disconnect_window_),
         scoped_ptr<ContinueWindow>(continue_window_),
         scoped_ptr<LocalInputMonitor>(local_input_monitor_));
 
+    it2me_host_user_interface_->Start(
+        host_, base::Bind(&ChromotingHost::Shutdown, host_, base::Closure()));
+
     xmpp_login_ = "host@domain";
     session1_ = new MockSession();
     session2_ = new MockSession();
@@ -403,7 +447,7 @@ class ChromotingHostTest : public testing::Test {
   MockConnectionToClientEventHandler handler_;
   MockSignalStrategy signal_strategy_;
   scoped_ptr<MockDesktopEnvironmentFactory> desktop_environment_factory_;
-  scoped_ptr<It2MeHostUserInterface> it2me_host_user_interface_;
+  scoped_ptr<MockIt2MeHostUserInterface> it2me_host_user_interface_;
   scoped_refptr<ChromotingHost> host_;
   MockHostStatusObserver host_status_observer_;
   protocol::MockSessionManager* session_manager_;
diff --git a/remoting/host/host_user_interface.cc b/remoting/host/host_user_interface.cc
index 576a899..dd8d33f 100644
--- a/remoting/host/host_user_interface.cc
+++ b/remoting/host/host_user_interface.cc
@@ -30,6 +30,13 @@ HostUserInterface::~HostUserInterface() {
   ShowDisconnectWindow(false, std::string());
 }
 
+void HostUserInterface::Init() {
+  DCHECK(ui_task_runner_->BelongsToCurrentThread());
+
+  disconnect_window_ = DisconnectWindow::Create();
+  local_input_monitor_ = LocalInputMonitor::Create();
+}
+
 void HostUserInterface::Start(ChromotingHost* host,
                               const base::Closure& disconnect_callback) {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
@@ -37,8 +44,6 @@ void HostUserInterface::Start(ChromotingHost* host,
 
   host_ = host;
   disconnect_callback_ = disconnect_callback;
-  disconnect_window_ = DisconnectWindow::Create();
-  local_input_monitor_ = LocalInputMonitor::Create();
   host_->AddStatusObserver(this);
 }
 
@@ -112,20 +117,6 @@ void HostUserInterface::ProcessOnClientDisconnected() {
   ShowDisconnectWindow(false, std::string());
 }
 
-void HostUserInterface::StartForTest(
-    ChromotingHost* host,
-    const base::Closure& disconnect_callback,
-    scoped_ptr<DisconnectWindow> disconnect_window,
-    scoped_ptr<LocalInputMonitor> local_input_monitor) {
-  DCHECK(network_task_runner()->BelongsToCurrentThread());
-  DCHECK(host_ == NULL);
-
-  host_ = host;
-  disconnect_callback_ = disconnect_callback;
-  disconnect_window_ = disconnect_window.Pass();
-  local_input_monitor_ = local_input_monitor.Pass();
-}
-
 void HostUserInterface::MonitorLocalInputs(bool enable) {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
 
diff --git a/remoting/host/host_user_interface.h b/remoting/host/host_user_interface.h
index 528df30..f4d3f60 100644
--- a/remoting/host/host_user_interface.h
+++ b/remoting/host/host_user_interface.h
@@ -32,6 +32,10 @@ class HostUserInterface : public HostStatusObserver {
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
   virtual ~HostUserInterface();
 
+  // Initialize the OS-specific UI objects.
+  // Init must be called from |ui_task_runner_|.
+  virtual void Init();
+
   // Start the HostUserInterface for |host|.  |disconnect_callback| will be
   // called on |ui_task_runner| when |host| is shut down.  |host| must remain
   // valid at least until ChromotingHost::Shutdown() completes.
@@ -61,13 +65,17 @@ class HostUserInterface : public HostStatusObserver {
   virtual void ProcessOnClientAuthenticated(const std::string& username);
   virtual void ProcessOnClientDisconnected();
 
-  // Used by unit-tests as an alternative to Start() so that mock versions of
-  // internal objects can be used.
-  void StartForTest(
-      ChromotingHost* host,
-      const base::Closure& disconnect_callback,
-      scoped_ptr<DisconnectWindow> disconnect_window,
-      scoped_ptr<LocalInputMonitor> local_input_monitor);
+  ChromotingHost* host_;
+
+  // Used to ask the host to disconnect the session.
+  base::Closure disconnect_callback_;
+
+  // Provide a user interface allowing the host user to close the connection.
+  scoped_ptr<DisconnectWindow> disconnect_window_;
+
+  // Monitor local inputs to allow remote inputs to be blocked while the local
+  // user is trying to do something.
+  scoped_ptr<LocalInputMonitor> local_input_monitor_;
 
  private:
   // Invoked from the UI thread when the user clicks on the Disconnect button
@@ -84,8 +92,6 @@ class HostUserInterface : public HostStatusObserver {
   // is connected).
   std::string authenticated_jid_;
 
-  ChromotingHost* host_;
-
   // Thread on which the ChromotingHost processes network events.
   // Notifications from the host, and some calls into it, use this thread.
   scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
@@ -93,16 +99,6 @@ class HostUserInterface : public HostStatusObserver {
   // Thread on which to run the user interface.
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 
-  // Used to ask the host to disconnect the session.
-  base::Closure disconnect_callback_;
-
-  // Provide a user interface allowing the host user to close the connection.
-  scoped_ptr<DisconnectWindow> disconnect_window_;
-
-  // Monitor local inputs to allow remote inputs to be blocked while the local
-  // user is trying to do something.
-  scoped_ptr<LocalInputMonitor> local_input_monitor_;
-
   bool is_monitoring_local_inputs_;
 
   // WeakPtr used to avoid tasks accessing the client after it is deleted.
diff --git a/remoting/host/it2me_host_user_interface.cc b/remoting/host/it2me_host_user_interface.cc
index 4b3145f..38e0624 100644
--- a/remoting/host/it2me_host_user_interface.cc
+++ b/remoting/host/it2me_host_user_interface.cc
@@ -28,6 +28,7 @@ It2MeHostUserInterface::It2MeHostUserInterface(
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
     : HostUserInterface(network_task_runner, ui_task_runner),
       ALLOW_THIS_IN_INITIALIZER_LIST(timer_weak_factory_(this)) {
+  DCHECK(ui_task_runner->BelongsToCurrentThread());
 }
 
 It2MeHostUserInterface::~It2MeHostUserInterface() {
@@ -36,11 +37,10 @@ It2MeHostUserInterface::~It2MeHostUserInterface() {
   ShowContinueWindow(false);
 }
 
-void It2MeHostUserInterface::Start(ChromotingHost* host,
-                                   const base::Closure& disconnect_callback) {
-  DCHECK(network_task_runner()->BelongsToCurrentThread());
+void It2MeHostUserInterface::Init() {
+  DCHECK(ui_task_runner()->BelongsToCurrentThread());
 
-  HostUserInterface::Start(host, disconnect_callback);
+  HostUserInterface::Init();
   continue_window_ = ContinueWindow::Create();
 }
 
@@ -73,18 +73,6 @@ void It2MeHostUserInterface::ProcessOnClientDisconnected() {
   StartContinueWindowTimer(false);
 }
 
-void It2MeHostUserInterface::StartForTest(
-    ChromotingHost* host,
-    const base::Closure& disconnect_callback,
-    scoped_ptr<DisconnectWindow> disconnect_window,
-    scoped_ptr<ContinueWindow> continue_window,
-    scoped_ptr<LocalInputMonitor> local_input_monitor) {
-  HostUserInterface::StartForTest(host, disconnect_callback,
-                                  disconnect_window.Pass(),
-                                  local_input_monitor.Pass());
-  continue_window_ = continue_window.Pass();
-}
-
 void It2MeHostUserInterface::ContinueSession(bool continue_session) {
   DCHECK(ui_task_runner()->BelongsToCurrentThread());
 
diff --git a/remoting/host/it2me_host_user_interface.h b/remoting/host/it2me_host_user_interface.h
index da034d1..28e3e22 100644
--- a/remoting/host/it2me_host_user_interface.h
+++ b/remoting/host/it2me_host_user_interface.h
@@ -30,8 +30,7 @@ class It2MeHostUserInterface : public HostUserInterface {
   virtual ~It2MeHostUserInterface();
 
   // HostUserInterface overrides.
-  virtual void Start(ChromotingHost* host,
-                     const base::Closure& disconnect_callback)  OVERRIDE;
+  virtual void Init() OVERRIDE;
 
   // HostStatusObserver implementation.  These methods will be called from the
   // network thread.
@@ -42,21 +41,13 @@ class It2MeHostUserInterface : public HostUserInterface {
       const std::string& username) OVERRIDE;
   virtual void ProcessOnClientDisconnected() OVERRIDE;
 
+  // Provide a user interface requiring the user to periodically re-confirm
+  // the connection.
+  scoped_ptr<ContinueWindow> continue_window_;
+
  private:
   class TimerTask;
 
-  // Allow ChromotingHostTest::SetUp() to call StartForTest().
-  friend class ChromotingHostTest;
-
-  // Used by unit-tests as an alternative to Start() so that mock versions of
-  // internal objects can be used.
-  void StartForTest(
-      ChromotingHost* host,
-      const base::Closure& disconnect_callback,
-      scoped_ptr<DisconnectWindow> disconnect_window,
-      scoped_ptr<ContinueWindow> continue_window,
-      scoped_ptr<LocalInputMonitor> local_input_monitor);
-
   // Called by the ContinueWindow implementation (on the UI thread) when the
   // user dismisses the Continue prompt.
   void ContinueSession(bool continue_session);
@@ -67,10 +58,6 @@ class It2MeHostUserInterface : public HostUserInterface {
   void ShowContinueWindow(bool show);
   void StartContinueWindowTimer(bool start);
 
-  // Provide a user interface requiring the user to periodically re-confirm
-  // the connection.
-  scoped_ptr<ContinueWindow> continue_window_;
-
   // Weak pointer factory used to abandon the "continue session" timer when
   // hiding the "continue session" dialog, or tearing down the IT2Me UI.
   base::WeakPtrFactory<It2MeHostUserInterface> timer_weak_factory_;
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index cc6a708..664d132 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -232,6 +232,7 @@ void HostNPScriptObject::It2MeImpl::Connect(
   it2me_host_user_interface_.reset(
       new It2MeHostUserInterface(host_context_->network_task_runner(),
                                  host_context_->ui_task_runner()));
+  it2me_host_user_interface_->Init();
 
   // Switch to the network thread to start the actual connection.
   host_context_->network_task_runner()->PostTask(
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 3c6c840..09ee9dc 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -367,6 +367,7 @@ class HostProcess
       host_user_interface_.reset(
           new HostUserInterface(context_->network_task_runner(),
                                 context_->ui_task_runner()));
+      host_user_interface_->Init();
     }
 
     StartWatchingConfigChanges();
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index 367e78c..03d9e17 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -286,6 +286,7 @@ class SimpleHost : public HeartbeatSender::Listener {
       it2me_host_user_interface_.reset(
           new It2MeHostUserInterface(context_.network_task_runner(),
                                      context_.ui_task_runner()));
+      it2me_host_user_interface_->Init();
       it2me_host_user_interface_->Start(
           host_,
           base::Bind(&ChromotingHost::Shutdown, host_, base::Closure()));