Linux: move X operations from the IO to UI2 thread.

Currently we perform several X operations on the IO thread including
geometry and clipboard work. This is causing races inside Xlib and
crashing the browser.

These are the result of synchronous calls from the renderer, so we
cannot route these requests to the UI thread without risking deadlock.
Thus we introduce the UI2 thread. This thread has a second connection
to the X server and can perform X operations safely the without UI
thread.

Work remains to be done:

Since we still have the hack where we pass GtkWidget pointers into the
renderer and back, we still have to access these structures from the
IO and UI2 threads. This still needs to be fixed, but this is not the
patch for it.

Also, not all the X calls from the IO thread have been moved over in
this patch; just a few small ones.

http://codereview.chromium.org/67145


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

10 files changed, 208 insertions, 59 deletions

diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index d5758f0..15ead3e 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -96,6 +96,15 @@ class BrowserProcess {
   // database. History has its own thread since it has much higher traffic.
   virtual base::Thread* db_thread() = 0;
 
+#if defined(OS_LINUX)
+  // Returns the thread that is used to process UI requests in cases were
+  // we can't route the request to the UI thread. Note that this thread
+  // should only be used by the IO thread and this method is only safe to call
+  // from the UI thread so, if you've ended up here, something has gone wrong.
+  // This method is only included for uniformity.
+  virtual base::Thread* background_x11_thread() = 0;
+#endif
+
   virtual sandbox::BrokerServices* broker_services() = 0;
 
   virtual IconManager* icon_manager() = 0;
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 5e1d9f3..5b7b6b6 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -171,6 +171,11 @@ BrowserProcessImpl::~BrowserProcessImpl() {
   io_thread_->message_loop()->PostTask(FROM_HERE,
       NewRunnableFunction(chrome_browser_net::EnsureDnsPrefetchShutdown));
 
+#if defined(OS_LINUX)
+  // The IO thread must outlive the BACKGROUND_X11 thread.
+  background_x11_thread_.reset();
+#endif
+
   // Need to stop io_thread_ before resource_dispatcher_host_, since
   // io_thread_ may still deref ResourceDispatcherHost and handle resource
   // request before going away.
@@ -284,6 +289,16 @@ void BrowserProcessImpl::CreateIOThread() {
   // invoke the io_thread() accessor.
   PluginService::GetInstance();
 
+#if defined(OS_LINUX)
+  // The lifetime of the BACKGROUND_X11 thread is a subset of the IO thread so
+  // we start it now.
+  scoped_ptr<base::Thread> background_x11_thread(
+      new BrowserProcessSubThread(ChromeThread::BACKGROUND_X11));
+  if (!background_x11_thread->Start())
+    return;
+  background_x11_thread_.swap(background_x11_thread);
+#endif
+
   scoped_ptr<base::Thread> thread(
       new BrowserProcessSubThread(ChromeThread::IO));
   base::Thread::Options options;
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 55b9dc2..0550ad9 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -73,6 +73,16 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
     return db_thread_.get();
   }
 
+#if defined(OS_LINUX)
+  virtual base::Thread* background_x11_thread() {
+    DCHECK(CalledOnValidThread());
+    // The BACKGROUND_X11 thread is created when the IO thread is created.
+    if (!created_io_thread_)
+      CreateIOThread();
+    return background_x11_thread_.get();
+  }
+#endif
+
   virtual ProfileManager* profile_manager() {
     DCHECK(CalledOnValidThread());
     if (!created_profile_manager_)
@@ -211,6 +221,10 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
 
   bool created_io_thread_;
   scoped_ptr<base::Thread> io_thread_;
+#if defined(OS_LINUX)
+  // This shares a created flag with the IO thread.
+  scoped_ptr<base::Thread> background_x11_thread_;
+#endif
 
   bool created_file_thread_;
   scoped_ptr<base::Thread> file_thread_;
diff --git a/chrome/browser/chrome_thread.cc b/chrome/browser/chrome_thread.cc
index 9c0d10c..762ae76 100644
--- a/chrome/browser/chrome_thread.cc
+++ b/chrome/browser/chrome_thread.cc
@@ -9,6 +9,10 @@ static const char* chrome_thread_names[ChromeThread::ID_COUNT] = {
   "Chrome_IOThread",  // IO
   "Chrome_FileThread",  // FILE
   "Chrome_DBThread",  // DB
+  "Chrome_HistoryThread",  // HISTORY
+#if defined(OS_LINUX)
+  "Chrome_Background_X11Thread",  // BACKGROUND_X11
+#endif
 };
 
 Lock ChromeThread::lock_;
@@ -17,6 +21,10 @@ ChromeThread* ChromeThread::chrome_threads_[ID_COUNT] = {
   NULL,  // IO
   NULL,  // FILE
   NULL,  // DB
+  NULL,  // HISTORY
+#if defined(OS_LINUX)
+  NULL,  // BACKGROUND_X11
+#endif
 };
 
 ChromeThread::ChromeThread(ChromeThread::ID identifier)
diff --git a/chrome/browser/chrome_thread.h b/chrome/browser/chrome_thread.h
index 6570f92..1fd1901 100644
--- a/chrome/browser/chrome_thread.h
+++ b/chrome/browser/chrome_thread.h
@@ -38,6 +38,16 @@ class ChromeThread : public base::Thread {
     // This is the thread that interacts with the database.
     DB,
 
+    // This is the thread that interacts with the history database.
+    HISTORY,
+
+#if defined(OS_LINUX)
+    // This thread has a second connection to the X server and is used to
+    // process UI requests when routing the request to the UI thread would risk
+    // deadlock.
+    BACKGROUND_X11,
+#endif
+
     // This identifier does not represent a thread.  Instead it counts the
     // number of well-known threads.  Insert new well-known threads before this
     // identifier.
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 06433a1..71ae0d8 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -47,13 +47,13 @@
 #include "third_party/WebKit/WebKit/chromium/public/win/WebScreenInfoFactory.h"
 #elif defined(OS_MACOSX)
 #include "third_party/WebKit/WebKit/chromium/public/mac/WebScreenInfoFactory.h"
-#elif defined(OS_LINUX)
-#include "third_party/WebKit/WebKit/chromium/public/gtk/WebScreenInfoFactory.h"
 #endif
 
 using WebKit::WebCache;
 using WebKit::WebScreenInfo;
+#if !defined(OS_LINUX)
 using WebKit::WebScreenInfoFactory;
+#endif
 
 namespace {
 
@@ -213,6 +213,16 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& message) {
   if (!handled) {
     handled = true;
     IPC_BEGIN_MESSAGE_MAP_EX(ResourceMessageFilter, message, msg_is_ok)
+      // On Linux we need to dispatch these messages to the UI2 thread because
+      // we cannot make X calls from the IO thread.  On other platforms, we can
+      // handle these calls directly.
+      IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetScreenInfo,
+                                      OnGetScreenInfo);
+      IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetWindowRect,
+                                      OnGetWindowRect);
+      IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetRootWindowRect,
+                                      OnGetRootWindowRect)
+
       IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWindow, OnMsgCreateWindow)
       IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWidget, OnMsgCreateWidget)
       IPC_MESSAGE_HANDLER(ViewHostMsg_SetCookie, OnSetCookie)
@@ -223,7 +233,6 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& message) {
 #if defined(OS_WIN)  // This hack is Windows-specific.
       IPC_MESSAGE_HANDLER(ViewHostMsg_LoadFont, OnLoadFont)
 #endif
-      IPC_MESSAGE_HANDLER(ViewHostMsg_GetScreenInfo, OnGetScreenInfo)
       IPC_MESSAGE_HANDLER(ViewHostMsg_GetPlugins, OnGetPlugins)
       IPC_MESSAGE_HANDLER(ViewHostMsg_GetPluginPath, OnGetPluginPath)
       IPC_MESSAGE_HANDLER(ViewHostMsg_DownloadUrl, OnDownloadUrl)
@@ -252,8 +261,6 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& message) {
                           OnClipboardReadAsciiText)
       IPC_MESSAGE_HANDLER(ViewHostMsg_ClipboardReadHTML,
                           OnClipboardReadHTML)
