Making sure that base::MessagePumpForUI from different modules are isolated from each other.

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/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/process_util.h"
+#include "base/stringprintf.h"
 #include "base/win/wrapped_window_proc.h"
 
 namespace base {
 
-static const wchar_t kWndClass[] = L"Chrome_MessagePumpWindow";
+static const wchar_t kWndClassFormat[] = L"Chrome_MessagePumpWindow%p";
 
 // 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;
 
 //-----------------------------------------------------------------------------
 // MessagePumpWin public:
 
 void MessagePumpWin::AddObserver(MessagePumpObserver* observer) {
   observers_.AddObserver(observer);
@@ skipping 49 matching lines @@
   int delay = static_cast<int>(timeout);
   if (delay < 0)
     delay = 0;
 
   return delay;
 }
 
 //-----------------------------------------------------------------------------
 // MessagePumpForUI public:
 
-MessagePumpForUI::MessagePumpForUI() {
+MessagePumpForUI::MessagePumpForUI()
+  : atom_(0),
+    instance_(NULL),
+    message_hwnd_(NULL) {
   InitMessageWnd();
 }
 
 MessagePumpForUI::~MessagePumpForUI() {
-  DestroyWindow(message_hwnd_);
-  UnregisterClass(kWndClass, GetModuleHandle(NULL));
+  if (message_hwnd_ != NULL)

[jar (doing other things), 2012/05/17 00:57:24]

nit: you probably don't need this test.  The constructor does a CHECK in
InitMessageWnd()

+    DestroyWindow(message_hwnd_);
+
+  if (atom_ != 0)

[jar (doing other things), 2012/05/17 00:57:24]

nit: The test is not needed since the constructors ensures a non-zero value. You
could DCHECK if you're paranoid, or even CHECK.

+    UnregisterClass(MAKEINTATOM(atom_), instance_);
 }
 
 void MessagePumpForUI::ScheduleWork() {
   if (InterlockedExchange(&have_work_, 1))
     return;  // Someone else continued the pumping.
 
   // Make sure the MessagePump does some work for us.
   PostMessage(message_hwnd_, kMsgHaveWork, reinterpret_cast<WPARAM>(this), 0);
 }
 
@@ skipping 121 matching lines @@
       break;
 
     if (more_work_is_plausible)
       continue;
 
     WaitForWork();  // Wait (sleep) until we have work to do again.
   }
 }
 
 void MessagePumpForUI::InitMessageWnd() {
-  HINSTANCE hinst = GetModuleHandle(NULL);
+  // Register a unique window class for each instance of the UI pump.
+  string16 class_name = base::StringPrintf(kWndClassFormat, this);
 
   WNDCLASSEX wc = {0};
   wc.cbSize = sizeof(wc);
   wc.lpfnWndProc = base::win::WrappedWindowProc<WndProcThunk>;
-  wc.hInstance = hinst;
-  wc.lpszClassName = kWndClass;
-  RegisterClassEx(&wc);
+  instance_ = base::GetModuleFromAddress(static_cast<void*>(wc.lpfnWndProc));

[jar (doing other things), 2012/05/17 02:48:09]

Talking to rvargas, he notes that a "minimal" change would be to only fix this
one line, and all would be well.

You don't need the printf, etc.  That would make this notably smaller.... you
don't need atom... since the class name is constant, etc.

[alexeypa (please no reviews), 2012/05/17 18:30:19]

I guess you are right. Sure such a fix will not give us any additional checks
but we don't really have time, so that should be bake in trunk.

+  wc.hInstance = instance_;
+  wc.lpszClassName = class_name.c_str();
+  atom_ = RegisterClassEx(&wc);
+  CHECK(atom_ != 0);
 
-  message_hwnd_ =
-      CreateWindow(kWndClass, 0, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, hinst, 0);
-  DCHECK(message_hwnd_);
+  message_hwnd_ = CreateWindow(
+      MAKEINTATOM(atom_), 0, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, instance_, 0);
+  CHECK(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.
   int delay = GetCurrentDelay();
   if (delay < 0)  // Negative value means no timers waiting.
     delay = INFINITE;
 
   DWORD result;
@@ skipping 319 matching lines @@
 
 void MessagePumpForIO::WillProcessIOEvent() {
   FOR_EACH_OBSERVER(IOObserver, io_observers_, WillProcessIOEvent());
 }
 
 void MessagePumpForIO::DidProcessIOEvent() {
   FOR_EACH_OBSERVER(IOObserver, io_observers_, DidProcessIOEvent());
 }
 
 }  // namespace base