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

base/message_pump_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/message_pump_win.h"
 
 #include <math.h>
 
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
@@ skipping 49 matching lines @@
   s.run_depth = state_ ? state_->run_depth + 1 : 1;
 
   RunState* previous_state = state_;
   state_ = &s;
 
   DoRunLoop();
 
   state_ = previous_state;
 }
 
+void MessagePumpWin::Run(Delegate* delegate) {
+  RunWithDispatcher(delegate, NULL);
+}
+
 void MessagePumpWin::Quit() {
   DCHECK(state_);
   state_->should_quit = true;
 }
 
 //-----------------------------------------------------------------------------
 // MessagePumpWin protected:
 
 int MessagePumpWin::GetCurrentDelay() const {
   if (delayed_work_time_.is_null())
@@ skipping 16 matching lines @@
 //-----------------------------------------------------------------------------
 // MessagePumpForUI public:
 
 MessagePumpForUI::MessagePumpForUI()
     : atom_(0),
       message_filter_(new MessageFilter) {
   InitMessageWnd();
 }
 
 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
   // loops only look at Windows Message queues, and don't look at *our* task
   // queues, etc., so we might not get a time slice in such. :-(
   // We could abort here, but the fear is that this failure mode is plausibly
   // common (queue is full, of about 2000 messages), so we'll do a near-graceful
   // recovery.  Nested loops are pretty transient (we think), so this will
   // probably be recoverable.
   InterlockedExchange(&have_work_, 0);  // Clarify that we didn't really insert.
@@ skipping 35 matching lines @@
                       delay_msec, NULL);
   if (ret)
     return;
   // If we can't set timers, we are in big trouble... but cross our fingers for
   // now.
   // TODO(jar): If we don't see this error, use a CHECK() here instead.
   UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", SET_TIMER_ERROR,
                             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.
   if (!state_)
     return;
 
   // Create a mini-message-pump to force immediate processing of only Windows
   // WM_PAINT messages.  Don't provide an infinite loop, but do enough peeking
   // to get the job done.  Actual common max is 4 peeks, but we'll be a little
   // safe here.
@@ skipping 286 matching lines @@
     return;  // Someone else continued the pumping.
 
   // Make sure the MessagePump does some work for us.
   BOOL ret = PostQueuedCompletionStatus(port_, 0,
                                         reinterpret_cast<ULONG_PTR>(this),
                                         reinterpret_cast<OVERLAPPED*>(this));
   if (ret)
     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);
 }
 
 void MessagePumpForIO::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
   // We know that we can't be blocked right now since this method can only be
   // called on the same thread as Run, so we only need to update our record of
   // how long to sleep when we do sleep.
   delayed_work_time_ = delayed_work_time;
 }
 
+void MessagePumpForIO::Stop() {
+}
+
 void MessagePumpForIO::RegisterIOHandler(HANDLE file_handle,
                                          IOHandler* handler) {
   ULONG_PTR key = HandlerToKey(handler, true);
   HANDLE port = CreateIoCompletionPort(file_handle, port_, key, 1);
   DPCHECK(port);
 }
 
 bool MessagePumpForIO::RegisterJobObject(HANDLE job_handle,
                                          IOHandler* handler) {
   // Job object notifications use the OVERLAPPED pointer to carry the message
@@ skipping 174 matching lines @@
 
 // static
 MessagePumpForIO::IOHandler* MessagePumpForIO::KeyToHandler(
     ULONG_PTR key,
     bool* has_valid_io_context) {
   *has_valid_io_context = ((key & 1) == 0);
   return reinterpret_cast<IOHandler*>(key & ~static_cast<ULONG_PTR>(1));
 }
 
 }  // namespace base