Add a Thread::CleanUpAfterMessageLoopDestruction() method which gets called after the message loop has been destroyed.

BUG=39723
TEST=ThreadTest.CleanUp 
Review URL: http://codereview.chromium.org/1593023

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

3 files changed, 158 insertions, 24 deletions

diff --git a/base/thread.cc b/base/thread.cc
index aeaa0a1..05323a2b 100644
--- a/base/thread.cc
+++ b/base/thread.cc
@@ -35,7 +35,7 @@ struct Thread::StartupData {
         event(false, false) {}
 };
 
-Thread::Thread(const char *name)
+Thread::Thread(const char* name)
     : stopping_(false),
       startup_data_(NULL),
       thread_(0),
@@ -134,34 +134,37 @@ void Thread::Run(MessageLoop* message_loop) {
 }
 
 void Thread::ThreadMain() {
-  // The message loop for this thread.
-  MessageLoop message_loop(startup_data_->options.message_loop_type);
+  {
+    // The message loop for this thread.
+    MessageLoop message_loop(startup_data_->options.message_loop_type);
 
-  // Complete the initialization of our Thread object.
-  thread_id_ = PlatformThread::CurrentId();
-  PlatformThread::SetName(name_.c_str());
-  ANNOTATE_THREAD_NAME(name_.c_str());  // Tell the name to race detector.
-  message_loop.set_thread_name(name_);
-  message_loop_ = &message_loop;
+    // Complete the initialization of our Thread object.
+    thread_id_ = PlatformThread::CurrentId();
+    PlatformThread::SetName(name_.c_str());
+    ANNOTATE_THREAD_NAME(name_.c_str());  // Tell the name to race detector.
+    message_loop.set_thread_name(name_);
+    message_loop_ = &message_loop;
 
-  // Let the thread do extra initialization.
-  // Let's do this before signaling we are started.
-  Init();
+    // Let the thread do extra initialization.
+    // Let's do this before signaling we are started.
+    Init();
 
-  startup_data_->event.Signal();
-  // startup_data_ can't be touched anymore since the starting thread is now
-  // unlocked.
+    startup_data_->event.Signal();
+    // startup_data_ can't be touched anymore since the starting thread is now
+    // unlocked.
 
-  Run(message_loop_);
+    Run(message_loop_);
 
-  // Let the thread do extra cleanup.
-  CleanUp();
+    // Let the thread do extra cleanup.
+    CleanUp();
 
-  // Assert that MessageLoop::Quit was called by ThreadQuitTask.
-  DCHECK(GetThreadWasQuitProperly());
+    // Assert that MessageLoop::Quit was called by ThreadQuitTask.
+    DCHECK(GetThreadWasQuitProperly());
 
-  // We can't receive messages anymore.
-  message_loop_ = NULL;
+    // We can't receive messages anymore.
+    message_loop_ = NULL;
+  }
+  CleanUpAfterMessageLoopDestruction();
   thread_id_ = 0;
 }
 
