Make sure that Clipboard operations that require dispatching

of windows messages are performed on the UI thread.

SetClipboardData requires the clipboard to be open with a handle
to a window that will be notified when the contents are going to
change again. If Windows messages are not processed, any other app
writing to the clipboard will be locked while we acknowledge their
request (by processing the message).

The IO thread doesn't pump windows messages anymore so write
clipboard operations cannot be performed from that thread and have
to be posted to another thread.

BUG=5823

Review URL: http://codereview.chromium.org/19733

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

5 files changed, 106 insertions, 19 deletions

diff --git a/base/clipboard.h b/base/clipboard.h
index 64f9e3a..abaaa1b 100644
--- a/base/clipboard.h
+++ b/base/clipboard.h
@@ -130,6 +130,11 @@ class Clipboard {
   static FormatType GetCFHDropFormatType();
   static FormatType GetFileDescriptorFormatType();
   static FormatType GetFileContentFormatZeroType();
+
+  // Duplicates any remote shared memory handle embedded inside |objects| that
+  // was created by |process| so that it can be used by this process.
+  static void DuplicateRemoteHandles(base::ProcessHandle process,
+                                     ObjectMap* objects);
 #endif
 
  private:
@@ -181,6 +186,9 @@ class Clipboard {
 
   // Mark this as mutable so const methods can still do lazy initialization.
   mutable HWND clipboard_owner_;
+
+  // True if we can create a window.
+  bool create_window_;
 #elif defined(OS_LINUX)
   // Data is stored in the |clipboard_data_| map until it is saved to the system
   // clipboard. The Store* functions save data to the |clipboard_data_| map. The
diff --git a/base/clipboard_unittest.cc b/base/clipboard_unittest.cc
index c752b4c..18f5fbc 100644
--- a/base/clipboard_unittest.cc
+++ b/base/clipboard_unittest.cc
@@ -6,12 +6,27 @@
 
 #include "base/basictypes.h"
 #include "base/clipboard.h"
+#include "base/message_loop.h"
 #include "base/scoped_clipboard_writer.h"
 #include "base/string_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
+#if defined(OS_WIN)
+class ClipboardTest : public PlatformTest {
+ protected:  
+  virtual void SetUp() {
+    message_loop_.reset(new MessageLoopForUI());
+  }
+  virtual void TearDown() {
+  }
+
+ private:
+  scoped_ptr<MessageLoop> message_loop_;
+};
+#elif defined(OS_POSIX)
 typedef PlatformTest ClipboardTest;
+#endif  // defined(OS_WIN)
 
 TEST_F(ClipboardTest, ClearTest) {
   Clipboard clipboard;
diff --git a/base/clipboard_win.cc b/base/clipboard_win.cc
index 907a402..865a1307 100644
--- a/base/clipboard_win.cc
+++ b/base/clipboard_win.cc
@@ -12,6 +12,7 @@
 
 #include "base/clipboard_util.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "base/string_util.h"
 
 namespace {
@@ -82,7 +83,7 @@ LRESULT CALLBACK ClipboardOwnerWndProc(HWND hwnd,
                                        LPARAM lparam) {
   LRESULT lresult = 0;
 
-  switch(message) {
+  switch (message) {
   case WM_RENDERFORMAT:
     // This message comes when SetClipboardData was sent a null data handle
     // and now it's come time to put the data on the clipboard.
@@ -122,14 +123,17 @@ HGLOBAL CreateGlobalData(const std::basic_string<charT>& str) {
 
 } // namespace
 
-Clipboard::Clipboard() {
-  // make a dummy HWND to be the clipboard's owner
-  WNDCLASSEX wcex = {0};
-  wcex.cbSize = sizeof(WNDCLASSEX);
-  wcex.lpfnWndProc = ClipboardOwnerWndProc;
-  wcex.hInstance = GetModuleHandle(NULL);
-  wcex.lpszClassName = L"ClipboardOwnerWindowClass";
-  ::RegisterClassEx(&wcex);
+Clipboard::Clipboard() : create_window_(false) {
+  if (MessageLoop::current()->type() == MessageLoop::TYPE_UI) {
+    // Make a dummy HWND to be the clipboard's owner.
+    WNDCLASSEX wcex = {0};
+    wcex.cbSize = sizeof(WNDCLASSEX);
+    wcex.lpfnWndProc = ClipboardOwnerWndProc;
+    wcex.hInstance = GetModuleHandle(NULL);
+    wcex.lpszClassName = L"ClipboardOwnerWindowClass";
+    ::RegisterClassEx(&wcex);
+    create_window_ = true;
+  }
 
   clipboard_owner_ = NULL;
 }
@@ -158,7 +162,7 @@ void Clipboard::WriteObjects(const ObjectMap& objects,
       WriteBitmapFromSharedMemory(&(iter->second[0].front()),
                                   &(iter->second[1].front()),
                                   process);
-    else 
+    else
       DispatchObject(static_cast<ObjectType>(iter->first), iter->second);
   }
 }
@@ -223,6 +227,7 @@ void Clipboard::WriteHyperlink(const char* title_data,
 }
 
 void Clipboard::WriteWebSmartPaste() {
+  DCHECK(clipboard_owner_);
   ::SetClipboardData(GetWebKitSmartPasteFormatType(), NULL);
 }
 
@@ -265,13 +270,15 @@ void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
 void Clipboard::WriteBitmapFromSharedMemory(const char* bitmap_data,
                                             const char* size_data,
                                             base::ProcessHandle process) {
-  const base::SharedMemoryHandle* remote_bitmap_handle =
-      reinterpret_cast<const base::SharedMemoryHandle*>(bitmap_data);
   const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
 
-  base::SharedMemory bitmap(*remote_bitmap_handle, false, process);
+  // bitmap_data has an encoded shared memory object. See
+  // DuplicateRemoteHandles().
+  char* ptr = const_cast<char*>(bitmap_data);
+  scoped_ptr<const base::SharedMemory> bitmap(*
+      reinterpret_cast<const base::SharedMemory**>(ptr));
 
-  // TODO(darin): share data in gfx/bitmap_header.cc somehow
+  // TODO(darin): share data in gfx/bitmap_header.cc somehow.
   BITMAPINFO bm_info = {0};
   bm_info.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
   bm_info.bmiHeader.biWidth = size->width();
@@ -286,7 +293,7 @@ void Clipboard::WriteBitmapFromSharedMemory(const char* bitmap_data,
   // a memcpy.
   HBITMAP source_hbitmap =
       ::CreateDIBSection(dc, &bm_info, DIB_RGB_COLORS, NULL,
-                         bitmap.handle(), 0);
+                         bitmap->handle(), 0);
 
   if (source_hbitmap) {
     // Now we can write the HBITMAP to the clipboard
@@ -364,8 +371,11 @@ void Clipboard::WriteFiles(const char* file_data, size_t file_len) {
 }
 
 void Clipboard::WriteToClipboard(FormatType format, HANDLE handle) {
-  if (handle && !::SetClipboardData(format, handle))
+  DCHECK(clipboard_owner_);
+  if (handle && !::SetClipboardData(format, handle)) {
+    DCHECK(ERROR_CLIPBOARD_NOT_OPEN != GetLastError());
     FreeData(format, handle);
+  }
 }
 
 bool Clipboard::IsFormatAvailable(unsigned int format) const {
@@ -591,6 +601,30 @@ Clipboard::FormatType Clipboard::GetFileContentFormatZeroType() {
 }
 
 // static
+void Clipboard::DuplicateRemoteHandles(base::ProcessHandle process,
+                                       ObjectMap* objects) {
+  for (ObjectMap::iterator iter = objects->begin(); iter != objects->end();
+       ++iter) {
+    if (iter->first == CBF_SMBITMAP) {
+      // There is a shared memory handle encoded on the first ObjectMapParam.
+      // Use it to open a local handle to the memory.
+      char* bitmap_data = &(iter->second[0].front());
+      base::SharedMemoryHandle* remote_bitmap_handle =
+          reinterpret_cast<base::SharedMemoryHandle*>(bitmap_data);
+
+      base::SharedMemory* bitmap = new base::SharedMemory(*remote_bitmap_handle,
+                                                          false, process);
+
+      // We store the object where the remote handle was located so it can
+      // be retrieved by the UI thread (see WriteBitmapFromSharedMemory()).
+      iter->second[0].clear();
+      for (size_t i = 0; i < sizeof(bitmap); i++)
+        iter->second[0].push_back(reinterpret_cast<char*>(&bitmap)[i]);
+    }
+  }
+}
+
+// static
 Clipboard::FormatType Clipboard::GetWebKitSmartPasteFormatType() {
   return ClipboardUtil::GetWebKitSmartPasteFormat()->cfFormat;
 }
@@ -604,7 +638,7 @@ void Clipboard::FreeData(FormatType format, HANDLE data) {
 }
 
 HWND Clipboard::GetClipboardWindow() const {
-  if (!clipboard_owner_) {
+  if (!clipboard_owner_ && create_window_) {
     clipboard_owner_ = ::CreateWindow(L"ClipboardOwnerWindowClass",
                                       L"ClipboardOwnerWindow",
                                       0, 0, 0, 0, 0,
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index 8f5ce19..98c2032 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -984,7 +984,9 @@ TEST_F(FileUtilTest, Contains) {
   data_dir = data_dir.Append(FILE_PATH_LITERAL("FilePathTest"));
 
   // Create a fresh, empty copy of this directory.
-  ASSERT_TRUE(file_util::Delete(data_dir, true));
+  if (file_util::PathExists(data_dir)) {
+    ASSERT_TRUE(file_util::Delete(data_dir, true));
+  }
   ASSERT_TRUE(file_util::CreateDirectory(data_dir));
 
   FilePath foo(data_dir.Append(FILE_PATH_LITERAL("foo")));
diff --git a/chrome/browser/resource_message_filter.cc b/chrome/browser/resource_message_filter.cc
index 63ea64d..dd0dd5a 100644
--- a/chrome/browser/resource_message_filter.cc
+++ b/chrome/browser/resource_message_filter.cc
@@ -62,6 +62,25 @@ class ContextMenuMessageDispatcher : public Task {
   DISALLOW_COPY_AND_ASSIGN(ContextMenuMessageDispatcher);
 };
 
+// Completes a clipboard write initiated by the renderer. The write must be
+// performed on the UI thread because the clipboard service from the IO thread
+// cannot create windows so it cannot be the "owner" of the clipboard's
+// contents.
+class WriteClipboardTask : public Task {
+ public:
+  explicit WriteClipboardTask(Clipboard::ObjectMap* objects)
+      : objects_(objects) {}
+  ~WriteClipboardTask() {}
+
+  void Run() {
+    g_browser_process->clipboard_service()->WriteObjects(*objects_.get());
+  }
+
+ private:
+  scoped_ptr<Clipboard::ObjectMap> objects_;
+};
+
+
 }  // namespace
 
 ResourceMessageFilter::ResourceMessageFilter(
@@ -445,10 +464,19 @@ void ResourceMessageFilter::OnDownloadUrl(const IPC::Message& message,
 
 void ResourceMessageFilter::OnClipboardWriteObjects(
     const Clipboard::ObjectMap& objects) {
+  // We cannot write directly from the IO thread, and cannot service the IPC
+  // on the UI thread. We'll copy the relevant data and get a handle to any
+  // shared memory so it doesn't go away when we resume the renderer, and post
+  // a task to perform the write on the UI thread.
+  Clipboard::ObjectMap* long_living_objects = new Clipboard::ObjectMap(objects); 
+
   // We pass the render_handle_ to assist the clipboard with using shared
   // memory objects. render_handle_ is a handle to the process that would
   // own any shared memory that might be in the object list.
-  GetClipboardService()->WriteObjects(objects, render_handle_);
+  Clipboard::DuplicateRemoteHandles(render_handle_, long_living_objects);
+
+  render_widget_helper_->ui_loop()->PostTask(FROM_HERE,
+      new WriteClipboardTask(long_living_objects));
 }
 
 void ResourceMessageFilter::OnClipboardIsFormatAvailable(unsigned int format,