Revert 61561 - Create an X window overlay with static ID for GL contexts.

Reverting due to Valgrind issues on Linux tests:

http://build.chromium.org/buildbot/memory/builders/Linux%20Tests%20(valgrind)(2)/builds/7671/steps/memory%20test:%20ui/logs/stdio

Suppression (error hash=#000000007803F359#):
{
   <insert_a_suppression_name_here>
   Memcheck:Addr4
   fun:_ZN20GtkNativeViewManager9OnDestroyEP10_GtkWidget
   fun:_ZN62_GLOBAL__N_gfx_gtk_native_view_id_manager.cc_00000000_E72B30A69OnDestroyEP10_GtkObjectPv
   fun:g_cclosure_marshal_VOID__VOID
   fun:g_closure_invoke
   fun:signal_emit_unlocked_R
   fun:g_signal_emit_valist
   fun:g_signal_emit
   fun:gtk_object_dispose
   fun:gtk_widget_dispose
   fun:g_object_run_dispose
   fun:gtk_object_destroy
   fun:_ZN23RenderWidgetHostViewGtk7DestroyEv
   fun:_ZN16RenderWidgetHost7DestroyEv
   fun:_ZN16RenderWidgetHost8ShutdownEv
   fun:_ZN14RenderViewHost8ShutdownEv
   fun:_ZN21RenderViewHostManagerD1Ev
   fun:_ZN11TabContentsD0Ev
   fun:_ZN13TabStripModel16InternalCloseTabEP11TabContentsib
   fun:_ZN13TabStripModel17InternalCloseTabsERKSt6vectorIiSaIiEEj
   fun:_ZN13TabStripModel18CloseTabContentsAtEij
   fun:_ZN7Browser13CloseContentsEP11TabContents
   fun:_ZN11TabContents5CloseEP14RenderViewHost
}

No absolute smoking gun.  But seems very likely.

--

This code creates an X window that overlays where tab contents are drawn. The handle associated with this window never changes (unlike the X window associated with a GTK widget that is created and destroyed whenever the widget is realized and unrealized respectively).  This is helpful when creating a GL context associated with an X window. Also, having a floating overlay allows us to handle detaching GPU accelerated tabs.

This code seems to have uncovered a race condition within the GPU process. When a tab that uses accelerated compositing is detached (e.g. http://webkit.org/blog/386/3d-transforms/), the tab contents often don't get displayed even though the tab is redrawn.  A slight delay of about 100ms in the GPU process on the call to glXSwapBuffers (in gl_context_linux.cc) fixes this. To witness that GL can draw to the overlay after delay, try it with a tab with constant animation (e.g. http://webkit.org/blog-files/3d-transforms/poster-circle.html).

I've written a bare bones test (about 150 lines of C) of the overlay technique, and it requires no such delay b/w reparenting the glXSwapBuffers. I've also confirmed that there is no synchronization issues w.r.t. the X server between my patch and GL draw commands. I propose filing a new bug for the race, if this patch gets accepted.

BUG=53345,56419
TEST=Open two tabs, one with GPU rendered content (http://webkit.org/blog/386/3d-transforms/). Detach the tab with GPU rendered content.  GPU process shouldn't crash.

Review URL: http://codereview.chromium.org/3509004
Patch from Jonathan Backer <backer@google.com>.

TBR=piman@chromium.org
Review URL: http://codereview.chromium.org/3548015

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

4 files changed, 16 insertions, 170 deletions

diff --git a/chrome/browser/gpu_process_host.cc b/chrome/browser/gpu_process_host.cc
index 8e2242e..f4c6164 100644
--- a/chrome/browser/gpu_process_host.cc
+++ b/chrome/browser/gpu_process_host.cc
@@ -177,7 +177,7 @@ void GpuProcessHost::OnControlMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(GpuHostMsg_GraphicsInfoCollected,
                         OnGraphicsInfoCollected)
 #if defined(OS_LINUX)
-    IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuHostMsg_GetViewXID, OnGetViewXID)
+    IPC_MESSAGE_HANDLER(GpuHostMsg_GetViewXID, OnGetViewXID)
 #elif defined(OS_MACOSX)
     IPC_MESSAGE_HANDLER(GpuHostMsg_AcceleratedSurfaceSetIOSurface,
                         OnAcceleratedSurfaceSetIOSurface)
@@ -210,38 +210,12 @@ void GpuProcessHost::OnGraphicsInfoCollected(const GPUInfo& gpu_info) {
 }
 
 #if defined(OS_LINUX)
-
-namespace {
-
-void SendDelayedReply(IPC::Message* reply_msg) {
-  GpuProcessHost::Get()->Send(reply_msg);
-}
-
-void GetViewXIDDispatcher(gfx::NativeViewId id, IPC::Message* reply_msg) {
-  unsigned long xid;
-
+void GpuProcessHost::OnGetViewXID(gfx::NativeViewId id, unsigned long* xid) {
   GtkNativeViewManager* manager = Singleton<GtkNativeViewManager>::get();
-  if (!manager->GetPermanentXIDForId(&xid, id)) {
+  if (!manager->GetXIDForId(xid, id)) {
     DLOG(ERROR) << "Can't find XID for view id " << id;
-    xid = 0;
+    *xid = 0;
   }
-
-  GpuHostMsg_GetViewXID::WriteReplyParams(reply_msg, xid);
-
-  // Have to reply from IO thread.
-  ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableFunction(&SendDelayedReply, reply_msg));
-}
-
-}
-
-void GpuProcessHost::OnGetViewXID(gfx::NativeViewId id,
-                                  IPC::Message *reply_msg) {
-  // Have to request a permanent overlay from UI thread.
-  ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableFunction(&GetViewXIDDispatcher, id, reply_msg));
 }
 
 #elif defined(OS_MACOSX)
diff --git a/chrome/browser/gpu_process_host.h b/chrome/browser/gpu_process_host.h
index a5268ba..e4d4559 100644
--- a/chrome/browser/gpu_process_host.h
+++ b/chrome/browser/gpu_process_host.h
@@ -95,7 +95,7 @@ class GpuProcessHost : public BrowserChildProcessHost {
   void OnSynchronizeReply();
   void OnGraphicsInfoCollected(const GPUInfo& gpu_info);
 #if defined(OS_LINUX)
-  void OnGetViewXID(gfx::NativeViewId id, IPC::Message* reply_msg);
+  void OnGetViewXID(gfx::NativeViewId id, unsigned long* xid);
 #elif defined(OS_MACOSX)
   void OnAcceleratedSurfaceSetIOSurface(
       const GpuHostMsg_AcceleratedSurfaceSetIOSurface_Params& params);
diff --git a/gfx/gtk_native_view_id_manager.cc b/gfx/gtk_native_view_id_manager.cc
index 51da036..0cb96df 100644
--- a/gfx/gtk_native_view_id_manager.cc
+++ b/gfx/gtk_native_view_id_manager.cc
@@ -4,87 +4,31 @@
 
 #include "gfx/gtk_native_view_id_manager.h"
 
-#include <gtk/gtk.h>
-#include <gdk/gdkx.h>
-#include <gdk/gdk.h>
-#include <X11/Xlib.h>
-
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "gfx/rect.h"
 
+#include <gtk/gtk.h>
+#include <gdk/gdkx.h>
+
 // -----------------------------------------------------------------------------
 // Bounce functions for GTK to callback into a C++ object...
-namespace {
 
-void OnRealize(gfx::NativeView widget, void* arg) {
+static void OnRealize(gfx::NativeView widget, void* arg) {
   GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
   manager->OnRealize(widget);
 }
 
-void OnUnrealize(gfx::NativeView widget, void *arg) {
+static void OnUnrealize(gfx::NativeView widget, void *arg) {
   GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
   manager->OnUnrealize(widget);
 }
 
-void OnDestroy(GtkObject* obj, void* arg) {
+static void OnDestroy(GtkObject* obj, void* arg) {
   GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
   manager->OnDestroy(reinterpret_cast<GtkWidget*>(obj));
 }
 
-void OnSizeAllocate(gfx::NativeView widget,
-                    GtkAllocation* alloc,
-                    void* arg) {
-  GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
-  manager->OnSizeAllocate(widget, alloc);
-}
-
-XID CreateOverlay(XID underlying) {
-  XID overlay = 0;
-  Display* dpy = gdk_x11_get_default_xdisplay();
-
-  // Prevent interference with reparenting/mapping of overlay
-  XSetWindowAttributes attr;
-  attr.override_redirect = True;
-
-  if (underlying) {
-    XID root;
-    int x, y;
-    unsigned int width, height;
-    unsigned int border_width;
-    unsigned int depth;
-    XGetGeometry(
-        dpy, underlying, &root, &x, &y,
-        &width, &height, &border_width, &depth);
-    overlay = XCreateWindow(
-        dpy,                      // display
-        underlying,               // parent
-        0, 0, width, height,      // x, y, width, height
-        0,                        // border width
-        CopyFromParent,           // depth
-        InputOutput,              // class
-        CopyFromParent,           // visual
-        CWOverrideRedirect,       // value mask
-        &attr);                   // value attributes
-    XMapWindow(dpy, overlay);
-  } else {
-    overlay = XCreateWindow(
-        dpy,                                 // display
-        gdk_x11_get_default_root_xwindow(),  // parent
-        0, 0, 1, 1,                          // x, y, width, height
-        0,                                   // border width
-        CopyFromParent,                      // depth
-        InputOutput,                         // class
-        CopyFromParent,                      // visual
-        CWOverrideRedirect,                  // value mask
-        &attr);                              // value attributes
-  }
-  XSync(dpy, False);
-  return overlay;
-}
-
-}
-
 // -----------------------------------------------------------------------------
 
 
@@ -121,14 +65,13 @@ gfx::NativeViewId GtkNativeViewManager::GetIdForWidget(gfx::NativeView widget) {
     CHECK(gdk_window);
     info.x_window_id = GDK_WINDOW_XID(gdk_window);
   }
+
   native_view_to_id_[widget] = new_id;
   id_to_info_[new_id] = info;
 
   g_signal_connect(widget, "realize", G_CALLBACK(::OnRealize), this);
   g_signal_connect(widget, "unrealize", G_CALLBACK(::OnUnrealize), this);
   g_signal_connect(widget, "destroy", G_CALLBACK(::OnDestroy), this);
-  g_signal_connect(
-      widget, "size-allocate", G_CALLBACK(::OnSizeAllocate), this);
 
   return new_id;
 }
@@ -146,24 +89,6 @@ bool GtkNativeViewManager::GetXIDForId(XID* output, gfx::NativeViewId id) {
   return true;
 }
 
-bool GtkNativeViewManager::GetPermanentXIDForId(XID* output,
-                                                gfx::NativeViewId id) {
-  AutoLock locked(lock_);
-
-  std::map<gfx::NativeViewId, NativeViewInfo>::iterator i =
-    id_to_info_.find(id);
-
-  if (i == id_to_info_.end())
-    return false;
-
-  if (!i->second.permanent_window_id) {
-    i->second.permanent_window_id = CreateOverlay(i->second.x_window_id);
-  }
-
-  *output = i->second.permanent_window_id;
-  return true;
-}
-
 // -----------------------------------------------------------------------------
 
 
@@ -191,14 +116,6 @@ void GtkNativeViewManager::OnRealize(gfx::NativeView widget) {
   CHECK(widget->window);
 
   i->second.x_window_id = GDK_WINDOW_XID(widget->window);
-
-  if (i->second.permanent_window_id) {
-    Display* dpy = gdk_x11_get_default_xdisplay();
-    XReparentWindow(
-        dpy, i->second.permanent_window_id, i->second.x_window_id, 0, 0);
-    XMapWindow(dpy, i->second.permanent_window_id);
-    XSync(dpy, False);
-  }
 }
 
 void GtkNativeViewManager::OnUnrealize(gfx::NativeView widget) {
@@ -212,13 +129,6 @@ void GtkNativeViewManager::OnUnrealize(gfx::NativeView widget) {
   CHECK(i != id_to_info_.end());
 
   i->second.x_window_id = 0;
-  if (i->second.permanent_window_id) {
-    Display* dpy = gdk_x11_get_default_xdisplay();
-    XUnmapWindow(dpy, i->second.permanent_window_id);
-    XReparentWindow(dpy, i->second.permanent_window_id,
-                    gdk_x11_get_default_root_xwindow(), 0, 0);
-    XSync(dpy, False);
-  }
 }
 
 void GtkNativeViewManager::OnDestroy(gfx::NativeView widget) {
@@ -234,26 +144,6 @@ void GtkNativeViewManager::OnDestroy(gfx::NativeView widget) {
 
   native_view_to_id_.erase(i);
   id_to_info_.erase(j);
-
-  if (j->second.permanent_window_id) {
-    XDestroyWindow(gdk_x11_get_default_xdisplay(),
-                   j->second.permanent_window_id);
-  }
-}
-
-void GtkNativeViewManager::OnSizeAllocate(
-    gfx::NativeView widget, GtkAllocation *alloc) {
-  AutoLock locked(lock_);
-
-  const gfx::NativeViewId id = GetWidgetId(widget);
-  std::map<gfx::NativeViewId, NativeViewInfo>::iterator i =
-    id_to_info_.find(id);
-
-  if (i->second.permanent_window_id) {
-    XResizeWindow(gdk_x11_get_default_xdisplay(),
-                  i->second.permanent_window_id,
-                  alloc->width, alloc->height);
-  }
 }
 
 // -----------------------------------------------------------------------------
diff --git a/gfx/gtk_native_view_id_manager.h b/gfx/gtk_native_view_id_manager.h
index 439d843..0a90e2f 100644
--- a/gfx/gtk_native_view_id_manager.h
+++ b/gfx/gtk_native_view_id_manager.h
@@ -6,7 +6,6 @@
 #define GFX_GTK_NATIVE_VIEW_ID_MANAGER_H_
 #pragma once
 
-#include <gtk/gtk.h>
 #include <map>
 
 #include "base/singleton.h"
@@ -60,27 +59,10 @@ class GtkNativeViewManager {
   // |*xid| is set to 0.
   bool GetXIDForId(XID* xid, gfx::NativeViewId id);
 
-  // Generate an XID that doesn't change.
-  //
-  // The GPU process assumes that the XID associated with a GL context
-  // does not change. To maintain this invariant we create and overlay
-  // whose XID is static. This requires reparenting as the underlying
-  // window comes and goes.  It incurs a bit of overhead, so a permanent
-  // XID must be requested.
-  //
-  // Must be called from the UI thread so that the widget that we
-  // overlay does not change while we construct the overlay.
-  //
-  // xid: (output) the resulting X window
-  // id: a value previously returned from GetIdForWidget
-  // returns: true if |id| is a valid id, false otherwise.
-  bool GetPermanentXIDForId(XID* xid, gfx::NativeViewId id);
-
   // These are actually private functions, but need to be called from statics.
   void OnRealize(gfx::NativeView widget);
   void OnUnrealize(gfx::NativeView widget);
   void OnDestroy(gfx::NativeView widget);
-  void OnSizeAllocate(gfx::NativeView widget, GtkAllocation *alloc);
 
   Lock& unrealize_lock() { return unrealize_lock_; }
 
@@ -91,11 +73,11 @@ class GtkNativeViewManager {
   friend struct DefaultSingletonTraits<GtkNativeViewManager>;
 
   struct NativeViewInfo {
-    NativeViewInfo() : x_window_id(0), permanent_window_id(0) { }
-    // XID associated with GTK widget.
+    NativeViewInfo()
+        : x_window_id(0) {
+    }
+
     XID x_window_id;
-    // Permanent overlay (0 if not requested yet).
-    XID permanent_window_id;
   };
 
   gfx::NativeViewId GetWidgetId(gfx::NativeView id);