summaryrefslogtreecommitdiffstats
path: root/remoting
diff options
context:
space:
mode:
authordbeam@chromium.org <dbeam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-05-24 04:29:15 +0000
committerdbeam@chromium.org <dbeam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-05-24 04:29:15 +0000
commit61cd852a6fd0ecc6454a2df1030fa55e580a3d1e (patch)
tree060ef13df1f6a534b11c2ffc4ce025ac145ee41b /remoting
parentd68bbd5dd956b7e8593e4410d15931ff6f8c1aca (diff)
downloadchromium_src-61cd852a6fd0ecc6454a2df1030fa55e580a3d1e.zip
chromium_src-61cd852a6fd0ecc6454a2df1030fa55e580a3d1e.tar.gz
chromium_src-61cd852a6fd0ecc6454a2df1030fa55e580a3d1e.tar.bz2
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
Diffstat (limited to 'remoting')
-rw-r--r--remoting/host/clipboard_win.cc10
-rw-r--r--remoting/host/local_input_monitor_win.cc10
-rw-r--r--remoting/host/win/host_service.cc2
-rw-r--r--remoting/host/win/host_service.h6
-rw-r--r--remoting/host/win/message_window.cc118
-rw-r--r--remoting/host/win/message_window.h67
-rw-r--r--remoting/host/win/message_window_unittest.cc61
-rw-r--r--remoting/remoting.gyp3
8 files changed, 263 insertions, 14 deletions
diff --git a/remoting/host/clipboard_win.cc b/remoting/host/clipboard_win.cc
index fb32dd0..419316a4 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 base::win::MessageWindow::Delegate {
+ public win::MessageWindow::Delegate {
public:
ClipboardWin();
@@ -114,7 +114,7 @@ class ClipboardWin : public Clipboard,
private:
void OnClipboardUpdate();
- // base::win::MessageWindow::Delegate interface.
+ // 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<base::win::MessageWindow> window_;
+ scoped_ptr<win::MessageWindow> window_;
DISALLOW_COPY_AND_ASSIGN(ClipboardWin);
};
@@ -162,7 +162,7 @@ void ClipboardWin::Start(
LOG(WARNING) << "AddClipboardFormatListener() is not available.";
}
- window_.reset(new base::win::MessageWindow());
+ window_.reset(new 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 5d4ce8c..98cd8a4 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 base::win::MessageWindow::Delegate {
+ public 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);
- // base::win::MessageWindow::Delegate interface.
+ // 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<base::win::MessageWindow> window_;
+ scoped_ptr<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 base::win::MessageWindow());
+ window_.reset(new 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 679603a..42a8ed0 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.
- base::win::MessageWindow window;
+ 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 20b290f..aa1d9ab 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 base::win::MessageWindow::Delegate,
+class HostService : public win::MessageWindow::Delegate,
public WtsTerminalMonitor {
public:
static HostService* GetInstance();
@@ -70,7 +70,7 @@ class HostService : public base::win::MessageWindow::Delegate,
// console application).
int RunInConsole();
- // base::win::MessageWindow::Delegate interface.
+ // 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
new file mode 100644
index 0000000..b36b3f6
--- /dev/null
+++ b/remoting/host/win/message_window.cc
@@ -0,0 +1,118 @@
+// 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
new file mode 100644
index 0000000..ecd2a36
--- /dev/null
+++ b/remoting/host/win/message_window.h
@@ -0,0 +1,67 @@
+// 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
new file mode 100644
index 0000000..d12af7b
--- /dev/null
+++ b/remoting/host/win/message_window_unittest.cc
@@ -0,0 +1,61 @@
+// 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 5c6eb3c..b883868 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -464,6 +464,8 @@
'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',
@@ -2640,6 +2642,7 @@
'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',