From 61cd852a6fd0ecc6454a2df1030fa55e580a3d1e Mon Sep 17 00:00:00 2001 From: "dbeam@chromium.org" Date: Fri, 24 May 2013 04:29:15 +0000 Subject: Revert 201955 "Allow multiple base::MessagePumpForUI instances t..." Broke browser_tests on XP Tests (dbg)(4): http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%28dbg%29%284%29/builds/33253 [844:3260:0523/194017:1212046:FATAL:message_window.cc(28)] Check failed: CalledOnValidThread(). Backtrace: base::Histogram::GetCountAndBucketData [0x08408FD1+2262176] base::Histogram::GetCountAndBucketData [0x0827FBBE+651405] base::Histogram::GetCountAndBucketData [0x082E1E60+1053487] base::Histogram::GetCountAndBucketData [0x0825CC46+508181] base::Histogram::GetCountAndBucketData [0x0825BB66+503861] base::Histogram::GetCountAndBucketData [0x0825A534+498179] base::Histogram::GetCountAndBucketData [0x08256C16+483557] base::Histogram::GetCountAndBucketData [0x08256C82+483665] base::Histogram::GetCountAndBucketData [0x08256324+481267] base::Histogram::GetCountAndBucketData [0x081F59AF+85630] base::Histogram::GetCountAndBucketData [0x081F84AC+96635] base::Histogram::GetCountAndBucketData [0x081F5917+85478] base::Histogram::GetCountAndBucketData [0x08249FD3+431266] base::Histogram::GetCountAndBucketData [0x08249255+427812] base::Histogram::GetCountAndBucketData [0x082462CD+415644] base::Histogram::GetCountAndBucketData [0x0831D883+1297746] base::Histogram::GetCountAndBucketData [0x0831D602+1297105] ViewHostMsg_TextInputStateChanged::Read [0x0D3DD6F9+6214944] ViewHostMsg_TextInputStateChanged::Read [0x0D3DD584+6214571] ViewHostMsg_TextInputStateChanged::Read [0x0D33048E+5505717] ViewHostMsg_TextInputStateChanged::Read [0x0D3303FA+5505569] ViewHostMsg_TextInputStateChanged::Read [0x0D32DDEA+5495825] ViewHostMsg_TextInputStateChanged::Read [0x0D32BAC7+5486830] ViewHostMsg_TextInputStateChanged::Read [0x0D3DC9B6+6211549] ViewHostMsg_TextInputStateChanged::Read [0x0D367071+5729944] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03E93429+30896697] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x025BD0DF+4853487] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02701C3F+6183503] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x026EECCB+6105819] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x026EF6DD+6108397] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x026EFEAF+6110399] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x026F673D+6137165] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02702637+6186055] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x026F4FA0+6131120] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x0262E070+5316224] (No symbol) [0x00632BF5] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03E2BD73+30473091] (No symbol) [0x00632975] std::_Init_locks::operator= [0x03F5574F+5791] std::_Init_locks::operator= [0x03F5557F+5327] RegisterWaitForInputIdle [0x7C817077+73] > Allow multiple base::MessagePumpForUI instances to be created simultanenously on Windows. > > The current implementation of base::MessagePumpForUI on Windows registers a window class with a predefined name in order to create a message-only window. The window class is unregistered when base::MessagePumpForUI is deleted. This causes issues when two or more instances of base::MessagePumpForUI are created/destroyed simultanenously on different threads. For instance once thread can unregister the window class right before the other thread is trying to create a window using this class. > > The CL addresses this problem by switching MessageWindow to implement a message-only window. It also moves MessageWindow from remoting/host/win to base/win along with the corresponding unit test. > > MessageWindow registers a uniquely named window class per MessageWindow instance making sure that different MessageWindow objects do not share any resources. In the future this can be optimized further by registering a common window class shared by all MessageWindow objects in a thread-safe manner (by using LazyInstance for example). > > BUG=241939 > > Review URL: https://chromiumcodereview.appspot.com/15261005 TBR=alexeypa@chromium.org Review URL: https://codereview.chromium.org/15973003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201974 0039d316-1c4b-4281-b951-d872f2087c98 --- base/message_pump_win.cc | 60 +++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 23 deletions(-) (limited to 'base/message_pump_win.cc') diff --git a/base/message_pump_win.cc b/base/message_pump_win.cc index e6ac6d0..fb962ca 100644 --- a/base/message_pump_win.cc +++ b/base/message_pump_win.cc @@ -9,6 +9,8 @@ #include "base/debug/trace_event.h" #include "base/message_loop.h" #include "base/metrics/histogram.h" +#include "base/process_util.h" +#include "base/win/wrapped_window_proc.h" namespace { @@ -23,13 +25,12 @@ enum MessageLoopProblems { namespace base { +static const wchar_t kWndClass[] = L"Chrome_MessagePumpWindow"; + // Message sent to get an additional time slice for pumping (processing) another // task (a series of such messages creates a continuous task pump). static const int kMsgHaveWork = WM_USER + 1; -// Used by MessagePumpUI to wake up the thread and check any pending timers. -static const int kTimerId = 1; - //----------------------------------------------------------------------------- // MessagePumpWin public: @@ -95,12 +96,14 @@ int MessagePumpWin::GetCurrentDelay() const { // MessagePumpForUI public: MessagePumpForUI::MessagePumpForUI() - : message_filter_(new MessageFilter), - window_(new win::MessageWindow()) { - CHECK(window_->Create(this)); + : instance_(NULL), + message_filter_(new MessageFilter) { + InitMessageWnd(); } MessagePumpForUI::~MessagePumpForUI() { + DestroyWindow(message_hwnd_); + UnregisterClass(kWndClass, instance_); } void MessagePumpForUI::ScheduleWork() { @@ -108,7 +111,8 @@ void MessagePumpForUI::ScheduleWork() { return; // Someone else continued the pumping. // Make sure the MessagePump does some work for us. - BOOL ret = PostMessage(window_->hwnd(), kMsgHaveWork, 0, 0); + BOOL ret = PostMessage(message_hwnd_, kMsgHaveWork, + reinterpret_cast(this), 0); if (ret) return; // There was room in the Window Message queue. @@ -155,7 +159,8 @@ void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) { // Create a WM_TIMER event that will wake us up to check for any pending // timers (in case we are running within a nested, external sub-pump). - BOOL ret = SetTimer(window_->hwnd(), kTimerId, delay_msec, NULL); + BOOL ret = SetTimer(message_hwnd_, reinterpret_cast(this), + delay_msec, NULL); if (ret) return; // If we can't set timers, we are in big trouble... but cross our fingers for @@ -192,23 +197,18 @@ void MessagePumpForUI::PumpOutPendingPaintMessages() { //----------------------------------------------------------------------------- // MessagePumpForUI private: -bool MessagePumpForUI::HandleMessage(HWND hwnd, - UINT message, - WPARAM wparam, - LPARAM lparam, - LRESULT* result) { +// static +LRESULT CALLBACK MessagePumpForUI::WndProcThunk( + HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) { switch (message) { case kMsgHaveWork: - HandleWorkMessage(); + reinterpret_cast(wparam)->HandleWorkMessage(); break; - case WM_TIMER: - HandleTimerMessage(); + reinterpret_cast(wparam)->HandleTimerMessage(); break; } - - // Do default processing for all messages. - return false; + return DefWindowProc(hwnd, message, wparam, lparam); } void MessagePumpForUI::DoRunLoop() { @@ -249,7 +249,7 @@ void MessagePumpForUI::DoRunLoop() { // don't want to disturb that timer if it is already in flight. However, // if we did do all remaining delayed work, then lets kill the WM_TIMER. if (more_work_is_plausible && delayed_work_time_.is_null()) - KillTimer(window_->hwnd(), kTimerId); + KillTimer(message_hwnd_, reinterpret_cast(this)); if (state_->should_quit) break; @@ -267,6 +267,20 @@ void MessagePumpForUI::DoRunLoop() { } } +void MessagePumpForUI::InitMessageWnd() { + WNDCLASSEX wc = {0}; + wc.cbSize = sizeof(wc); + wc.lpfnWndProc = base::win::WrappedWindowProc; + wc.hInstance = base::GetModuleFromAddress(wc.lpfnWndProc); + wc.lpszClassName = kWndClass; + instance_ = wc.hInstance; + RegisterClassEx(&wc); + + message_hwnd_ = + CreateWindow(kWndClass, 0, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, instance_, 0); + DCHECK(message_hwnd_); +} + void MessagePumpForUI::WaitForWork() { // Wait until a message is available, up to the time needed by the timer // manager to fire the next set of timers. @@ -324,7 +338,7 @@ void MessagePumpForUI::HandleWorkMessage() { } void MessagePumpForUI::HandleTimerMessage() { - KillTimer(window_->hwnd(), kTimerId); + KillTimer(message_hwnd_, reinterpret_cast(this)); // If we are being called outside of the context of Run, then don't do // anything. This could correspond to a MessageBox call or something of @@ -368,7 +382,7 @@ bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) { } // While running our main message pump, we discard kMsgHaveWork messages. - if (msg.message == kMsgHaveWork && msg.hwnd == window_->hwnd()) + if (msg.message == kMsgHaveWork && msg.hwnd == message_hwnd_) return ProcessPumpReplacementMessage(); if (CallMsgFilter(const_cast(&msg), kMessageFilterCode)) @@ -415,7 +429,7 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() { } DCHECK(!have_message || kMsgHaveWork != msg.message || - msg.hwnd != window_->hwnd()); + msg.hwnd != message_hwnd_); // Since we discarded a kMsgHaveWork message, we must update the flag. int old_have_work = InterlockedExchange(&have_work_, 0); -- cgit v1.1