[Sync] Add Stop() method to TaskPump and call it in XmppConnection destructor

XmppConnection doesn't destroy TaskPump immediately, but it must stop
it from pumping tasks immediately.  (See bug for details.)

Add unittests for TaskPump.

BUG=86567
TEST=

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

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

9 files changed, 136 insertions, 4 deletions

diff --git a/jingle/jingle.gyp b/jingle/jingle.gyp
index bbdbc3e..f87558f 100644
--- a/jingle/jingle.gyp
+++ b/jingle/jingle.gyp
@@ -124,6 +124,8 @@
       'sources': [
         'notifier/base/fake_base_task.cc',
         'notifier/base/fake_base_task.h',
+        'notifier/base/mock_task.cc',
+        'notifier/base/mock_task.h',
       ],
       'dependencies': [
         'notifier',
@@ -157,6 +159,7 @@
         'notifier/base/chrome_async_socket_unittest.cc',
         'notifier/base/fake_ssl_client_socket_unittest.cc',
         'notifier/base/proxy_resolving_client_socket_unittest.cc',
+        'notifier/base/task_pump_unittest.cc',
         'notifier/base/xmpp_connection_unittest.cc',
         'notifier/base/weak_xmpp_client_unittest.cc',
         'notifier/communicator/xmpp_connection_generator_unittest.cc',
diff --git a/jingle/notifier/base/mock_task.cc b/jingle/notifier/base/mock_task.cc
new file mode 100644
index 0000000..0c398cd
--- /dev/null
+++ b/jingle/notifier/base/mock_task.cc
@@ -0,0 +1,13 @@
+// Copyright (c) 2011 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 "jingle/notifier/base/mock_task.h"
+
+namespace notifier {
+
+MockTask::MockTask(TaskParent* parent) : talk_base::Task(parent) {}
+
+MockTask::~MockTask() {}
+
+}  // namespace notifier
diff --git a/jingle/notifier/base/mock_task.h b/jingle/notifier/base/mock_task.h
new file mode 100644
index 0000000..bb582da
--- /dev/null
+++ b/jingle/notifier/base/mock_task.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2011 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.
+//
+// A mock of talk_base::Task.
+
+#ifndef JINGLE_NOTIFIER_MOCK_TASK_H_
+#define JINGLE_NOTIFIER_MOCK_TASK_H_
+#pragma once
+
+#include "talk/base/task.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace notifier {
+
+class MockTask : public talk_base::Task {
+ public:
+  MockTask(TaskParent* parent);
+
+  virtual ~MockTask();
+
+  MOCK_METHOD0(ProcessStart, int());
+};
+
+}  // namespace notifier
+
+#endif  // JINGLE_NOTIFIER_MOCK_TASK_H_
diff --git a/jingle/notifier/base/task_pump.cc b/jingle/notifier/base/task_pump.cc
index 9423ffa..0395314 100644
--- a/jingle/notifier/base/task_pump.cc
+++ b/jingle/notifier/base/task_pump.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -10,7 +10,8 @@ namespace notifier {
 TaskPump::TaskPump()
     : scoped_runnable_method_factory_(
         ALLOW_THIS_IN_INITIALIZER_LIST(this)),
-      posted_wake_(false) {}
+      posted_wake_(false),
+      stopped_(false) {}
 
 TaskPump::~TaskPump() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
@@ -18,7 +19,7 @@ TaskPump::~TaskPump() {
 
 void TaskPump::WakeTasks() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
-  if (!posted_wake_) {
+  if (!stopped_ && !posted_wake_) {
     MessageLoop* current_message_loop = MessageLoop::current();
     CHECK(current_message_loop);
     // Do the requested wake up.
@@ -37,8 +38,15 @@ int64 TaskPump::CurrentTime() {
   return 0;
 }
 
+void TaskPump::Stop() {
+  stopped_ = true;
+}
+
 void TaskPump::CheckAndRunTasks() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
+  if (stopped_) {
+    return;
+  }
   posted_wake_ = false;
   // We shouldn't be using libjingle for timeout tasks, so we should
   // have no timeout tasks at all.
diff --git a/jingle/notifier/base/task_pump.h b/jingle/notifier/base/task_pump.h
index fb0a7eb..958a152 100644
--- a/jingle/notifier/base/task_pump.h
+++ b/jingle/notifier/base/task_pump.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -21,12 +21,17 @@ class TaskPump : public talk_base::TaskRunner {
   virtual void WakeTasks();
   virtual int64 CurrentTime();
 
+  // No tasks will be processed after this is called, even if
+  // WakeTasks() is called.
+  void Stop();
+
  private:
   void CheckAndRunTasks();
 
   base::NonThreadSafe non_thread_safe_;
   ScopedRunnableMethodFactory<TaskPump> scoped_runnable_method_factory_;
   bool posted_wake_;
+  bool stopped_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskPump);
 };
diff --git a/jingle/notifier/base/task_pump_unittest.cc b/jingle/notifier/base/task_pump_unittest.cc
new file mode 100644
index 0000000..0f4e1f4
--- /dev/null
+++ b/jingle/notifier/base/task_pump_unittest.cc
@@ -0,0 +1,50 @@
+// Copyright (c) 2011 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 "jingle/notifier/base/task_pump.h"
+
+#include "base/message_loop.h"
+#include "jingle/notifier/base/mock_task.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace notifier {
+
+namespace {
+
+using ::testing::Return;
+
+class TaskPumpTest : public testing::Test {
+ private:
+  MessageLoop message_loop_;
+};
+
+TEST_F(TaskPumpTest, Basic) {
+  TaskPump task_pump;
+  MockTask* task = new MockTask(&task_pump);
+  // We have to do this since the state enum is protected in
+  // talk_base::Task.
+  const int TASK_STATE_DONE = 2;
+  EXPECT_CALL(*task, ProcessStart()).WillOnce(Return(TASK_STATE_DONE));
+  task->Start();
+
+  MessageLoop::current()->RunAllPending();
+}
+
+TEST_F(TaskPumpTest, Stop) {
+  TaskPump task_pump;
+  MockTask* task = new MockTask(&task_pump);
+  // We have to do this since the state enum is protected in
+  // talk_base::Task.
+  const int TASK_STATE_ERROR = 3;
+  ON_CALL(*task, ProcessStart()).WillByDefault(Return(TASK_STATE_ERROR));
+  EXPECT_CALL(*task, ProcessStart()).Times(0);
+  task->Start();
+
+  task_pump.Stop();
+  MessageLoop::current()->RunAllPending();
+}
+
+}  // namespace
+
+}  // namespace notifier
diff --git a/jingle/notifier/base/xmpp_connection.cc b/jingle/notifier/base/xmpp_connection.cc
index 86138dd..da1b5a1 100644
--- a/jingle/notifier/base/xmpp_connection.cc
+++ b/jingle/notifier/base/xmpp_connection.cc
@@ -76,6 +76,7 @@ XmppConnection::XmppConnection(
 XmppConnection::~XmppConnection() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   ClearClient();
+  task_pump_->Stop();
   MessageLoop* current_message_loop = MessageLoop::current();
   CHECK(current_message_loop);
   // We do this because XmppConnection may get destroyed as a result
diff --git a/jingle/notifier/base/xmpp_connection.h b/jingle/notifier/base/xmpp_connection.h
index e3699cb..278a6a2 100644
--- a/jingle/notifier/base/xmpp_connection.h
+++ b/jingle/notifier/base/xmpp_connection.h
@@ -93,6 +93,7 @@ class XmppConnection : public sigslot::has_slots<> {
   FRIEND_TEST(XmppConnectionTest, Connect);
   FRIEND_TEST(XmppConnectionTest, MultipleConnect);
   FRIEND_TEST(XmppConnectionTest, ConnectThenError);
+  FRIEND_TEST(XmppConnectionTest, TasksDontRunAfterXmppConnectionDestructor);
 
   DISALLOW_COPY_AND_ASSIGN(XmppConnection);
 };
diff --git a/jingle/notifier/base/xmpp_connection_unittest.cc b/jingle/notifier/base/xmpp_connection_unittest.cc
index f142750..3b85363 100644
--- a/jingle/notifier/base/xmpp_connection_unittest.cc
+++ b/jingle/notifier/base/xmpp_connection_unittest.cc
@@ -10,6 +10,8 @@
 #include "base/basictypes.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
+#include "jingle/notifier/base/mock_task.h"
+#include "jingle/notifier/base/task_pump.h"
 #include "jingle/notifier/base/weak_xmpp_client.h"
 #include "net/base/cert_verifier.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -242,4 +244,26 @@ TEST_F(XmppConnectionTest, ConnectThenError) {
   EXPECT_EQ(NULL, weak_ptr.get());
 }
 
+// We don't destroy XmppConnection's task pump on destruction, but it
+// should still not run any more tasks.
+TEST_F(XmppConnectionTest, TasksDontRunAfterXmppConnectionDestructor) {
+  {
+    XmppConnection xmpp_connection(buzz::XmppClientSettings(),
+                                   url_request_context_getter_,
+                                   &mock_xmpp_connection_delegate_, NULL);
+
+    MockTask* task = new MockTask(xmpp_connection.task_pump_.get());
+    // We have to do this since the state enum is protected in
+    // talk_base::Task.
+    const int TASK_STATE_ERROR = 3;
+    ON_CALL(*task, ProcessStart())
+        .WillByDefault(Return(TASK_STATE_ERROR));
+    EXPECT_CALL(*task, ProcessStart()).Times(0);
+    task->Start();
+  }
+
+  // This should destroy |task_pump|, but |task| still shouldn't run.
+  message_loop_.RunAllPending();
+}
+
 }  // namespace notifier