Add joinable option to SimpleThread::Options as was just done for Thread.

base/threading/simple_thread_unittest.cc

Index: base/threading/simple_thread_unittest.cc
diff --git a/base/threading/simple_thread_unittest.cc b/base/threading/simple_thread_unittest.cc
index 14dd4591f1819d8e505911cd9f4cb9fb09cd54f2..b22b3eef941531c60c1a3206c88da2aa702f714e 100644
--- a/base/threading/simple_thread_unittest.cc
+++ b/base/threading/simple_thread_unittest.cc
@@ -2,10 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "base/atomic_sequence_num.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/test/gtest_util.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
+#include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -17,11 +23,46 @@ class SetIntRunner : public DelegateSimpleThread::Delegate {
   SetIntRunner(int* ptr, int val) : ptr_(ptr), val_(val) { }
   ~SetIntRunner() override {}
 
+ private:
   void Run() override { *ptr_ = val_; }
 
- private:
   int* ptr_;
   int val_;
+
+  DISALLOW_COPY_AND_ASSIGN(SetIntRunner);
+};
+
+// Signals |started_| when Run() is invoked and waits until |released_| is
+// signaled to return, signaling |done_| before doing so. Useful for tests that
+// care to control Run()'s flow. Note: the WaitableEvents need to be owned by
+// the test as neither the ControlledRunner nor Run() live long enough to
+// fullfil this task in all tests. All tests must wait for |done_| to be
+// signaled before destroying the provided WaitableEvents.
+class ControlledRunner : public DelegateSimpleThread::Delegate {
+ public:
+  explicit ControlledRunner(WaitableEvent* started,

[Lei Zhang, 2016/08/10 21:12:38]

no need for explicit

[gab, 2016/08/11 13:06:18]

Done.

+                            WaitableEvent* released,
+                            WaitableEvent* done)
+      : started_(started), released_(released), done_(done) {}
+
+ private:
+  void Run() override {
+    started_->Signal();
+
+    // The WaitableEvents are guaranteed to outlive Run() but the
+    // ControlledRunner isn't after |started_| is signaled in some death tests,
+    // copy required member state to avoid user-after-frees.

[Lei Zhang, 2016/08/10 21:12:38]

s/user/use/

[gab, 2016/08/11 13:06:18]

Done.

+    WaitableEvent* const released_copy = released_;
+    WaitableEvent* const done_copy = done_;
+    released_copy->Wait();
+    done_copy->Signal();
+  }
+
+  WaitableEvent* const started_;
+  WaitableEvent* const released_;
+  WaitableEvent* const done_;
+
+  DISALLOW_COPY_AND_ASSIGN(ControlledRunner);
 };
 
 class WaitEventRunner : public DelegateSimpleThread::Delegate {
@@ -29,22 +70,28 @@ class WaitEventRunner : public DelegateSimpleThread::Delegate {
   explicit WaitEventRunner(WaitableEvent* event) : event_(event) { }
   ~WaitEventRunner() override {}
 
+ private:
   void Run() override {
     EXPECT_FALSE(event_->IsSignaled());
     event_->Signal();
     EXPECT_TRUE(event_->IsSignaled());
   }
- private:
+
   WaitableEvent* event_;
+
+  DISALLOW_COPY_AND_ASSIGN(WaitEventRunner);
 };
 
 class SeqRunner : public DelegateSimpleThread::Delegate {
  public:
   explicit SeqRunner(AtomicSequenceNumber* seq) : seq_(seq) { }
-  void Run() override { seq_->GetNext(); }
 
  private:
+  void Run() override { seq_->GetNext(); }
+
   AtomicSequenceNumber* seq_;
+
+  DISALLOW_COPY_AND_ASSIGN(SeqRunner);
 };
 
 // We count up on a sequence number, firing on the event when we've hit our
@@ -56,6 +103,7 @@ class VerifyPoolRunner : public DelegateSimpleThread::Delegate {
                    int total, WaitableEvent* event)
       : seq_(seq), total_(total), event_(event) { }
 
+ private:
   void Run() override {
     if (seq_->GetNext() == total_) {
       event_->Signal();
@@ -64,10 +112,11 @@ class VerifyPoolRunner : public DelegateSimpleThread::Delegate {
     }
   }
 
- private:
   AtomicSequenceNumber* seq_;
   int total_;
   WaitableEvent* event_;
+
+  DISALLOW_COPY_AND_ASSIGN(VerifyPoolRunner);
 };
 
 }  // namespace
