Remove unused APIs from the AutoThreadTaskRunner interface.

This CL also re-instates CHECKs for the success of PostTask() calls to the underlying task-runner, to catch cases where the underlying message loop is being stopped prematurely. 

BUG=145856


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

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

4 files changed, 31 insertions, 122 deletions

diff --git a/remoting/base/auto_thread_task_runner.cc b/remoting/base/auto_thread_task_runner.cc
index 6d5f0b5..f155cbc 100644
--- a/remoting/base/auto_thread_task_runner.cc
+++ b/remoting/base/auto_thread_task_runner.cc
@@ -9,45 +9,27 @@
 namespace remoting {
 
 AutoThreadTaskRunner::AutoThreadTaskRunner(
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : task_runner_(task_runner) {
-}
-
-AutoThreadTaskRunner::AutoThreadTaskRunner(
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    const base::Closure& stop_callback)
-    : stop_callback_(stop_callback),
-      task_runner_(task_runner) {
-}
-
-AutoThreadTaskRunner::AutoThreadTaskRunner(
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    scoped_refptr<AutoThreadTaskRunner> parent)
-    : parent_(parent),
-      task_runner_(task_runner) {
-}
-
-AutoThreadTaskRunner::AutoThreadTaskRunner(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    scoped_refptr<AutoThreadTaskRunner> parent,
-    const base::Closure& stop_callback)
-    : parent_(parent),
-      stop_callback_(stop_callback),
+    const base::Closure& stop_task)
+    : stop_task_(stop_task),
       task_runner_(task_runner) {
+  DCHECK(!stop_task_.is_null());
 }
 
 bool AutoThreadTaskRunner::PostDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
-  return task_runner_->PostDelayedTask(from_here, task, delay);
+  CHECK(task_runner_->PostDelayedTask(from_here, task, delay));
+  return true;
 }
 
 bool AutoThreadTaskRunner::PostNonNestableDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
-  return task_runner_->PostNonNestableDelayedTask(from_here, task, delay);
+  CHECK(task_runner_->PostNonNestableDelayedTask(from_here, task, delay));
+  return true;
 }
 
 bool AutoThreadTaskRunner::RunsTasksOnCurrentThread() const {
@@ -55,8 +37,7 @@ bool AutoThreadTaskRunner::RunsTasksOnCurrentThread() const {
 }
 
 AutoThreadTaskRunner::~AutoThreadTaskRunner() {
-  if (!stop_callback_.is_null())
-    stop_callback_.Run();
+  CHECK(task_runner_->PostTask(FROM_HERE, stop_task_));
 }
 
 }  // namespace remoting
