Cleanup client shutdown sequence.

BUG=None
TEST=Client doesn't crash when reloading the tab.

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

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

3 files changed, 36 insertions, 38 deletions

diff --git a/remoting/jingle_glue/jingle_client.cc b/remoting/jingle_glue/jingle_client.cc
index 79688a9..d363b27 100644
--- a/remoting/jingle_glue/jingle_client.cc
+++ b/remoting/jingle_glue/jingle_client.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/jingle_glue/jingle_client.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
@@ -51,12 +52,12 @@ void JingleClient::Init() {
     initialized_ = true;
   }
 
-  message_loop()->PostTask(
+  message_loop_->PostTask(
       FROM_HERE, NewRunnableMethod(this, &JingleClient::DoInitialize));
 }
 
 void JingleClient::DoInitialize() {
-  DCHECK_EQ(message_loop(), MessageLoop::current());
+  DCHECK_EQ(message_loop_, MessageLoop::current());
 
   if (!network_manager_.get()) {
     VLOG(1) << "Creating talk_base::NetworkManager.";
@@ -111,40 +112,32 @@ void JingleClient::DoStartSession() {
   }
 }
 
-void JingleClient::Close() {
-  Close(NULL);
-}
-
-void JingleClient::Close(Task* closed_task) {
+void JingleClient::Close(const base::Closure& closed_task) {
   {
     base::AutoLock auto_lock(state_lock_);
     // If the client is already closed then don't close again.
     if (closed_) {
-      if (closed_task)
-        message_loop_->PostTask(FROM_HERE, closed_task);
+      message_loop_->PostTask(FROM_HERE, closed_task);
       return;
     }
-    closed_task_.reset(closed_task);
-    closed_ = true;
   }
 
-  message_loop()->PostTask(
-      FROM_HERE, NewRunnableMethod(this, &JingleClient::DoClose));
+  message_loop_->PostTask(
+      FROM_HERE, NewRunnableMethod(this, &JingleClient::DoClose, closed_task));
 }
 
-void JingleClient::DoClose() {
-  DCHECK_EQ(message_loop(), MessageLoop::current());
-  DCHECK(closed_);
+void JingleClient::DoClose(const base::Closure& closed_task) {
+  DCHECK_EQ(message_loop_, MessageLoop::current());
 
   session_manager_.reset();
-  signal_strategy_->EndSession();
-  // TODO(ajwong): SignalStrategy should drop all resources at EndSession().
-  signal_strategy_ = NULL;
-
-  if (closed_task_.get()) {
-    closed_task_->Run();
-    closed_task_.reset();
+  if (signal_strategy_) {
+    signal_strategy_->EndSession();
+    // TODO(ajwong): SignalStrategy should drop all resources at EndSession().
+    signal_strategy_ = NULL;
   }
+
+  closed_ = true;
+  closed_task.Run();
 }
 
 std::string JingleClient::GetFullJid() {
@@ -161,7 +154,7 @@ MessageLoop* JingleClient::message_loop() {
 }
 
 cricket::SessionManager* JingleClient::session_manager() {
-  DCHECK_EQ(message_loop(), MessageLoop::current());
+  DCHECK_EQ(message_loop_, MessageLoop::current());
   return session_manager_.get();
 }
 
diff --git a/remoting/jingle_glue/jingle_client.h b/remoting/jingle_glue/jingle_client.h
index 1000c35..d0b140b5 100644
--- a/remoting/jingle_glue/jingle_client.h
+++ b/remoting/jingle_glue/jingle_client.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -65,11 +66,10 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
   // |callback| specifies callback object for the client and must not be NULL.
   void Init();
 
-  // Closes XMPP connection and stops the thread. Must be called before the
-  // object is destroyed. If specified, |closed_task| is executed after the
-  // connection is successfully closed.
-  void Close();
-  void Close(Task* closed_task);
+  // Closes XMPP connection and stops the thread. Must be called
+  // before the object is destroyed. |closed_task| is executed after
+  // the connection is successfully closed.
+  void Close(const base::Closure& closed_task);
 
   // Returns JID with resource ID. Empty string is returned if full JID is not
   // known yet, i.e. authentication hasn't finished.
@@ -95,7 +95,7 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>,
 
   void DoInitialize();
   void DoStartSession();
-  void DoClose();
+  void DoClose(const base::Closure& closed_task);
 
   // Updates current state of the connection. Must be called only in
   // the jingle thread.
diff --git a/remoting/jingle_glue/jingle_client_unittest.cc b/remoting/jingle_glue/jingle_client_unittest.cc
index 9c50678..952aa36 100644
--- a/remoting/jingle_glue/jingle_client_unittest.cc
+++ b/remoting/jingle_glue/jingle_client_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/synchronization/waitable_event.h"
 #include "remoting/jingle_glue/jingle_client.h"
 #include "remoting/jingle_glue/jingle_thread.h"
@@ -66,7 +67,10 @@ TEST_F(JingleClientTest, OnStateChanged) {
       buzz::XmppEngine::STATE_OPENING, &state_changed_event));
   state_changed_event.Wait();
 
-  client_->Close();
+  base::WaitableEvent closed_event(true, false);
+  client_->Close(base::Bind(&base::WaitableEvent::Signal,
+                            base::Unretained(&closed_event)));
+  closed_event.Wait();
 
   thread_.Stop();
 }
@@ -74,7 +78,11 @@ TEST_F(JingleClientTest, OnStateChanged) {
 TEST_F(JingleClientTest, Close) {
   EXPECT_CALL(callback_, OnStateChange(_, _))
       .Times(0);
-  client_->Close();
+  base::WaitableEvent closed_event(true, false);
+  client_->Close(base::Bind(&base::WaitableEvent::Signal,
+                            base::Unretained(&closed_event)));
+  closed_event.Wait();
+
   // Verify that the channel doesn't call callback anymore.
   thread_.message_loop()->PostTask(FROM_HERE, NewRunnableFunction(
       &JingleClientTest::ChangeState, signal_strategy_.get(),
@@ -85,19 +93,16 @@ TEST_F(JingleClientTest, Close) {
 
 TEST_F(JingleClientTest, ClosedTask) {
   bool closed = false;
-  client_->Close(NewRunnableFunction(&JingleClientTest::OnClosed,
-                                     &closed));
+  client_->Close(base::Bind(&JingleClientTest::OnClosed, &closed));
   thread_.Stop();
   EXPECT_TRUE(closed);
 }
 
 TEST_F(JingleClientTest, DoubleClose) {
   bool closed1 = false;
-  client_->Close(NewRunnableFunction(&JingleClientTest::OnClosed,
-                                     &closed1));
+  client_->Close(base::Bind(&JingleClientTest::OnClosed, &closed1));
   bool closed2 = false;
-  client_->Close(NewRunnableFunction(&JingleClientTest::OnClosed,
-                                     &closed2));
+  client_->Close(base::Bind(&JingleClientTest::OnClosed, &closed2));
   thread_.Stop();
   EXPECT_TRUE(closed1 && closed2);
 }