De-flake the LibDispatchTaskRunner tests.

BUG=165117,165118
TEST=We have to go deeper.

Review URL: https://codereview.chromium.org/11547006

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

3 files changed, 67 insertions, 16 deletions

diff --git a/base/mac/libdispatch_task_runner.cc b/base/mac/libdispatch_task_runner.cc
index 12d2822..4b5abaf 100644
--- a/base/mac/libdispatch_task_runner.cc
+++ b/base/mac/libdispatch_task_runner.cc
@@ -10,7 +10,10 @@ namespace base {
 namespace mac {
 
 LibDispatchTaskRunner::LibDispatchTaskRunner(const char* name)
-    : queue_(dispatch_queue_create(name, NULL)) {
+    : queue_(dispatch_queue_create(name, NULL)),
+      queue_finalized_(false, false) {
+  dispatch_set_context(queue_, this);
+  dispatch_set_finalizer_f(queue_, &LibDispatchTaskRunner::Finalizer);
 }
 
 bool LibDispatchTaskRunner::PostDelayedTask(
@@ -50,12 +53,27 @@ bool LibDispatchTaskRunner::PostNonNestableDelayedTask(
   return PostDelayedTask(from_here, task, delay);
 }
 
+void LibDispatchTaskRunner::Shutdown() {
+  dispatch_release(queue_);
+  queue_ = NULL;
+  queue_finalized_.Wait();
+}
+
 dispatch_queue_t LibDispatchTaskRunner::GetDispatchQueue() const {
   return queue_;
 }
 
 LibDispatchTaskRunner::~LibDispatchTaskRunner() {
-  dispatch_release(queue_);
+  if (queue_) {
+    dispatch_set_context(queue_, NULL);
+    dispatch_set_finalizer_f(queue_, NULL);
+    dispatch_release(queue_);
+  }
+}
+
+void LibDispatchTaskRunner::Finalizer(void* context) {
+  LibDispatchTaskRunner* self = static_cast<LibDispatchTaskRunner*>(context);
+  self->queue_finalized_.Signal();
 }
 
 }  // namespace mac
diff --git a/base/mac/libdispatch_task_runner.h b/base/mac/libdispatch_task_runner.h
index 722b73d..b1d90e2 100644
--- a/base/mac/libdispatch_task_runner.h
+++ b/base/mac/libdispatch_task_runner.h
@@ -8,6 +8,7 @@
 #include <dispatch/dispatch.h>
 
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/waitable_event.h"
 
 namespace base {
 namespace mac {
@@ -48,16 +49,30 @@ class BASE_EXPORT LibDispatchTaskRunner : public base::SingleThreadTaskRunner {
       const Closure& task,
       base::TimeDelta delay) OVERRIDE;
 
+  // This blocks the calling thread until all work on the dispatch queue has
+  // been run and the queue has been destroyed. Destroying a queue requires
+  // ALL retained references to it to be released. Any new tasks posted to
+  // this thread after shutdown are dropped.
+  void Shutdown();
+
   // Returns the dispatch queue associated with this task runner, for use with
   // system APIs that take dispatch queues. The caller is responsible for
   // retaining the result.
+  //
+  // All properties (context, finalizer, etc.) are managed by this class, and
+  // clients should only use the result of this for dispatch_async().
   dispatch_queue_t GetDispatchQueue() const;
 
  protected:
   virtual ~LibDispatchTaskRunner();
 
  private:
+  static void Finalizer(void* context);
+
   dispatch_queue_t queue_;
+
+  // The event on which Shutdown waits until Finalizer runs.
+  base::WaitableEvent queue_finalized_;
 };
 
 }  // namespace mac
diff --git a/base/mac/libdispatch_task_runner_unittest.cc b/base/mac/libdispatch_task_runner_unittest.cc
index abd8c30..c3488d2 100644
--- a/base/mac/libdispatch_task_runner_unittest.cc
+++ b/base/mac/libdispatch_task_runner_unittest.cc
@@ -24,6 +24,7 @@ class LibDispatchTaskRunnerTest : public testing::Test {
         (&message_loop_)->PostTask(FROM_HERE, MessageLoop::QuitClosure());
     });
     message_loop_.Run();
+    task_runner_->Shutdown();
   }
 
   // VerifyTaskOrder takes the expectations from TaskOrderMarkers and compares
@@ -34,7 +35,7 @@ class LibDispatchTaskRunnerTest : public testing::Test {
 
     for (size_t i = 0; i < num_expectations; ++i) {
       if (i >= actual_size) {
-        EXPECT_LT(i, actual_size) << "Expected " << expectations[i];
+        EXPECT_LE(i, actual_size) << "Expected " << expectations[i];
         continue;
       }
 
@@ -126,17 +127,15 @@ TEST_F(LibDispatchTaskRunnerTest, NoMessageLoop) {
   VerifyTaskOrder(expectations, arraysize(expectations));
 }
 
-// This test is flaky, see http://crbug.com/165117.
-TEST_F(LibDispatchTaskRunnerTest, FLAKY_DispatchAndPostTasks) {
+TEST_F(LibDispatchTaskRunnerTest, DispatchAndPostTasks) {
   dispatch_async(task_runner_->GetDispatchQueue(), ^{
       TaskOrderMarker marker(this, "First Block");
-      task_runner_->PostTask(FROM_HERE,
-          BoundRecordTaskOrder(this, "Second Task"));
   });
   task_runner_->PostTask(FROM_HERE, BoundRecordTaskOrder(this, "First Task"));
   dispatch_async(task_runner_->GetDispatchQueue(), ^{
       TaskOrderMarker marker(this, "Second Block");
   });
+  task_runner_->PostTask(FROM_HERE, BoundRecordTaskOrder(this, "Second Task"));
   DispatchLastTask();
 
   const char* const expectations[] = {
@@ -152,24 +151,22 @@ TEST_F(LibDispatchTaskRunnerTest, FLAKY_DispatchAndPostTasks) {
   VerifyTaskOrder(expectations, arraysize(expectations));
 }
 
-// This test is flaky, see http://crbug.com/165118.
-TEST_F(LibDispatchTaskRunnerTest, FLAKY_NonNestable) {
+TEST_F(LibDispatchTaskRunnerTest, NonNestable) {
   task_runner_->PostTask(FROM_HERE, base::BindBlock(^{
       TaskOrderMarker marker(this, "First");
       task_runner_->PostNonNestableTask(FROM_HERE, base::BindBlock(^{
-          TaskOrderMarker marker(this, "Third NonNestable");
+          TaskOrderMarker marker(this, "Second NonNestable");
+          (&message_loop_)->PostTask(FROM_HERE, MessageLoop::QuitClosure());
       }));
   }));
-  task_runner_->PostTask(FROM_HERE, BoundRecordTaskOrder(this, "Second"));
-  DispatchLastTask();
+  message_loop_.Run();
+  task_runner_->Shutdown();
 
   const char* const expectations[] = {
     "BEGIN First",
     "END First",
-    "BEGIN Second",
-    "END Second",
-    "BEGIN Third NonNestable",
-    "END Third NonNestable"
+    "BEGIN Second NonNestable",
+    "END Second NonNestable"
   };
   VerifyTaskOrder(expectations, arraysize(expectations));
 }
@@ -188,6 +185,7 @@ TEST_F(LibDispatchTaskRunnerTest, PostDelayed) {
   }), delta);
   task_runner_->PostTask(FROM_HERE, BoundRecordTaskOrder(this, "Second"));
   message_loop_.Run();
+  task_runner_->Shutdown();
 
   const char* const expectations[] = {
     "BEGIN First",
@@ -201,3 +199,23 @@ TEST_F(LibDispatchTaskRunnerTest, PostDelayed) {
 
   EXPECT_GE(run_time, post_time + delta);
 }
+
+TEST_F(LibDispatchTaskRunnerTest, PostAfterShutdown) {
+  EXPECT_TRUE(task_runner_->PostTask(FROM_HERE,
+      BoundRecordTaskOrder(this, "First")));
+  EXPECT_TRUE(task_runner_->PostTask(FROM_HERE,
+      BoundRecordTaskOrder(this, "Second")));
+  task_runner_->Shutdown();
+  EXPECT_FALSE(task_runner_->PostTask(FROM_HERE, base::BindBlock(^{
+      TaskOrderMarker marker(this, "Not Run");
+      ADD_FAILURE() << "Should not run a task after Shutdown";
+  })));
+
+  const char* const expectations[] = {
+    "BEGIN First",
+    "END First",
+    "BEGIN Second",
+    "END Second"
+  };
+  VerifyTaskOrder(expectations, arraysize(expectations));
+}