-      IPC_MESSAGE_HANDLER(ViewHostMsg_GetWindowRect, OnGetWindowRect)
-      IPC_MESSAGE_HANDLER(ViewHostMsg_GetRootWindowRect, OnGetRootWindowRect)
       IPC_MESSAGE_HANDLER(ViewHostMsg_GetMimeTypeFromExtension,
                           OnGetMimeTypeFromExtension)
       IPC_MESSAGE_HANDLER(ViewHostMsg_GetMimeTypeFromFile,
@@ -460,12 +467,17 @@ void ResourceMessageFilter::OnLoadFont(LOGFONT font) {
 }
 #endif
 
+#if !defined(OS_LINUX)
 void ResourceMessageFilter::OnGetScreenInfo(gfx::NativeViewId view,
-                                            WebScreenInfo* results) {
+                                            IPC::Message* reply_msg) {
   // TODO(darin): Change this into a routed message so that we can eliminate
   // the NativeViewId parameter.
-  *results = WebScreenInfoFactory::screenInfo(gfx::NativeViewFromId(view));
+  WebScreenInfo results =
+      WebScreenInfoFactory::screenInfo(gfx::NativeViewFromId(view));
+  ViewHostMsg_GetScreenInfo::WriteReplyParams(reply_msg, results);
+  Send(reply_msg);
 }
+#endif
 
 void ResourceMessageFilter::OnGetPlugins(bool refresh,
                                          std::vector<WebPluginInfo>* plugins) {
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index 19566be..1ce2b95 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -125,8 +125,7 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnLoadFont(LOGFONT font);
 #endif
 
-  void OnGetScreenInfo(gfx::NativeViewId window,
-                       WebKit::WebScreenInfo* results);
+  void OnGetScreenInfo(gfx::NativeViewId window, IPC::Message* reply);
   void OnGetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins);
   void OnGetPluginPath(const GURL& url,
                        const std::string& mime_type,
@@ -156,8 +155,8 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnClipboardReadText(string16* result);
   void OnClipboardReadAsciiText(std::string* result);
   void OnClipboardReadHTML(string16* markup, GURL* src_url);
-  void OnGetWindowRect(gfx::NativeViewId window, gfx::Rect *rect);
-  void OnGetRootWindowRect(gfx::NativeViewId window, gfx::Rect *rect);
+  void OnGetWindowRect(gfx::NativeViewId window, IPC::Message* reply);
+  void OnGetRootWindowRect(gfx::NativeViewId window, IPC::Message* reply);
   void OnGetMimeTypeFromExtension(const FilePath::StringType& ext,
                                   std::string* mime_type);
   void OnGetMimeTypeFromFile(const FilePath& file_path,
@@ -203,6 +202,13 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnOpenChannelToExtension(const std::string& extension_id, int* port_id);
   void OnExtensionPostMessage(int port_id, const std::string& message);
 
+#if defined(OS_LINUX)
+  void SendBackgroundX11Reply(IPC::Message* reply_msg);
+  void DoOnGetScreenInfo(gfx::NativeViewId view, IPC::Message* reply_msg);
+  void DoOnGetWindowRect(gfx::NativeViewId view, IPC::Message* reply_msg);
+  void DoOnGetRootWindowRect(gfx::NativeViewId view, IPC::Message* reply_msg);
+#endif
+
   // We have our own clipboard service because we want to access the clipboard
   // on the IO thread instead of forwarding (possibly synchronous) messages to
   // the UI thread.
diff --git a/chrome/browser/renderer_host/resource_message_filter_gtk.cc b/chrome/browser/renderer_host/resource_message_filter_gtk.cc
index 0616cd9..750356a 100644
--- a/chrome/browser/renderer_host/resource_message_filter_gtk.cc
+++ b/chrome/browser/renderer_host/resource_message_filter_gtk.cc
@@ -6,42 +6,106 @@
 
 #include <gtk/gtk.h>
 
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/common/render_messages.h"
+#include "chrome/common/x11_util.h"
+
+#include "third_party/WebKit/WebKit/chromium/public/WebScreenInfo.h"
+#include "third_party/WebKit/WebKit/chromium/public/x11/WebScreenInfoFactory.h"
+
+using WebKit::WebScreenInfo;
+using WebKit::WebScreenInfoFactory;
+
 // We get null window_ids passed into the two functions below; please see
 // http://crbug.com/9060 for more details.
 
-void ResourceMessageFilter::OnGetWindowRect(gfx::NativeViewId window_id,
-                                            gfx::Rect* rect) {
-  if (!window_id) {
-    *rect = gfx::Rect();
-    return;
+// Called on the IO thread.
+void ResourceMessageFilter::SendBackgroundX11Reply(IPC::Message* reply_msg) {
+  Send(reply_msg);
+}
+
+// Called on the BACKGROUND_X11 thread.
+void ResourceMessageFilter::DoOnGetScreenInfo(gfx::NativeViewId view,
+                                              IPC::Message* reply_msg) {
+  Display* display = x11_util::GetSecondaryDisplay();
+  int screen = x11_util::GetDefaultScreen(display);
+  WebScreenInfo results = WebScreenInfoFactory::screenInfo(display, screen);
+  ViewHostMsg_GetScreenInfo::WriteReplyParams(reply_msg, results);
+
+  ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::SendBackgroundX11Reply, reply_msg));
+}
+
+// Called on the BACKGROUND_X11 thread.
+void ResourceMessageFilter::DoOnGetWindowRect(gfx::NativeViewId view,
+                                              IPC::Message* reply_msg) {
+  gfx::Rect rect;
+
+  if (view) {
+    XID window = x11_util::GetX11WindowFromGtkWidget(
+        gfx::NativeViewFromId(view));
+
+    int x, y;
+    unsigned width, height;
+    x11_util::GetWindowGeometry(&x, &y, &width, &height, window);
+    rect = gfx::Rect(x, y, width, height);
   }
 
-  // Ideally this would be gtk_widget_get_window but that's only
-  // from gtk 2.14 onwards. :(
-  GdkWindow* window = gfx::NativeViewFromId(window_id)->window;
-  DCHECK(window);
-  gint x, y, width, height;
-  // This is the "position of a window in root window coordinates".
-  gdk_window_get_origin(window, &x, &y);
-  gdk_window_get_size(window, &width, &height);
-  *rect = gfx::Rect(x, y, width, height);
+  ViewHostMsg_GetWindowRect::WriteReplyParams(reply_msg, rect);
+
+  ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::SendBackgroundX11Reply, reply_msg));
 }
 
-void ResourceMessageFilter::OnGetRootWindowRect(gfx::NativeViewId window_id,
-                                                gfx::Rect* rect) {
-  if (!window_id) {
-    *rect = gfx::Rect();
-    return;
+// Called on the BACKGROUND_X11 thread.
+void ResourceMessageFilter::DoOnGetRootWindowRect(gfx::NativeViewId view,
+                                                  IPC::Message* reply_msg) {
+  gfx::Rect rect;
+
+  if (view && gfx::NativeViewFromId(view)->window) {
+    // Windows uses GetAncestor(window, GA_ROOT) here which probably means
+    // we want the top level window.
+    // TODO(agl): calling GTK from this thread is not safe. However, we still
+    // have to solve the issue where we pass GtkWidget* into the renderer and
+    // the solution to that should also fix this problem.
+    GdkWindow* gdk_window =
+        gdk_window_get_toplevel(gfx::NativeViewFromId(view)->window);
+    XID window = x11_util::GetX11WindowFromGdkWindow(gdk_window);
+    int x, y;
+    unsigned width, height;
+    x11_util::GetWindowGeometry(&x, &y, &width, &height, window);
+    rect = gfx::Rect(x, y, width, height);
   }
 
-  // Windows uses GetAncestor(window, GA_ROOT) here which probably means
-  // we want the top level window.
-  GdkWindow* window =
-  gdk_window_get_toplevel(gfx::NativeViewFromId(window_id)->window);
-  DCHECK(window);
-  gint x, y, width, height;
-  // This is the "position of a window in root window coordinates".
-  gdk_window_get_origin(window, &x, &y);
-  gdk_window_get_size(window, &width, &height);
-  *rect = gfx::Rect(x, y, width, height);
+  ViewHostMsg_GetRootWindowRect::WriteReplyParams(reply_msg, rect);
+
+  ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::SendBackgroundX11Reply, reply_msg));
+}
+
+// Called on the IO thread.
+void ResourceMessageFilter::OnGetScreenInfo(gfx::NativeViewId view,
+                                            IPC::Message* reply_msg) {
+  ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::DoOnGetScreenInfo, view, reply_msg));
+}
+
+// Called on the IO thread.
+void ResourceMessageFilter::OnGetWindowRect(gfx::NativeViewId view,
+                                            IPC::Message* reply_msg) {
+  ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::DoOnGetWindowRect, view, reply_msg));
+}
+
+// Called on the IO thread.
+void ResourceMessageFilter::OnGetRootWindowRect(gfx::NativeViewId view,
+                                                IPC::Message* reply_msg) {
+  ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask(
+      FROM_HERE, NewRunnableMethod(
+        this, &ResourceMessageFilter::DoOnGetRootWindowRect, view, reply_msg));
 }