diff --git a/base/thread.h b/base/thread.h
index 1f2690c..88bff7b 100644
--- a/base/thread.h
+++ b/base/thread.h
@@ -17,6 +17,13 @@ namespace base {
 // the thread.  When this object is destroyed the thread is terminated.  All
 // pending tasks queued on the thread's message loop will run to completion
 // before the thread is terminated.
+//
+// After the thread is stopped, the destruction sequence is:
+//
+//  (1) Thread::CleanUp()
+//  (2) MessageLoop::~MessageLoop
+//  (3.b)    MessageLoop::DestructionObserver::WillDestroyCurrentMessageLoop
+//  (4) Thread::CleanUpAfterMessageLoopDestruction()
 class Thread : PlatformThread::Delegate {
  public:
   struct Options {
@@ -35,7 +42,7 @@ class Thread : PlatformThread::Delegate {
 
   // Constructor.
   // name is a display string to identify the thread.
-  explicit Thread(const char *name);
+  explicit Thread(const char* name);
 
   // Destroys the thread, stopping it if necessary.
   //
@@ -120,6 +127,11 @@ class Thread : PlatformThread::Delegate {
   // Called just after the message loop ends
   virtual void CleanUp() {}
 
+  // Called after the message loop has been deleted. In general clients
+  // should prefer to use CleanUp(). This method is used when code needs to
+  // be run after all of the MessageLoop::DestructionObservers have completed.
+  virtual void CleanUpAfterMessageLoopDestruction() {}
+
   static void SetThreadWasQuitProperly(bool flag);
   static bool GetThreadWasQuitProperly();
 
diff --git a/base/thread_unittest.cc b/base/thread_unittest.cc
index 90aec51..4ceeb80 100644
--- a/base/thread_unittest.cc
+++ b/base/thread_unittest.cc
@@ -2,11 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/thread.h"
+
+#include <vector>
+
 #include "base/dynamic_annotations.h"
 #include "base/lock.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
-#include "base/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -54,6 +57,88 @@ class SleepInsideInitThread : public Thread {
   bool init_called_;
 };
 
+enum ThreadEvent {
+  // Thread::Init() was called.
+  THREAD_EVENT_INIT,
+
+  // The MessageLoop for the thread was deleted.
+  THREAD_EVENT_MESSAGE_LOOP_DESTROYED,
+
+  // Thread::CleanUp() was called.
+  THREAD_EVENT_CLEANUP,
+
+  // Thread::CleanUpAfterMessageLoopDestruction() was called.
+  THREAD_EVENT_CLEANUP_AFTER_LOOP,
+};
+
+typedef std::vector<ThreadEvent> EventList;
+
+class CaptureToEventList : public Thread {
+ public:
+  // This Thread pushes events into the vector |event_list| to show
+  // the order they occured in. |event_list| must remain valid for the
+  // lifetime of this thread.
+  explicit CaptureToEventList(EventList* event_list)
+      : Thread("none"), event_list_(event_list) {
+  }
+
+  virtual ~CaptureToEventList() {
+    // Must call Stop() manually to have our CleanUp() function called.
+    Stop();
+  }
+
+  virtual void Init() {
+    event_list_->push_back(THREAD_EVENT_INIT);
+  }
+
+  virtual void CleanUp() {
+    event_list_->push_back(THREAD_EVENT_CLEANUP);
+  }
+
+  virtual void CleanUpAfterMessageLoopDestruction() {
+    event_list_->push_back(THREAD_EVENT_CLEANUP_AFTER_LOOP);
+  }
+
+ private:
+  EventList* event_list_;
+};
+
+// Observer that writes a value into |event_list| when a message loop has been
+// destroyed.
+class CapturingDestructionObserver : public MessageLoop::DestructionObserver {
+ public:
+  // |event_list| must remain valid throughout the observer's lifetime.
+  explicit CapturingDestructionObserver(EventList* event_list)
+      : event_list_(event_list) {
+  }
+
+  // DestructionObserver implementation:
+  virtual void WillDestroyCurrentMessageLoop() {
+    event_list_->push_back(THREAD_EVENT_MESSAGE_LOOP_DESTROYED);
+    event_list_ = NULL;
+  }
+
+ private:
+  EventList* event_list_;
+};
+
+// Task that adds a destruction observer to the current message loop.
+class RegisterDestructionObserver : public Task {
+ public:
+  explicit RegisterDestructionObserver(
+      MessageLoop::DestructionObserver* observer)
+      : observer_(observer) {
+  }
+
+  virtual void Run() {
+    MessageLoop::current()->AddDestructionObserver(observer_);
+    observer_ = NULL;
+  }
+
+ private:
+  MessageLoop::DestructionObserver* observer_;
+};
+
 }  // namespace
 
 TEST_F(ThreadTest, Restart) {
@@ -141,3 +226,37 @@ TEST_F(ThreadTest, SleepInsideInit) {
   t.Start();
   EXPECT_TRUE(t.InitCalled());
 }
+
+// Make sure that the destruction sequence is:
+//
+//  (1) Thread::CleanUp()
+//  (2) MessageLoop::~MessageLoop()
+//      MessageLoop::DestructionObservers called.
+//  (3) Thread::CleanUpAfterMessageLoopDestruction
+TEST_F(ThreadTest, CleanUp) {
+  EventList captured_events;
+  CapturingDestructionObserver loop_destruction_observer(&captured_events);
+
+  {
+    // Start a thread which writes its event into |captured_events|.
+    CaptureToEventList t(&captured_events);
+    EXPECT_TRUE(t.Start());
+    EXPECT_TRUE(t.message_loop());
+    EXPECT_TRUE(t.IsRunning());
+
+    // Register an observer that writes into |captured_events| once the
+    // thread's message loop is destroyed.
+    t.message_loop()->PostTask(
+        FROM_HERE,
+        new RegisterDestructionObserver(&loop_destruction_observer));
+
+    // Upon leaving this scope, the thread is deleted.
+  }
+
+  // Check the order of events during shutdown.
+  ASSERT_EQ(4u, captured_events.size());
+  EXPECT_EQ(THREAD_EVENT_INIT, captured_events[0]);
+  EXPECT_EQ(THREAD_EVENT_CLEANUP, captured_events[1]);
+  EXPECT_EQ(THREAD_EVENT_MESSAGE_LOOP_DESTROYED, captured_events[2]);
+  EXPECT_EQ(THREAD_EVENT_CLEANUP_AFTER_LOOP, captured_events[3]);
+}