diff --git a/remoting/base/auto_thread_task_runner.h b/remoting/base/auto_thread_task_runner.h
index bc4fd7c..ea43521 100644
--- a/remoting/base/auto_thread_task_runner.h
+++ b/remoting/base/auto_thread_task_runner.h
@@ -16,48 +16,21 @@ class MessageLoop;
 namespace remoting {
 
 // A wrapper around |SingleThreadTaskRunner| that provides automatic lifetime
-// management, by invoking a caller-supplied callback when no more references
-// remain, that can stop the wrapped task runner. The caller may also provide a
-// reference to a parent |AutoThreadTaskRunner| to express dependencies between
-// child and parent thread lifetimes.
+// management, by posting a caller-supplied task to the underlying task runner
+// when no more references remain.
 class AutoThreadTaskRunner : public base::SingleThreadTaskRunner {
  public:
   // Constructs an instance of |AutoThreadTaskRunner| wrapping |task_runner|.
-  // |stop_callback| is called (on arbitraty thread) when the last reference to
-  // the object is dropped.
-  explicit AutoThreadTaskRunner(
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+  // |stop_task| is posted to |task_runner| when the last reference to
+  // the AutoThreadTaskRunner is dropped.
   AutoThreadTaskRunner(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-                       const base::Closure& stop_callback);
-
-  // TODO(alexeypa): Remove the |parent| reference once Thread class supports
-  // stopping the thread's message loop and joining the thread separately.
-  // See http://crbug.com/145856.
-  //
-  // Background: currently the only legitimate way of stopping a thread is to
-  // call Thread::Stop() from the same thread that started it. Thread::Stop()
-  // will stop the thread message loop by posting a private task and join
-  // the thread. Thread::Stop() verifies that it is called from the right thread
-  // and that the message loop has been stopped by the private task mentioned
-  // above.
-  //
-  // Due to NPAPI/PPAPI limitations tasks cannot be posted to the main message
-  // loop when the host plugin is being shut down. This presents a challenge
-  // since Thread::Stop() cannot be called from an arbitrary thread. To work
-  // around this we keep a reference to the parent task runner (typically
-  // the one that started the thread) to keep it alive while the worker thread
-  // is in use. Thread::Stop() is called to stop the worker thread when
-  // the parent task runner exits.
-  AutoThreadTaskRunner(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-                       scoped_refptr<AutoThreadTaskRunner> parent);
-  AutoThreadTaskRunner(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-                       scoped_refptr<AutoThreadTaskRunner> parent,
-                       const base::Closure& stop_callback);
+                       const base::Closure& stop_task);
 
   // SingleThreadTaskRunner implementation
-  virtual bool PostDelayedTask(const tracked_objects::Location& from_here,
-                               const base::Closure& task,
-                               base::TimeDelta delay) OVERRIDE;
+  virtual bool PostDelayedTask(
+      const tracked_objects::Location& from_here,
+      const base::Closure& task,
+      base::TimeDelta delay) OVERRIDE;
   virtual bool PostNonNestableDelayedTask(
       const tracked_objects::Location& from_here,
       const base::Closure& task,
@@ -65,16 +38,10 @@ class AutoThreadTaskRunner : public base::SingleThreadTaskRunner {
   virtual bool RunsTasksOnCurrentThread() const OVERRIDE;
 
  private:
-  friend class base::DeleteHelper<AutoThreadTaskRunner>;
-
   virtual ~AutoThreadTaskRunner();
 
-  // An reference to the parent message loop to keep it alive while
-  // |task_runner_| is running.
-  scoped_refptr<AutoThreadTaskRunner> parent_;
-
-  // This callback quits |task_runner_|. It can be run on any thread.
-  base::Closure stop_callback_;
+  // Task posted to |task_runner_| to notify the caller that it may be stopped.
+  base::Closure stop_task_;
 
   // The wrapped task runner.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
diff --git a/remoting/base/auto_thread_task_runner_unittest.cc b/remoting/base/auto_thread_task_runner_unittest.cc
index 07ea8bb..95c5416 100644
--- a/remoting/base/auto_thread_task_runner_unittest.cc
+++ b/remoting/base/auto_thread_task_runner_unittest.cc
@@ -5,19 +5,12 @@
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
-#include "base/threading/thread.h"
 #include "remoting/base/auto_thread_task_runner.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
-void PostQuitTask(MessageLoop* message_loop) {
-  message_loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
-}
-
-void SetFlagTask(
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-  bool* success) {
+void SetFlagTask(bool* success) {
   *success = true;
 }
 
@@ -25,48 +18,20 @@ void SetFlagTask(
 
 namespace remoting {
 
-TEST(AutoThreadTaskRunnerTest, StartAndStopMain) {
+TEST(AutoThreadTaskRunnerTest, StartAndStop) {
   // Create a task runner.
   MessageLoop message_loop;
   scoped_refptr<AutoThreadTaskRunner> task_runner =
       new AutoThreadTaskRunner(
-          message_loop.message_loop_proxy(),
-          base::Bind(&PostQuitTask, &message_loop));
+          message_loop.message_loop_proxy(), MessageLoop::QuitClosure());
 
   // Post a task to make sure it is executed.
   bool success = false;
-  message_loop.PostTask(FROM_HERE, base::Bind(&SetFlagTask, task_runner,
-                                              &success));
+  message_loop.PostTask(FROM_HERE, base::Bind(&SetFlagTask, &success));
 
   task_runner = NULL;
   message_loop.Run();
   EXPECT_TRUE(success);
 }
 
-TEST(AutoThreadTaskRunnerTest, StartAndStopWorker) {
-  // Create a task runner.
-  MessageLoop message_loop;
-  scoped_refptr<AutoThreadTaskRunner> task_runner =
-      new AutoThreadTaskRunner(
-          message_loop.message_loop_proxy(),
-          base::Bind(&PostQuitTask, &message_loop));
-
-  // Create a child task runner.
-  base::Thread thread("Child task runner");
-  thread.Start();
-  task_runner = new AutoThreadTaskRunner(thread.message_loop_proxy(),
-                                         task_runner);
-
-  // Post a task to make sure it is executed.
-  bool success = false;
-  message_loop.PostTask(FROM_HERE, base::Bind(&SetFlagTask, task_runner,
-                                              &success));
-
-  task_runner = NULL;
-  message_loop.Run();
-  EXPECT_TRUE(success);
-
-  thread.Stop();
-}
-
 }  // namespace remoting
diff --git a/remoting/host/win/host_service.cc b/remoting/host/win/host_service.cc
index 0b7a114..3dfdf02 100644
--- a/remoting/host/win/host_service.cc
+++ b/remoting/host/win/host_service.cc
@@ -22,7 +22,7 @@
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/wrapped_window_proc.h"
-#include "remoting/base/auto_thread_task_runner.h"
+#include "remoting/base/auto_thread.h"
 #include "remoting/base/breakpad.h"
 #include "remoting/base/scoped_sc_handle_win.h"
 #include "remoting/base/stoppable.h"
@@ -87,10 +87,6 @@ void usage(const FilePath& program_name) {
                             UTF16ToWide(program_name.value()).c_str());
 }
 
-void QuitMessageLoop(MessageLoop* message_loop) {
-  message_loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
-}
-
 }  // namespace
 
 namespace remoting {
@@ -224,15 +220,15 @@ void HostService::CreateLauncher(
 
 void HostService::RunMessageLoop(MessageLoop* message_loop) {
   // Launch the I/O thread.
-  base::Thread io_thread(kIoThreadName);
-  base::Thread::Options io_thread_options(MessageLoop::TYPE_IO, 0);
-  if (!io_thread.StartWithOptions(io_thread_options)) {
+  scoped_refptr<AutoThreadTaskRunner> io_thread =
+      AutoThread::CreateWithType(kIoThreadName, main_task_runner_,
+                                 MessageLoop::TYPE_IO);
+  if (!io_thread) {
     LOG(FATAL) << "Failed to start the I/O thread";
     return;
   }
 
-  CreateLauncher(new AutoThreadTaskRunner(io_thread.message_loop_proxy(),
-                                          main_task_runner_));
+  CreateLauncher(io_thread);
 
   // Run the service.
   message_loop->Run();
@@ -294,7 +290,7 @@ void HostService::RunAsServiceImpl() {
   // reference is dropped QuitClosure() will be posted to the loop.
   main_task_runner_ =
       new AutoThreadTaskRunner(message_loop.message_loop_proxy(),
-                               base::Bind(&QuitMessageLoop, &message_loop));
+                               MessageLoop::QuitClosure());
 
   // Register the service control handler.
   service_status_handle_ = RegisterServiceCtrlHandlerExW(
@@ -347,7 +343,7 @@ int HostService::RunInConsole() {
   // reference is dropped, QuitClosure() will be posted to the loop.
   main_task_runner_ =
       new AutoThreadTaskRunner(message_loop.message_loop_proxy(),
-                               base::Bind(&QuitMessageLoop, &message_loop));
+                               MessageLoop::QuitClosure());
 
   int result = kInitializationFailed;