ProcessSingleton now uses base::win::MessageWindow to create a message-only window.

Collateral changes:
- base::win::MessageWindow registes a single window class used by all message-only windows it creates. The class is registered via base::LazyInstance.
- Added base::win::MessageWindow::FindWindow() wrapper used to find other message-only windows, including the other created by a different process.
- Removed chrome::kMessageWindowClass constant.
- chrome_frame_test::ContextMenuTest destroys the clipboard during the test teardown.

Review URL: https://chromiumcodereview.appspot.com/18348025

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

12 files changed, 145 insertions, 123 deletions

diff --git a/base/win/message_window.cc b/base/win/message_window.cc
index 8a9c287..eb5e069 100644
--- a/base/win/message_window.cc
+++ b/base/win/message_window.cc
@@ -4,20 +4,72 @@
 
 #include "base/win/message_window.h"
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/process_util.h"
-#include "base/strings/string16.h"
-#include "base/strings/stringprintf.h"
 #include "base/win/wrapped_window_proc.h"
 
-const wchar_t kClassNameFormat[] = L"Chrome_MessageWindow_%p";
+const wchar_t kMessageWindowClassName[] = L"Chrome_MessageWindow";
 
 namespace base {
 namespace win {
 
-MessageWindow::MessageWindow()
+// Used along with LazyInstance to register a window class for message-only
+// windows created by MessageWindow.
+class MessageWindow::WindowClass {
+ public:
+  WindowClass();
+  ~WindowClass();
+
+  ATOM atom() { return atom_; }
+  HINSTANCE instance() { return instance_; }
+
+ private:
+  ATOM atom_;
+  HINSTANCE instance_;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowClass);
+};
+
+static LazyInstance<MessageWindow::WindowClass> g_window_class =
+    LAZY_INSTANCE_INITIALIZER;
+
+MessageWindow::WindowClass::WindowClass()
     : atom_(0),
-      window_(NULL) {
+      instance_(base::GetModuleFromAddress(&MessageWindow::WindowProc)) {
+  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 = kMessageWindowClassName;
+  window_class.hIconSm = NULL;
+  atom_ = RegisterClassEx(&window_class);
+  if (atom_ == 0) {
+    LOG_GETLASTERROR(ERROR)
+        << "Failed to register the window class for a message-only window";
+  }
+}
+
+MessageWindow::WindowClass::~WindowClass() {
+  if (atom_ != 0) {
+    BOOL result = UnregisterClass(MAKEINTATOM(atom_), instance_);
+    // Hitting this DCHECK usually means that some MessageWindow objects were
+    // leaked. For example not calling
+    // ui::Clipboard::DestroyClipboardForCurrentThread() results in a leaked
+    // MessageWindow.
+    DCHECK(result);
+  }
+}
+
+MessageWindow::MessageWindow()
+    : window_(NULL) {
 }
 
 MessageWindow::~MessageWindow() {
@@ -27,13 +79,6 @@ MessageWindow::~MessageWindow() {
     BOOL result = DestroyWindow(window_);
     DCHECK(result);
   }
-
-  if (atom_ != 0) {
-    BOOL result = UnregisterClass(
-        MAKEINTATOM(atom_),
-        base::GetModuleFromAddress(&MessageWindow::WindowProc));
-    DCHECK(result);
-  }
 }
 
 bool MessageWindow::Create(const MessageCallback& message_callback) {
@@ -45,41 +90,23 @@ bool MessageWindow::CreateNamed(const MessageCallback& message_callback,
   return DoCreate(message_callback, window_name.c_str());
 }
 
+// static
+HWND MessageWindow::FindWindow(const string16& window_name) {
+  return FindWindowEx(HWND_MESSAGE, NULL, kMessageWindowClassName,
+                      window_name.c_str());
+}
+
 bool MessageWindow::DoCreate(const MessageCallback& message_callback,
-                           const wchar_t* window_name) {
+                             const wchar_t* window_name) {
   DCHECK(CalledOnValidThread());
-  DCHECK(!atom_);
   DCHECK(message_callback_.is_null());
   DCHECK(!window_);
 
   message_callback_ = message_callback;
 
-  // Register a separate window class for each instance of |MessageWindow|.
-  string16 class_name = base::StringPrintf(kClassNameFormat, this);
-  HINSTANCE instance = base::GetModuleFromAddress(&MessageWindow::WindowProc);
-
-  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 for a message-only window";
-    return false;
-  }
-
-  window_ = CreateWindow(MAKEINTATOM(atom_), window_name, 0, 0, 0, 0, 0,
-                         HWND_MESSAGE, 0, instance, this);
+  WindowClass& window_class = g_window_class.Get();
+  window_ = CreateWindow(MAKEINTATOM(window_class.atom()), window_name, 0, 0, 0,
+                         0, 0, HWND_MESSAGE, 0, window_class.instance(), this);
   if (!window_) {
     LOG_GETLASTERROR(ERROR) << "Failed to create a message-only window";
     return false;
diff --git a/base/win/message_window.h b/base/win/message_window.h
index 4fd5074..ae0c6f0 100644
--- a/base/win/message_window.h
+++ b/base/win/message_window.h
@@ -20,6 +20,9 @@ namespace win {
 // Implements a message-only window.
 class BASE_EXPORT MessageWindow : public base::NonThreadSafe {
  public:
+  // Used to register a process-wide message window class.
+  class WindowClass;
+
   // Implement this callback to handle messages received by the message window.
   // If the callback returns |false|, the first four parameters are passed to
   // DefWindowProc(). Otherwise, |*result| is returned by the window procedure.
@@ -41,7 +44,14 @@ class BASE_EXPORT MessageWindow : public base::NonThreadSafe {
 
   HWND hwnd() const { return window_; }
 
+  // Retrieves a handle of the first message-only window with matching
+  // |window_name|.
+  static HWND FindWindow(const string16& window_name);
+
  private:
+  // Give |WindowClass| access to WindowProc().
+  friend class WindowClass;
+
   // Contains the actual window creation code.
   bool DoCreate(const MessageCallback& message_callback,
                 const wchar_t* window_name);
@@ -50,9 +60,6 @@ class BASE_EXPORT MessageWindow : public base::NonThreadSafe {
   static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam,
                                      LPARAM lparam);
 
-  // Atom representing the registered window class.
-  ATOM atom_;
-
   // Invoked to handle messages received by the window.
   MessageCallback message_callback_;
 
diff --git a/base/win/message_window_unittest.cc b/base/win/message_window_unittest.cc
index c933ef7..00248bf 100644
--- a/base/win/message_window_unittest.cc
+++ b/base/win/message_window_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/guid.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/message_window.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -31,6 +32,7 @@ TEST(MessageWindowTest, Create) {
   EXPECT_TRUE(window.Create(base::Bind(&HandleMessage)));
 }
 
+// Checks that a named window can be created.
 TEST(MessageWindowTest, CreateNamed) {
   win::MessageWindow window;
   EXPECT_TRUE(window.CreateNamed(base::Bind(&HandleMessage),
@@ -45,4 +47,15 @@ TEST(MessageWindowTest, SendMessage) {
   EXPECT_EQ(SendMessage(window.hwnd(), WM_USER, 100, 0), 100);
 }
 
+// Verifies that a named window can be found by name.
+TEST(MessageWindowTest, FindWindow) {
+  string16 name = UTF8ToUTF16(base::GenerateGUID());
+  win::MessageWindow window;
+  EXPECT_TRUE(window.CreateNamed(base::Bind(&HandleMessage), name));
+
+  HWND hwnd = win::MessageWindow::FindWindow(name);
+  EXPECT_TRUE(hwnd != NULL);
+  EXPECT_EQ(SendMessage(hwnd, WM_USER, 200, 0), 200);
+}
+
 }  // namespace base
diff --git a/chrome/browser/chrome_process_finder_win.cc b/chrome/browser/chrome_process_finder_win.cc
index 34f6761..abd6c2b 100644
--- a/chrome/browser/chrome_process_finder_win.cc
+++ b/chrome/browser/chrome_process_finder_win.cc
@@ -16,6 +16,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/win/message_window.h"
 #include "base/win/metro.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/win_util.h"
@@ -98,8 +99,7 @@ std::string EscapeQueryParamValue(const std::string& text, bool use_plus) {
 namespace chrome {
 
 HWND FindRunningChromeWindow(const base::FilePath& user_data_dir) {
-  return FindWindowEx(HWND_MESSAGE, NULL, chrome::kMessageWindowClass,
-                      user_data_dir.value().c_str());
+  return base::win::MessageWindow::FindWindow(user_data_dir.value());
 }
 
 NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window,
diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h
index 737daba..a584147 100644
--- a/chrome/browser/process_singleton.h
+++ b/chrome/browser/process_singleton.h
@@ -28,6 +28,10 @@
 #include "base/files/scoped_temp_dir.h"
 #endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
 
+#if defined(OS_WIN)
+#include "base/win/message_window.h"
+#endif  // defined(OS_WIN)
+
 class CommandLine;
 
 // ProcessSingleton ----------------------------------------------------------
@@ -84,10 +88,6 @@ class ProcessSingleton : public base::NonThreadSafe {
   // Clear any lock state during shutdown.
   void Cleanup();
 
-#if defined(OS_WIN)
-  LRESULT WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
-#endif
-
 #if defined(OS_LINUX) || defined(OS_OPENBSD)
   static void DisablePromptForTesting();
 #endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
@@ -123,13 +123,10 @@ class ProcessSingleton : public base::NonThreadSafe {
   NotificationCallback notification_callback_;  // Handler for notifications.
 
 #if defined(OS_WIN)
-  // This ugly behemoth handles startup commands sent from another process.
-  LRESULT OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds);
-
   bool EscapeVirtualization(const base::FilePath& user_data_dir);
 
   HWND remote_window_;  // The HWND_MESSAGE of another browser.
-  HWND window_;  // The HWND_MESSAGE window.
+  base::win::MessageWindow window_;  // The message-only window.
   bool is_virtualized_;  // Stuck inside Microsoft Softricity VM environment.
   HANDLE lock_file_;
   base::FilePath user_data_dir_;
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 237004a..94647c7 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -7,6 +7,7 @@
 #include <shellapi.h>
 
 #include "base/base_paths.h"
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
@@ -21,7 +22,6 @@
 #include "base/win/scoped_handle.h"
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
-#include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chrome_process_finder_win.h"
@@ -93,24 +93,6 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) {
   return !*result;
 }
 
-// This function thunks to the object's version of the windowproc, taking in
-// consideration that there are several messages being dispatched before
-// WM_NCCREATE which we let windows handle.
-LRESULT CALLBACK ThunkWndProc(HWND hwnd, UINT message,
-                              WPARAM wparam, LPARAM lparam) {
-  ProcessSingleton* singleton =
-      reinterpret_cast<ProcessSingleton*>(ui::GetWindowUserData(hwnd));
-  if (message == WM_NCCREATE) {
-    CREATESTRUCT* cs = reinterpret_cast<CREATESTRUCT*>(lparam);
-    singleton = reinterpret_cast<ProcessSingleton*>(cs->lpCreateParams);
-    CHECK(singleton);
-    ui::SetWindowUserData(hwnd, singleton);
-  } else if (!singleton) {
-    return ::DefWindowProc(hwnd, message, wparam, lparam);
-  }
-  return singleton->WndProc(hwnd, message, wparam, lparam);
-}
-
 bool ParseCommandLine(const COPYDATASTRUCT* cds,
                       CommandLine* parsed_command_line,
                       base::FilePath* current_directory) {
@@ -171,6 +153,31 @@ bool ParseCommandLine(const COPYDATASTRUCT* cds,
   return false;
 }
 
+bool ProcessLaunchNotification(
+    const ProcessSingleton::NotificationCallback& notification_callback,
+    UINT message,
+    WPARAM wparam,
+    LPARAM lparam,
+    LRESULT* result) {
+  if (message != WM_COPYDATA)
+    return false;
+
+  // Handle the WM_COPYDATA message from another process.
+  HWND hwnd = reinterpret_cast<HWND>(wparam);
+  const COPYDATASTRUCT* cds = reinterpret_cast<COPYDATASTRUCT*>(lparam);
+
+  CommandLine parsed_command_line(CommandLine::NO_PROGRAM);
+  base::FilePath current_directory;
+  if (!ParseCommandLine(cds, &parsed_command_line, &current_directory)) {
+    *result = TRUE;
+    return true;
+  }
+
+  *result = notification_callback.Run(parsed_command_line, current_directory) ?
+      TRUE : FALSE;
+  return true;
+}
+
 // Returns true if Chrome needs to be relaunched into Windows 8 immersive mode.
 // Following conditions apply:-
 // 1. Windows 8 or greater.
@@ -260,20 +267,12 @@ bool ProcessSingleton::EscapeVirtualization(
 ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& notification_callback)
-    : window_(NULL), notification_callback_(notification_callback),
+    : notification_callback_(notification_callback),
       is_virtualized_(false), lock_file_(INVALID_HANDLE_VALUE),
       user_data_dir_(user_data_dir) {
 }
 
 ProcessSingleton::~ProcessSingleton() {
-  // We need to unregister the window as late as possible so that we can detect
-  // another instance of chrome running. Otherwise we may end up writing out
-  // data while a new chrome is starting up.
-  if (window_) {
-    ::DestroyWindow(window_);
-    ::UnregisterClass(chrome::kMessageWindowClass,
-                      base::GetModuleFromAddress(&ThunkWndProc));
-  }
   if (lock_file_ != INVALID_HANDLE_VALUE)
     ::CloseHandle(lock_file_);
 }
@@ -439,22 +438,12 @@ bool ProcessSingleton::Create() {
           << "Lock file can not be created! Error code: " << error;
 
       if (lock_file_ != INVALID_HANDLE_VALUE) {
-        HINSTANCE hinst = base::GetModuleFromAddress(&ThunkWndProc);
-
-        WNDCLASSEX wc = {0};
-        wc.cbSize = sizeof(wc);
-        wc.lpfnWndProc = base::win::WrappedWindowProc<ThunkWndProc>;
-        wc.hInstance = hinst;
-        wc.lpszClassName = chrome::kMessageWindowClass;
-        ATOM clazz = ::RegisterClassEx(&wc);
-        DCHECK(clazz);
-
         // Set the window's title to the path of our user data directory so
         // other Chrome instances can decide if they should forward to us.
-        window_ = ::CreateWindow(MAKEINTATOM(clazz),
-                                 user_data_dir_.value().c_str(),
-                                 0, 0, 0, 0, 0, HWND_MESSAGE, 0, hinst, this);
-        CHECK(window_);
+        bool result = window_.CreateNamed(
+            base::Bind(&ProcessLaunchNotification, notification_callback_),
+            user_data_dir_.value());
+        CHECK(result && window_.hwnd());
       }
 
       if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
@@ -468,30 +457,8 @@ bool ProcessSingleton::Create() {
     }
   }
 
-  return window_ != NULL;
+  return window_.hwnd() != NULL;
 }
 
 void ProcessSingleton::Cleanup() {
 }
-
-LRESULT ProcessSingleton::OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds) {
-  CommandLine parsed_command_line(CommandLine::NO_PROGRAM);
-  base::FilePath current_directory;
-  if (!ParseCommandLine(cds, &parsed_command_line, &current_directory))
-    return TRUE;
-  return notification_callback_.Run(parsed_command_line, current_directory) ?
-      TRUE : FALSE;
-}
-
-LRESULT ProcessSingleton::WndProc(HWND hwnd, UINT message,
-                                  WPARAM wparam, LPARAM lparam) {
-  switch (message) {
-    case WM_COPYDATA:
-      return OnCopyData(reinterpret_cast<HWND>(wparam),
-                        reinterpret_cast<COPYDATASTRUCT*>(lparam));
-    default:
-      break;
-  }
-
-  return ::DefWindowProc(hwnd, message, wparam, lparam);
-}
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 66482f1..a5dda14 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -138,7 +138,6 @@ const base::FilePath::CharType kMetroDriverDll[] = FPL("metro_driver.dll");
 const wchar_t kStatusTrayWindowClass[] = L"Chrome_StatusTrayWindow";
 #endif  // defined(OS_WIN)
 
-const wchar_t kMessageWindowClass[] = L"Chrome_MessageWindow";
 const wchar_t kCrashReportLog[] = L"Reported Crashes.txt";
 const wchar_t kTestingInterfaceDLL[] = L"testing_interface.dll";
 const char    kInitialProfile[] = "Default";
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 1457939..04d6073 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -40,7 +40,6 @@ extern const base::FilePath::CharType* const kHelperFlavorSuffixes[];
 extern const base::FilePath::CharType kMetroDriverDll[];
 extern const wchar_t kStatusTrayWindowClass[];
 #endif  // defined(OS_WIN)
-extern const wchar_t kMessageWindowClass[];
 extern const wchar_t kCrashReportLog[];
 extern const wchar_t kTestingInterfaceDLL[];
 extern const char    kInitialProfile[];
diff --git a/chrome_frame/test/chrome_frame_test_utils.cc b/chrome_frame/test/chrome_frame_test_utils.cc
index 80db1b3..c3c5d4c 100644
--- a/chrome_frame/test/chrome_frame_test_utils.cc
+++ b/chrome_frame/test/chrome_frame_test_utils.cc
@@ -532,6 +532,10 @@ std::wstring GetClipboardText() {
   return UTF16ToWide(text16);
 }
 
+void DestroyClipboard() {
+  ui::Clipboard::DestroyClipboardForCurrentThread();
+}
+
 void SetClipboardText(const std::wstring& text) {
   ui::ScopedClipboardWriter clipboard_writer(
       ui::Clipboard::GetForCurrentThread(),
diff --git a/chrome_frame/test/chrome_frame_test_utils.h b/chrome_frame/test/chrome_frame_test_utils.h
index f0d499f..8960ac6 100644
--- a/chrome_frame/test/chrome_frame_test_utils.h
+++ b/chrome_frame/test/chrome_frame_test_utils.h
@@ -303,6 +303,10 @@ bool AddCFMetaTag(std::string* html_data);
 // Get text data from the clipboard.
 std::wstring GetClipboardText();
 
+// Destroys the clipboard for the current thread. This function must be called
+// if GetClipboardText() or SetClipboardText() have been invoked.
+void DestroyClipboard();
+
 // Puts the given text data on the clipboard. All previous items on the
 // clipboard are removed.
 void SetClipboardText(const std::wstring& text);
diff --git a/chrome_frame/test/ui_test.cc b/chrome_frame/test/ui_test.cc
index 11b270a..f44fec1 100644
--- a/chrome_frame/test/ui_test.cc
+++ b/chrome_frame/test/ui_test.cc
@@ -470,6 +470,11 @@ class ContextMenuTest : public MockIEEventSinkTest, public testing::Test {
     EXPECT_CALL(acc_observer_, OnAccDocLoad(_)).Times(testing::AnyNumber());
   }
 
+  virtual void TearDown() {
+    // Destroy the clipboard here because it is not destroyed automatically.
+    DestroyClipboard();
+  }
+
   // Common helper function for "Save xxx As" tests.
   void DoSaveAsTest(const wchar_t* role, const wchar_t* menu_item_name,
                     const wchar_t* file_ext) {
diff --git a/win8/delegate_execute/command_execute_impl.cc b/win8/delegate_execute/command_execute_impl.cc
index 03fb4da..1bed342 100644
--- a/win8/delegate_execute/command_execute_impl.cc
+++ b/win8/delegate_execute/command_execute_impl.cc
@@ -12,6 +12,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/win/message_window.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_co_mem.h"
 #include "base/win/scoped_handle.h"
@@ -253,9 +254,8 @@ STDMETHODIMP CommandExecuteImpl::GetValue(enum AHE_TYPE* pahe) {
   // New Aura/Ash world we don't want to go throgh FindWindow path
   // and instead take decision based on launch mode.
 #if !defined(USE_AURA)
-  HWND chrome_window = ::FindWindowEx(HWND_MESSAGE, NULL,
-                                      chrome::kMessageWindowClass,
-                                      user_data_dir.value().c_str());
+  HWND chrome_window = base::win::MessageWindow::FindWindow(
+      user_data_dir.value());
   if (chrome_window) {
     AtlTrace("Found chrome window %p\n", chrome_window);
     // The failure cases below are deemed to happen due to the inherently racy