@@ -133,6 +182,88 @@ TEST(SimpleThreadTest, NamedWithOptions) {
             std::string("event_waiter/") + IntToString(thread.tid()));
 }
 
+namespace {
+
+// A test fixture which allows stepping through phases of a non-joinable
+// DelegateSimpleThread. SimpleThreadControlledNonJoinableTest instances will
+// remain alive at least until |done_| is signaled by the ControlledRunner's
+// Run() method.
+class SimpleThreadControlledNonJoinableTest : public testing::Test {
+ public:
+  SimpleThreadControlledNonJoinableTest()
+      : started_(WaitableEvent::ResetPolicy::MANUAL,
+                 WaitableEvent::InitialState::NOT_SIGNALED),
+        released_(WaitableEvent::ResetPolicy::MANUAL,
+                  WaitableEvent::InitialState::NOT_SIGNALED),
+        done_(WaitableEvent::ResetPolicy::MANUAL,
+              WaitableEvent::InitialState::NOT_SIGNALED) {}
+
+  void SetUp() override {
+    runner_ = MakeUnique<ControlledRunner>(&started_, &released_, &done_);
+
+    SimpleThread::Options options;
+    options.joinable = false;
+    thread_ = MakeUnique<DelegateSimpleThread>(runner_.get(), "non_joinable",
+                                               options);
+
+    EXPECT_FALSE(thread_->HasBeenStarted());
+    thread_->Start();
+    EXPECT_TRUE(thread_->HasBeenStarted());
+  }
+
+  void TearDown() override {
+    // Ensure |runner_| has been released and is done with the WaitableEvents
+    // provided to it.
+    ReleaseAndWaitUntilDone();
+  }
+
+  void WaitUntilStarted() { started_.Wait(); }
+
+  void ReleaseAndWaitUntilDone() {
+    released_.Signal();
+    done_.Wait();
+  }
+
+ protected:
+  std::unique_ptr<ControlledRunner> runner_;
+  std::unique_ptr<DelegateSimpleThread> thread_;
+
+ private:
+  WaitableEvent started_;
+  WaitableEvent released_;
+  WaitableEvent done_;
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleThreadControlledNonJoinableTest);
+};
+
+}  // namespace
+
+TEST_F(SimpleThreadControlledNonJoinableTest, DieOnJoin) {
+  EXPECT_FALSE(thread_->HasBeenJoined());
+  EXPECT_DCHECK_DEATH({ thread_->Join(); });
+}
+
+TEST_F(SimpleThreadControlledNonJoinableTest,
+       DeathOnActiveDelegateDestruction) {
+  WaitUntilStarted();
+
+  // Non-joinable DelegateSimpleThread is allowed to go away the minute Run() is
+  // active.
+  thread_.reset();
+
+  // The Delegate can't go away while Run() is in progress however.
+  EXPECT_DCHECK_DEATH({ runner_.reset(); });
+}
+
+TEST_F(SimpleThreadControlledNonJoinableTest,
+       InactiveDelegateDestructionIsOkay) {
+  WaitUntilStarted();
+  ReleaseAndWaitUntilDone();
+  // The Delegate should be safe to be destroyed after its Run() method
+  // completed.
+  runner_.reset();
+}
+
 TEST(SimpleThreadTest, ThreadPool) {
   AtomicSequenceNumber seq;
   SeqRunner runner(&seq);