Make sure that the UI window created by base::MessagePumpForUI is destoyed on the same thread (Wind…

base/message_pump_win.cc

Index: base/message_pump_win.cc
diff --git a/base/message_pump_win.cc b/base/message_pump_win.cc
index dcbb3201cd8f57c38d6a3b744727d514559df336..05591241b679b8bead362e1066c679e88fbdf658 100644
--- a/base/message_pump_win.cc
+++ b/base/message_pump_win.cc
@@ -67,6 +67,10 @@ void MessagePumpWin::RunWithDispatcher(
   state_ = previous_state;
 }
 
+void MessagePumpWin::Run(Delegate* delegate) {
+  RunWithDispatcher(delegate, NULL);
+}
+
 void MessagePumpWin::Quit() {
   DCHECK(state_);
   state_->should_quit = true;
@@ -103,20 +107,30 @@ MessagePumpForUI::MessagePumpForUI()
 }
 
 MessagePumpForUI::~MessagePumpForUI() {
-  DestroyWindow(message_hwnd_);
-  UnregisterClass(MAKEINTATOM(atom_),
-                  base::GetModuleFromAddress(&WndProcThunk));
+  DCHECK(!atom_);
+  DCHECK(!message_hwnd_);
 }
 
 void MessagePumpForUI::ScheduleWork() {
   if (InterlockedExchange(&have_work_, 1))
     return;  // Someone else continued the pumping.
 
-  // Make sure the MessagePump does some work for us.
-  BOOL ret = PostMessage(message_hwnd_, kMsgHaveWork,
-                         reinterpret_cast<WPARAM>(this), 0);
-  if (ret)
-    return;  // There was room in the Window Message queue.
+  if (!message_hwnd_lock_.Try())

[darin (slow to review), 2013/06/18 18:28:42]

Please add a comment explaining the design of this code.  The use of .Try() is a
bit subtle.  You should explain what it means for .Try() to fail (i.e., that it
could only fail if we were in the process of shutting down).

[alexeypa (please no reviews), 2013/06/18 19:44:23]

Done.

+    return;
+
+  {
+    base::AutoLock lock(message_hwnd_lock_, base::AutoLock::AlreadyAcquired());
+
+    // Do nothing if the window has been destroyed already.
+    if (!message_hwnd_)
+      return;
+
+    // Make sure the MessagePump does some work for us.
+    if (PostMessage(message_hwnd_, kMsgHaveWork,
+                    reinterpret_cast<WPARAM>(this), 0)) {
+      return;
+    }
+  }
 
   // We have failed to insert a have-work message, so there is a chance that we
   // will starve tasks/timers while sitting in a nested message loop.  Nested
@@ -172,6 +186,25 @@ void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
 
+void MessagePumpForUI::Stop() {
+  HWND message_hwnd;
+  {
+    base::AutoLock lock(message_hwnd_lock_);
+
+    // Let ScheduleWork() know that the window has been destoyed.
+    message_hwnd = message_hwnd_;
+    message_hwnd_ = NULL;
+  }
+
+  // Destoy the window and its class. The destructor checks whether
+  // |message_hwnd_| and |atom_| are destroyed, so the variables should be
+  // cleared here.
+  DestroyWindow(message_hwnd);
+  UnregisterClass(MAKEINTATOM(atom_),
+                  base::GetModuleFromAddress(&WndProcThunk));
+  atom_ = 0;
+}
+
 void MessagePumpForUI::PumpOutPendingPaintMessages() {
   // If we are being called outside of the context of Run, then don't try to do
   // any work.
@@ -478,7 +511,7 @@ void MessagePumpForIO::ScheduleWork() {
     return;  // Post worked perfectly.
 
   // See comment in MessagePumpForUI::ScheduleWork() for this error recovery.
-  InterlockedExchange(&have_work_, 0);  // Clarify that we didn't succeed.
+  InterlockedExchange(&have_work_, 0);
   UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", COMPLETION_POST_ERROR,
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
@@ -490,6 +523,9 @@ void MessagePumpForIO::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
   delayed_work_time_ = delayed_work_time;
 }
 
+void MessagePumpForIO::Stop() {
+}
+
 void MessagePumpForIO::RegisterIOHandler(HANDLE file_handle,
                                          IOHandler* handler) {
   ULONG_PTR key = HandlerToKey(handler, true);