Separate out HostConnection into an interface and a jingle-based

implementation.  Refactor to inject the running thread for Jingle.

Review URL: http://codereview.chromium.org/2753006

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

2 files changed, 42 insertions, 58 deletions

diff --git a/remoting/jingle_glue/jingle_client.cc b/remoting/jingle_glue/jingle_client.cc
index bf6baee..8fb0714 100644
--- a/remoting/jingle_glue/jingle_client.cc
+++ b/remoting/jingle_glue/jingle_client.cc
@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// TODO(ajwong): Check the initialization sentinels.  Can we base it off of
+// state_ instead of a member variable?  Also, we assign and read from a few of
+// the member variables on two threads.  We need to audit this for thread
+// safety.
+
 #include "remoting/jingle_glue/jingle_client.h"
 
 #include "base/logging.h"
@@ -24,9 +29,12 @@
 
 namespace remoting {
 
-JingleClient::JingleClient()
-    : callback_(NULL),
-      state_(START) { }
+JingleClient::JingleClient(JingleThread* thread)
+    : client_(NULL),
+      thread_(thread),
+      state_(START),
+      callback_(NULL) {
+}
 
 JingleClient::~JingleClient() {
   // JingleClient can be destroyed only after it's closed.
@@ -38,67 +46,46 @@ void JingleClient::Init(
     const std::string& auth_token_service, Callback* callback) {
   DCHECK(username != "");
   DCHECK(callback != NULL);
-  DCHECK(thread_ == NULL);  // Init() can be called only once.
+  DCHECK(callback_ == NULL);  // Init() can be called only once.
 
   callback_ = callback;
 
-  thread_.reset(new JingleThread());
-  thread_->Start();
-  thread_->message_loop()->PostTask(
+  message_loop()->PostTask(
       FROM_HERE, NewRunnableMethod(this, &JingleClient::DoInitialize,
                                    username, auth_token, auth_token_service));
 }
 
-class JingleClient::ConnectRequest {
- public:
-  ConnectRequest()
-      : completed_event_(true, false) { }
-
-  JingleChannel* Wait() {
-    completed_event_.Wait();
-    return channel_;
-  };
-
-  void Done(JingleChannel* channel) {
-    channel_ = channel;
-    completed_event_.Signal();
-  };
-
- private:
-  base::WaitableEvent completed_event_;
-  JingleChannel* channel_;
-};
-
 JingleChannel* JingleClient::Connect(const std::string& host_jid,
                                      JingleChannel::Callback* callback) {
-  ConnectRequest request;
-  thread_->message_loop()->PostTask(
+  // Ownership if channel is given to DoConnect.
+  scoped_refptr<JingleChannel> channel = new JingleChannel(callback);
+  message_loop()->PostTask(
       FROM_HERE, NewRunnableMethod(this, &JingleClient::DoConnect,
-                                   &request, host_jid, callback));
-  return request.Wait();
+                                   channel, host_jid, callback));
+  return channel;
 }
 
-void JingleClient::DoConnect(ConnectRequest* request,
+void JingleClient::DoConnect(scoped_refptr<JingleChannel> channel,
                              const std::string& host_jid,
                              JingleChannel::Callback* callback) {
+  DCHECK_EQ(message_loop(), MessageLoop::current());
+
   talk_base::StreamInterface* stream =
       tunnel_session_client_->CreateTunnel(buzz::Jid(host_jid), "");
   DCHECK(stream != NULL);
 
-  JingleChannel* channel = new JingleChannel(callback);
-  channel->Init(thread_.get(), stream, host_jid);
-  request->Done(channel);
+  channel->Init(thread_, stream, host_jid);
 }
 
 void JingleClient::Close() {
-  DCHECK(thread_ != NULL);  // Close() be called only after Init().
   message_loop()->PostTask(
       FROM_HERE, NewRunnableMethod(this, &JingleClient::DoClose));
-  thread_->Stop();
-  thread_.reset(NULL);
 }
 
 void JingleClient::DoClose() {
+  DCHECK_EQ(message_loop(), MessageLoop::current());
+  DCHECK(callback_ != NULL);  // Close() should only be called after Init().
+
   client_->Disconnect();
   // Client is deleted by TaskRunner.
   client_ = NULL;
@@ -112,6 +99,8 @@ void JingleClient::DoClose() {
 void JingleClient::DoInitialize(const std::string& username,
                                 const std::string& auth_token,
                                 const std::string& auth_token_service) {
+  DCHECK_EQ(message_loop(), MessageLoop::current());
+
   buzz::Jid login_jid(username);
 
   buzz::XmppClientSettings settings;
@@ -154,9 +143,10 @@ void JingleClient::DoInitialize(const std::string& username,
                                        session_manager_.get()));
 #endif  // USE_SSL_TUNNEL
 
-  receiver_ = new cricket::SessionManagerTask(client_, session_manager_.get());
-  receiver_->EnableOutgoingMessages();
-  receiver_->Start();
+  cricket::SessionManagerTask* receiver =
+      new cricket::SessionManagerTask(client_, session_manager_.get());
+  receiver->EnableOutgoingMessages();
+  receiver->Start();
 
   tunnel_session_client_->SignalIncomingTunnel.connect(
       this, &JingleClient::OnIncomingTunnel);
@@ -168,9 +158,6 @@ std::string JingleClient::GetFullJid() {
 }
 
 MessageLoop* JingleClient::message_loop() {
-  if (thread_ == NULL) {
-    return NULL;
-  }
   return thread_->message_loop();
 }
 
@@ -210,7 +197,7 @@ void JingleClient::OnIncomingTunnel(
     talk_base::StreamInterface* stream =
         client->AcceptTunnel(session);
     scoped_refptr<JingleChannel> channel(new JingleChannel(channel_callback));
-    channel->Init(thread_.get(), stream, jid.Str());
+    channel->Init(thread_, stream, jid.Str());
     callback_->OnNewConnection(this, channel);
   } else {
     client->DeclineTunnel(session);
diff --git a/remoting/jingle_glue/jingle_client.h b/remoting/jingle_glue/jingle_client.h
index 2a68042..a074757 100644
--- a/remoting/jingle_glue/jingle_client.h
+++ b/remoting/jingle_glue/jingle_client.h
@@ -60,13 +60,15 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
                                  scoped_refptr<JingleChannel> channel) = 0;
   };
 
-  JingleClient();
+  // Creates a JingleClient object that executes on |thread|.  This does not
+  // take ownership of |thread| and expects that the thread is started before
+  // the constructor is called, and only stopped after the JingleClient object
+  // has been destructed.
+  JingleClient(JingleThread* thread);
   virtual ~JingleClient();
 
-  // Starts jingle thread and XMPP connection inialization. Must be called
-  // only once. message_loop() is guaranteed to exist after this method returns,
-  // but the connection may not be open yet. |callback| specifies callback
-  // object for the client and must not be NULL.
+  // Starts the XMPP connection inialization. Must be called only once.
+  // |callback| specifies callback object for the client and must not be NULL.
   void Init(const std::string& username, const std::string& auth_token,
             const std::string& auth_token_service, Callback* callback);
 
@@ -92,13 +94,10 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
   // Returns XmppClient object for the xmpp connection or NULL if not connected.
   buzz::XmppClient* xmpp_client() { return client_; }
 
-  // Message loop for the jingle thread or NULL if the thread is not started.
+  // Message loop used by this object to execute tasks.
   MessageLoop* message_loop();
 
  private:
-  // Used by Connect().
-  class ConnectRequest;
-
   void OnConnectionStateChanged(buzz::XmppEngine::State state);
 
   void OnIncomingTunnel(cricket::TunnelSessionClient* client, buzz::Jid jid,
@@ -109,7 +108,7 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
                     const std::string& auth_token_service);
 
   // Used by Connect().
-  void DoConnect(ConnectRequest* request,
+  void DoConnect(scoped_refptr<JingleChannel> channel,
                  const std::string& host_jid,
                  JingleChannel::Callback* callback);
 
@@ -123,9 +122,8 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
   buzz::PreXmppAuth* CreatePreXmppAuth(
       const buzz::XmppClientSettings& settings);
 
-
   buzz::XmppClient* client_;
-  scoped_ptr<JingleThread> thread_;
+  JingleThread* thread_;
   State state_;
   Callback* callback_;
 
@@ -136,7 +134,6 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
   scoped_ptr<cricket::BasicPortAllocator> port_allocator_;
   scoped_ptr<cricket::SessionManager> session_manager_;
   scoped_ptr<cricket::TunnelSessionClient> tunnel_session_client_;
-  cricket::SessionManagerTask* receiver_;
 
   DISALLOW_COPY_AND_ASSIGN(JingleClient);
 };