diff --git a/chrome/browser/renderer_host/resource_message_filter_mac.mm b/chrome/browser/renderer_host/resource_message_filter_mac.mm
index c4b7531..7d740ac 100644
--- a/chrome/browser/renderer_host/resource_message_filter_mac.mm
+++ b/chrome/browser/renderer_host/resource_message_filter_mac.mm
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/renderer_host/resource_message_filter.h"
+#include "chrome/common/render_messages.h"
 
 #import <Cocoa/Cocoa.h>
 
@@ -23,28 +24,31 @@ gfx::Rect NSRectToRect(const NSRect rect, NSScreen* screen) {
 // http://crbug.com/9060 for more details.
 
 void ResourceMessageFilter::OnGetWindowRect(gfx::NativeViewId window_id,
-                                            gfx::Rect* rect) {
+                                            IPC::Message* reply_msg) {
   NSView* view = gfx::NativeViewFromId(window_id);
-  if (!view) {
-    *rect = gfx::Rect();
-    return;
+  gfx::Rect rect;
+
+  if (view) {
+    NSRect bounds = [view bounds];
+    bounds = [view convertRect:bounds toView:nil];
+    bounds.origin = [[view window] convertBaseToScreen:bounds.origin];
+    rect = NSRectToRect(bounds, [[view window] screen]);
   }
 
-  NSRect bounds = [view bounds];
-  bounds = [view convertRect:bounds toView:nil];
-  bounds.origin = [[view window] convertBaseToScreen:bounds.origin];
-  *rect = NSRectToRect(bounds, [[view window] screen]);
+  ViewHostMsg_GetWindowRect::WriteReplyParams(reply_msg, rect);
+  Send(reply_msg);
 }
 
 void ResourceMessageFilter::OnGetRootWindowRect(gfx::NativeViewId window_id,
-                                                gfx::Rect* rect) {
+                                                IPC::Message* reply_msg) {
   NSView* view = gfx::NativeViewFromId(window_id);
-  if (!view) {
-    *rect = gfx::Rect();
-    return;
+  gfx::Rect rect;
+  if (view) {
+    NSWindow* window = [view window];
+    NSRect bounds = [window frame];
+    gfx::Rect rect = NSRectToRect(bounds, [window screen]);
   }
 
-  NSWindow* window = [view window];
-  NSRect bounds = [window frame];
-  *rect = NSRectToRect(bounds, [window screen]);
+  ViewHostMsg_GetRootWindowRect::WriteReplyParams(reply_msg, rect);
+  Send(reply_msg);
 }
diff --git a/chrome/browser/renderer_host/resource_message_filter_win.cc b/chrome/browser/renderer_host/resource_message_filter_win.cc
index 222e7ba..2aae6ed 100644
--- a/chrome/browser/renderer_host/resource_message_filter_win.cc
+++ b/chrome/browser/renderer_host/resource_message_filter_win.cc
@@ -3,23 +3,30 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/renderer_host/resource_message_filter.h"
+#include "chrome/common/render_messages.h"
 
 // We get null window_ids passed into the two functions below; please see
 // http://crbug.com/9060 for more details.
 
 void ResourceMessageFilter::OnGetWindowRect(gfx::NativeViewId window_id,
-                                            gfx::Rect* rect) {
+                                            IPC::Message* reply_msg) {
   HWND window = gfx::NativeViewFromId(window_id);
   RECT window_rect = {0};
   GetWindowRect(window, &window_rect);
-  *rect = window_rect;
+  gfx::Rect rect(window_rect);
+
+  ViewHostMsg_GetWindowRect::WriteReplyParams(reply_msg, rect);
+  Send(reply_msg);
 }
 
 void ResourceMessageFilter::OnGetRootWindowRect(gfx::NativeViewId window_id,
-                                                gfx::Rect* rect) {
+                                                IPC::Message* reply_msg) {
   HWND window = gfx::NativeViewFromId(window_id);
   RECT window_rect = {0};
   HWND root_window = ::GetAncestor(window, GA_ROOT);
   GetWindowRect(root_window, &window_rect);
-  *rect = window_rect;
+  gfx::Rect rect(window_rect);
+
+  ViewHostMsg_GetRootWindowRect::WriteReplyParams(reply_msg, rect);
+  Send(reply_msg);
 }