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

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

8 files changed, 14 insertions, 263 deletions

diff --git a/remoting/host/clipboard_win.cc b/remoting/host/clipboard_win.cc
index 419316a4..fb32dd0 100644
--- a/remoting/host/clipboard_win.cc
+++ b/remoting/host/clipboard_win.cc
@@ -13,12 +13,12 @@
 #include "base/string16.h"
 #include "base/threading/platform_thread.h"
 #include "base/utf_string_conversions.h"
+#include "base/win/message_window.h"
 #include "base/win/scoped_hglobal.h"
 #include "base/win/windows_version.h"
 #include "base/win/wrapped_window_proc.h"
 #include "remoting/base/constants.h"
 #include "remoting/base/util.h"
-#include "remoting/host/win/message_window.h"
 #include "remoting/proto/event.pb.h"
 #include "remoting/protocol/clipboard_stub.h"
 
@@ -101,7 +101,7 @@ typedef BOOL (WINAPI RemoveClipboardFormatListenerFn)(HWND);
 namespace remoting {
 
 class ClipboardWin : public Clipboard,
-                     public win::MessageWindow::Delegate {
+                     public base::win::MessageWindow::Delegate {
  public:
   ClipboardWin();
 
@@ -114,7 +114,7 @@ class ClipboardWin : public Clipboard,
  private:
   void OnClipboardUpdate();
 
-  // win::MessageWindow::Delegate interface.
+  // base::win::MessageWindow::Delegate interface.
   virtual bool HandleMessage(HWND hwnd,
                              UINT message,
                              WPARAM wparam,
@@ -126,7 +126,7 @@ class ClipboardWin : public Clipboard,
   RemoveClipboardFormatListenerFn* remove_clipboard_format_listener_;
 
   // Used to subscribe to WM_CLIPBOARDUPDATE messages.
-  scoped_ptr<win::MessageWindow> window_;
+  scoped_ptr<base::win::MessageWindow> window_;
 
   DISALLOW_COPY_AND_ASSIGN(ClipboardWin);
 };
@@ -162,7 +162,7 @@ void ClipboardWin::Start(
     LOG(WARNING) << "AddClipboardFormatListener() is not available.";
   }
 
-  window_.reset(new win::MessageWindow());
+  window_.reset(new base::win::MessageWindow());
   if (!window_->Create(this)) {
     LOG(ERROR) << "Couldn't create clipboard window.";
     window_.reset();
diff --git a/remoting/host/local_input_monitor_win.cc b/remoting/host/local_input_monitor_win.cc
index 98cd8a4..5d4ce8c 100644
--- a/remoting/host/local_input_monitor_win.cc
+++ b/remoting/host/local_input_monitor_win.cc
@@ -11,8 +11,8 @@
 #include "base/single_thread_task_runner.h"
 #include "base/stringprintf.h"
 #include "base/threading/non_thread_safe.h"
+#include "base/win/message_window.h"
 #include "remoting/host/client_session_control.h"
-#include "remoting/host/win/message_window.h"
 #include "third_party/skia/include/core/SkPoint.h"
 
 namespace remoting {
@@ -37,7 +37,7 @@ class LocalInputMonitorWin : public base::NonThreadSafe,
  private:
   // The actual implementation resides in LocalInputMonitorWin::Core class.
   class Core : public base::RefCountedThreadSafe<Core>,
-               public win::MessageWindow::Delegate {
+               public base::win::MessageWindow::Delegate {
    public:
     Core(scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
          scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
@@ -56,7 +56,7 @@ class LocalInputMonitorWin : public base::NonThreadSafe,
     // Handles WM_INPUT messages.
     LRESULT OnInput(HRAWINPUT input_handle);
 
-    // win::MessageWindow::Delegate interface.
+    // base::win::MessageWindow::Delegate interface.
     virtual bool HandleMessage(HWND hwnd,
                                UINT message,
                                WPARAM wparam,
@@ -70,7 +70,7 @@ class LocalInputMonitorWin : public base::NonThreadSafe,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 
     // Used to receive raw input.
-    scoped_ptr<win::MessageWindow> window_;
+    scoped_ptr<base::win::MessageWindow> window_;
 
     // Points to the object receiving mouse event notifications.
     base::WeakPtr<ClientSessionControl> client_session_control_;
@@ -127,7 +127,7 @@ LocalInputMonitorWin::Core::~Core() {
 void LocalInputMonitorWin::Core::StartOnUiThread() {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
 
-  window_.reset(new win::MessageWindow());
+  window_.reset(new base::win::MessageWindow());
   if (!window_->Create(this)) {
     LOG_GETLASTERROR(ERROR) << "Failed to create the raw input window";
     window_.reset();
diff --git a/remoting/host/win/host_service.cc b/remoting/host/win/host_service.cc
index 42a8ed0..679603a 100644
--- a/remoting/host/win/host_service.cc
+++ b/remoting/host/win/host_service.cc
@@ -385,7 +385,7 @@ int HostService::RunInConsole() {
   }
 
   // Create a window for receiving session change notifications.
-  win::MessageWindow window;
+  base::win::MessageWindow window;
   if (!window.Create(this)) {
     LOG_GETLASTERROR(ERROR)
         << "Failed to create the session notification window";
diff --git a/remoting/host/win/host_service.h b/remoting/host/win/host_service.h
index aa1d9ab..20b290f 100644
--- a/remoting/host/win/host_service.h
+++ b/remoting/host/win/host_service.h
@@ -12,8 +12,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/win/message_window.h"
 #include "net/base/ip_endpoint.h"
-#include "remoting/host/win/message_window.h"
 #include "remoting/host/win/wts_terminal_monitor.h"
 
 class CommandLine;
@@ -28,7 +28,7 @@ class AutoThreadTaskRunner;
 class Stoppable;
 class WtsTerminalObserver;
 
-class HostService : public win::MessageWindow::Delegate,
+class HostService : public base::win::MessageWindow::Delegate,
                     public WtsTerminalMonitor {
  public:
   static HostService* GetInstance();
@@ -70,7 +70,7 @@ class HostService : public win::MessageWindow::Delegate,
   // console application).
   int RunInConsole();
 
-  // win::MessageWindow::Delegate interface.
+  // base::win::MessageWindow::Delegate interface.
   virtual bool HandleMessage(HWND hwnd,
                              UINT message,
                              WPARAM wparam,
diff --git a/remoting/host/win/message_window.cc b/remoting/host/win/message_window.cc
deleted file mode 100644
index b36b3f6..0000000
--- a/remoting/host/win/message_window.cc
+++ /dev/null
@@ -1,118 +0,0 @@
-// Copyright 2013 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 "remoting/host/win/message_window.h"
-
-#include "base/logging.h"
-#include "base/process_util.h"
-#include "base/stringprintf.h"
-#include "base/utf_string_conversions.h"
-#include "base/win/wrapped_window_proc.h"
-
-const char kClassNameFormat[] = "Chromoting_MessageWindow_%p";
-
-namespace remoting {
-namespace win {
-
-MessageWindow::MessageWindow()
-    : atom_(0),
-      instance_(NULL),
-      window_(NULL) {
-  class_name_ = base::StringPrintf(kClassNameFormat, this);
-  instance_ = base::GetModuleFromAddress(static_cast<WNDPROC>(
-      &base::win::WrappedWindowProc<WindowProc>));
-}
-
-MessageWindow::MessageWindow(const std::string& class_name, HINSTANCE instance)
-    : atom_(0),
-      class_name_(class_name),
-      instance_(instance),
-      window_(NULL) {
-}
-
-MessageWindow::~MessageWindow() {
-  DCHECK(CalledOnValidThread());
-
-  if (window_ != NULL) {
-    DestroyWindow(window_);
-    window_ = NULL;
-  }
-
-  if (atom_ != 0) {
-    UnregisterClass(MAKEINTATOM(atom_), instance_);
-    atom_ = 0;
-  }
-}
-
-bool MessageWindow::Create(Delegate* delegate) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!atom_);
-  DCHECK(!window_);
-
-  // Register a separate window class for each instance of |MessageWindow|.
-  string16 class_name = UTF8ToUTF16(class_name_);
-  WNDCLASSEX window_class;
-  window_class.cbSize = sizeof(window_class);
-  window_class.style = 0;
-  window_class.lpfnWndProc = &base::win::WrappedWindowProc<WindowProc>;
-  window_class.cbClsExtra = 0;
-  window_class.cbWndExtra = 0;
-  window_class.hInstance = instance_;
-  window_class.hIcon = NULL;
-  window_class.hCursor = NULL;
-  window_class.hbrBackground = NULL;
-  window_class.lpszMenuName = NULL;
-  window_class.lpszClassName = class_name.c_str();
-  window_class.hIconSm = NULL;
-  atom_ = RegisterClassEx(&window_class);
-  if (atom_ == 0) {
-    LOG_GETLASTERROR(ERROR)
-        << "Failed to register the window class '" << class_name_ << "'";
-    return false;
-  }
-
-  window_ = CreateWindow(MAKEINTATOM(atom_), 0, 0, 0, 0, 0, 0, HWND_MESSAGE, 0,
-                         instance_, delegate);
-  if (!window_) {
-    LOG_GETLASTERROR(ERROR) << "Failed to create a message-only window";
-    return false;
-  }
-
-  return true;
-}
-
-// static
-LRESULT CALLBACK MessageWindow::WindowProc(HWND hwnd,
-                                           UINT message,
-                                           WPARAM wparam,
-                                           LPARAM lparam) {
-  Delegate* delegate = NULL;
-
-  // Set up the delegate before handling WM_CREATE.
-  if (message == WM_CREATE) {
-    CREATESTRUCT* cs = reinterpret_cast<CREATESTRUCT*>(lparam);
-    delegate = reinterpret_cast<Delegate*>(cs->lpCreateParams);
-
-    // Store pointer to the delegate to the window's user data.
-    SetLastError(ERROR_SUCCESS);
-    LONG_PTR result = SetWindowLongPtr(
-        hwnd, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(delegate));
-    CHECK(result != 0 || GetLastError() == ERROR_SUCCESS);
-  } else {
-    delegate = reinterpret_cast<Delegate*>(GetWindowLongPtr(hwnd,
-                                                            GWLP_USERDATA));
-  }
-
-  // Handle the message.
-  if (delegate) {
-    LRESULT message_result;
-    if (delegate->HandleMessage(hwnd, message, wparam, lparam, &message_result))
-      return message_result;
-  }
-
-  return DefWindowProc(hwnd, message, wparam, lparam);
-}
-
-}  // namespace win
-}  // namespace remoting
diff --git a/remoting/host/win/message_window.h b/remoting/host/win/message_window.h
deleted file mode 100644
index ecd2a36..0000000
--- a/remoting/host/win/message_window.h
+++ /dev/null
@@ -1,67 +0,0 @@
-// Copyright 2013 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.
-
-#ifndef REMOTING_HOST_WIN_MESSAGE_WINDOW_H_
-#define REMOTING_HOST_WIN_MESSAGE_WINDOW_H_
-
-#include <windows.h>
-
-#include <string>
-
-#include "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "base/threading/non_thread_safe.h"
-
-namespace remoting {
-namespace win {
-
-// Implements a message-only window.
-class MessageWindow : base::NonThreadSafe {
- public:
-  // Handles incoming window messages.
-  class Delegate {
-   public:
-    virtual ~Delegate() {}
-
-    virtual bool HandleMessage(HWND hwnd,
-                               UINT message,
-                               WPARAM wparam,
-                               LPARAM lparam,
-                               LRESULT* result) = 0;
-  };
-
-  MessageWindow();
-  MessageWindow(const std::string& class_name, HINSTANCE instance);
-  ~MessageWindow();
-
-  // Registers the window class and creates the window. The incoming messages
-  // will be handled by |delegate|. |delegate| must outlive |this|.
-  bool Create(Delegate* delegate);
-
-  HWND hwnd() const { return window_; }
-
- private:
-  // Invoked by the OS to process incoming window messages.
-  static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam,
-                                     LPARAM lparam);
-
-  // Atom representing the registered window class.
-  ATOM atom_;
-
-  // MessageWindow class name.
-  std::string class_name_;
-
-  // Instance of the module containing the window procedure.
-  HINSTANCE instance_;
-
-  // Handle of the input window.
-  HWND window_;
-
-  DISALLOW_COPY_AND_ASSIGN(MessageWindow);
-};
-
-}  // namespace win
-}  // namespace remoting
-
-#endif  // REMOTING_HOST_WIN_MESSAGE_WINDOW_H_
diff --git a/remoting/host/win/message_window_unittest.cc b/remoting/host/win/message_window_unittest.cc
deleted file mode 100644
index d12af7b..0000000
--- a/remoting/host/win/message_window_unittest.cc
+++ /dev/null
@@ -1,61 +0,0 @@
-// 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 "remoting/host/win/message_window.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace remoting {
-
-namespace {
-
-class MessageWindowDelegate : public win::MessageWindow::Delegate {
- public:
-  MessageWindowDelegate();
-  virtual ~MessageWindowDelegate();
-
-  // MessageWindow::Delegate interface.
-  virtual bool HandleMessage(HWND hwnd,
-                             UINT message,
-                             WPARAM wparam,
-                             LPARAM lparam,
-                             LRESULT* result) OVERRIDE;
-};
-
-MessageWindowDelegate::MessageWindowDelegate() {
-}
-
-MessageWindowDelegate::~MessageWindowDelegate() {
-}
-
-bool MessageWindowDelegate::HandleMessage(
-    HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam, LRESULT* result) {
-  // Return |wparam| as the result of WM_USER message.
-  if (message == WM_USER) {
-    *result = wparam;
-    return true;
-  }
-
-  return false;
-}
-
-}  // namespace
-
-// Checks that a window can be created.
-TEST(MessageWindowTest, Create) {
-  MessageWindowDelegate delegate;
-  win::MessageWindow window;
-  EXPECT_TRUE(window.Create(&delegate));
-}
-
-// Verifies that the created window can receive messages.
-TEST(MessageWindowTest, SendMessage) {
-  MessageWindowDelegate delegate;
-  win::MessageWindow window;
-  EXPECT_TRUE(window.Create(&delegate));
-
-  EXPECT_EQ(SendMessage(window.hwnd(), WM_USER, 100, 0), 100);
-}
-
-}  // namespace remoting
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index b883868..5c6eb3c 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -464,8 +464,6 @@
             'host/vlog_net_log.h',
             'host/win/launch_process_with_token.cc',
             'host/win/launch_process_with_token.h',
-            'host/win/message_window.cc',
-            'host/win/message_window.h',
             'host/win/omaha.cc',
             'host/win/omaha.h',
             'host/win/rdp_client.cc',
@@ -2642,7 +2640,6 @@
         'host/setup/pin_validator_unittest.cc',
         'host/token_validator_factory_impl_unittest.cc',
         'host/video_scheduler_unittest.cc',
-        'host/win/message_window_unittest.cc',
         'host/win/rdp_client_unittest.cc',
         'host/win/worker_process_launcher.cc',
         'host/win/worker_process_launcher.h',