Defer window destruction until GPU finished drawing.

When a tab is closed, it takes a while before the GPU stops drawing into the window. Destroying the window before the GPU has flushed its drawing pipeline causes unsightly X11 errors. The custom widget class that we use for drawing
tab contents has a lock that is set when the GPU process is drawing to that widget. We use this lock to determine who should delete the associated window:
- if the lock is clear at the time of widget destruction, the widget destroys the window
- if the lock is set, the GPU process signals to the browser to destroy the widget (it has to be done in the browser process b/c the X window is wrapped in a GdkWindow that resides in the browser address space)

Most the management is done in GtkNativeViewManager. I've added another map from XID to GtkWidget to facilitate this management.

BUG=55158

TEST=Open two windows with http://webkit.org/blog-files/3d-transforms/poster-circle.html Close one of them. There should be no X11 errors generated.

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

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

7 files changed, 110 insertions, 7 deletions

diff --git a/chrome/browser/gpu_process_host.cc b/chrome/browser/gpu_process_host.cc
index c321f97..eeb50bd 100644
--- a/chrome/browser/gpu_process_host.cc
+++ b/chrome/browser/gpu_process_host.cc
@@ -171,6 +171,7 @@ void GpuProcessHost::OnControlMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(GpuHostMsg_SynchronizeReply, OnSynchronizeReply)
 #if defined(OS_LINUX)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuHostMsg_GetViewXID, OnGetViewXID)
+    IPC_MESSAGE_HANDLER(GpuHostMsg_ReleaseXID, OnReleaseXID)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuHostMsg_ResizeXID, OnResizeXID)
 #elif defined(OS_MACOSX)
     IPC_MESSAGE_HANDLER(GpuHostMsg_AcceleratedSurfaceSetIOSurface,
@@ -228,6 +229,11 @@ void GetViewXIDDispatcher(gfx::NativeViewId id, IPC::Message* reply_msg) {
       NewRunnableFunction(&SendDelayedReply, reply_msg));
 }
 
+void ReleaseXIDDispatcher(unsigned long xid) {
+  GtkNativeViewManager* manager = Singleton<GtkNativeViewManager>::get();
+  manager->ReleasePermanentXID(xid);
+}
+
 void ResizeXIDDispatcher(unsigned long xid, gfx::Size size,
     IPC::Message *reply_msg) {
   GdkWindow* window = reinterpret_cast<GdkWindow*>(gdk_xid_table_lookup(xid));
@@ -255,6 +261,13 @@ void GpuProcessHost::OnGetViewXID(gfx::NativeViewId id,
       NewRunnableFunction(&GetViewXIDDispatcher, id, reply_msg));
 }
 
+void GpuProcessHost::OnReleaseXID(unsigned long xid) {
+  // Have to release a permanent XID from UI thread.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableFunction(&ReleaseXIDDispatcher, xid));
+}
+
 void GpuProcessHost::OnResizeXID(unsigned long xid, gfx::Size size,
                                  IPC::Message *reply_msg) {
   // Have to resize the window from UI thread.
diff --git a/chrome/browser/gpu_process_host.h b/chrome/browser/gpu_process_host.h
index 8219355..05ae39e 100644
--- a/chrome/browser/gpu_process_host.h
+++ b/chrome/browser/gpu_process_host.h
@@ -87,6 +87,7 @@ class GpuProcessHost : public BrowserChildProcessHost, public NonThreadSafe {
   void OnSynchronizeReply();
 #if defined(OS_LINUX)
   void OnGetViewXID(gfx::NativeViewId id, IPC::Message* reply_msg);
+  void OnReleaseXID(unsigned long xid);
   void OnResizeXID(unsigned long xid, gfx::Size size, IPC::Message* reply_msg);
 #elif defined(OS_MACOSX)
   void OnAcceleratedSurfaceSetIOSurface(
diff --git a/chrome/common/gpu_messages_internal.h b/chrome/common/gpu_messages_internal.h
index 10b0961..6ad6449 100644
--- a/chrome/common/gpu_messages_internal.h
+++ b/chrome/common/gpu_messages_internal.h
@@ -94,6 +94,11 @@ IPC_BEGIN_MESSAGES(GpuHost)
                               gfx::NativeViewId, /* view */
                               unsigned long /* xid */)
 
+  // Release the lock on the window.
+  // If the associated view has been destroyed, destroy the window.
+  IPC_MESSAGE_CONTROL1(GpuHostMsg_ReleaseXID,
+                       unsigned long /* xid */)
+
   IPC_SYNC_MESSAGE_CONTROL2_1(GpuHostMsg_ResizeXID,
                               unsigned long, /* xid */
                               gfx::Size, /* size */
diff --git a/chrome/gpu/gpu_command_buffer_stub.cc b/chrome/gpu/gpu_command_buffer_stub.cc
index 3c87c99..6112782 100644
--- a/chrome/gpu/gpu_command_buffer_stub.cc
+++ b/chrome/gpu/gpu_command_buffer_stub.cc
@@ -159,6 +159,10 @@ GpuCommandBufferStub::~GpuCommandBufferStub() {
     DestroyWindow(static_cast<HWND>(compositor_window_));
     compositor_window_ = NULL;
   }
+#elif defined(OS_LINUX)
+  ChildThread* gpu_thread = ChildThread::current();
+  gpu_thread->Send(
+      new GpuHostMsg_ReleaseXID(handle_));
 #endif
 }
 
diff --git a/gfx/gtk_native_view_id_manager.cc b/gfx/gtk_native_view_id_manager.cc
index 6aa460c..8d9334d 100644
--- a/gfx/gtk_native_view_id_manager.cc
+++ b/gfx/gtk_native_view_id_manager.cc
@@ -63,7 +63,7 @@ gfx::NativeViewId GtkNativeViewManager::GetIdForWidget(gfx::NativeView widget) {
   info.widget = widget;
   if (GTK_WIDGET_REALIZED(widget)) {
     GdkWindow *gdk_window = widget->window;
-    CHECK(gdk_window);
+    DCHECK(gdk_window);
     info.x_window_id = GDK_WINDOW_XID(gdk_window);
   }
 
@@ -102,17 +102,52 @@ bool GtkNativeViewManager::GetPermanentXIDForId(XID* output,
 
   // We only return permanent XIDs for widgets that allow us to guarantee that
   // the XID will not change.
-  if (!GTK_IS_PRESERVE_WINDOW(i->second.widget))
-    return false;
-
+  DCHECK(GTK_IS_PRESERVE_WINDOW(i->second.widget));
   GtkPreserveWindow* widget =
       reinterpret_cast<GtkPreserveWindow*>(i->second.widget);
   gtk_preserve_window_set_preserve(widget, TRUE);
 
   *output = GDK_WINDOW_XID(i->second.widget->window);
+
+  // Update the reference count on the permanent XID.
+  PermanentXIDInfo info;
+  info.widget = widget;
+  info.ref_count = 1;
+  std::pair<std::map<XID, PermanentXIDInfo>::iterator, bool> ret =
+    perm_xid_to_info_.insert(std::make_pair(*output, info));
+
+  if (!ret.second) {
+    DCHECK(ret.first->second.widget == widget);
+    ret.first->second.ref_count++;
+  }
+
   return true;
 }
 
+void GtkNativeViewManager::ReleasePermanentXID(XID xid) {
+  AutoLock locked(lock_);
+
+  std::map<XID, PermanentXIDInfo>::iterator i =
+    perm_xid_to_info_.find(xid);
+
+  if (i == perm_xid_to_info_.end())
+    return;
+
+  if (i->second.ref_count > 1) {
+    i->second.ref_count--;
+  } else {
+    if (i->second.widget) {
+      gtk_preserve_window_set_preserve(i->second.widget, FALSE);
+    } else {
+      GdkWindow* window = reinterpret_cast<GdkWindow*>(
+          gdk_xid_table_lookup(xid));
+      DCHECK(window);
+      gdk_window_destroy(window);
+    }
+    perm_xid_to_info_.erase(i);
+  }
+}
+
 // -----------------------------------------------------------------------------
 
 
@@ -151,8 +186,6 @@ void GtkNativeViewManager::OnUnrealize(gfx::NativeView widget) {
     id_to_info_.find(id);
 
   CHECK(i != id_to_info_.end());
-
-  i->second.x_window_id = 0;
 }
 
 void GtkNativeViewManager::OnDestroy(gfx::NativeView widget) {
@@ -166,6 +199,18 @@ void GtkNativeViewManager::OnDestroy(gfx::NativeView widget) {
     id_to_info_.find(i->second);
   CHECK(j != id_to_info_.end());
 
+  // If the XID is supposed to outlive the widget, mark it
+  // in the lookup table.
+  if (GTK_IS_PRESERVE_WINDOW(widget) &&
+      gtk_preserve_window_get_preserve(
+          reinterpret_cast<GtkPreserveWindow*>(widget))) {
+    std::map<XID, PermanentXIDInfo>::iterator k =
+      perm_xid_to_info_.find(GDK_WINDOW_XID(widget->window));
+
+    if (k != perm_xid_to_info_.end())
+      k->second.widget = NULL;
+  }
+
   native_view_to_id_.erase(i);
   id_to_info_.erase(j);
 }
diff --git a/gfx/gtk_native_view_id_manager.h b/gfx/gtk_native_view_id_manager.h
index 513537a..05cf0d7 100644
--- a/gfx/gtk_native_view_id_manager.h
+++ b/gfx/gtk_native_view_id_manager.h
@@ -12,6 +12,7 @@
 #include "gfx/native_widget_types.h"
 
 typedef unsigned long XID;
+struct _GtkPreserveWindow;
 
 // NativeViewIds are the opaque values which the renderer holds as a reference
 // to a window. These ids are often used in sync calls from the renderer and
@@ -70,6 +71,14 @@ class GtkNativeViewManager {
   // returns: true if |id| is a valid id, false otherwise.
   bool GetPermanentXIDForId(XID* xid, gfx::NativeViewId id);
 
+  // Must be called from the UI thread because we may need to access a
+  // GtkWidget or destroy a GdkWindow.
+  //
+  // If the widget associated with the XID is still alive, allow the widget
+  // to destroy the associated XID when it wants. Otherwise, destroy the
+  // GdkWindow associated with the XID.
+  void ReleasePermanentXID(XID xid);
+
   // These are actually private functions, but need to be called from statics.
   void OnRealize(gfx::NativeView widget);
   void OnUnrealize(gfx::NativeView widget);
@@ -105,6 +114,26 @@ class GtkNativeViewManager {
   std::map<gfx::NativeView, gfx::NativeViewId> native_view_to_id_;
   std::map<gfx::NativeViewId, NativeViewInfo> id_to_info_;
 
+  struct PermanentXIDInfo {
+    PermanentXIDInfo() : widget(NULL), ref_count(0) {
+    }
+    _GtkPreserveWindow* widget;
+    int ref_count;
+  };
+
+  // Used to maintain the reference count for permanent XIDs
+  // (referenced by GetPermanentXIDForId and dereferenced by
+  // ReleasePermanentXID). Only those XIDs with a positive reference count
+  // will be in the table.
+  //
+  // In general, several GTK widgets may share the same X window. We assume
+  // that is not true of the widgets stored in this registry.
+  //
+  // An XID will map to NULL, if there is an outstanding reference but the
+  // widget was destroyed. In this case, the destruction of the X window
+  // is deferred to the dropping of all references.
+  std::map<XID, PermanentXIDInfo> perm_xid_to_info_;
+
   DISALLOW_COPY_AND_ASSIGN(GtkNativeViewManager);
 };
 
diff --git a/gfx/gtk_preserve_window.cc b/gfx/gtk_preserve_window.cc
index c49bd31..20215d1 100644
--- a/gfx/gtk_preserve_window.cc
+++ b/gfx/gtk_preserve_window.cc
@@ -61,10 +61,13 @@ GtkWidget* gtk_preserve_window_new() {
 
 static void gtk_preserve_window_destroy(GtkObject* object) {
   GtkWidget* widget = reinterpret_cast<GtkWidget*>(object);
+  GtkPreserveWindowPrivate* priv = GTK_PRESERVE_WINDOW_GET_PRIVATE(widget);
 
   if (widget->window) {
     gdk_window_set_user_data(widget->window, NULL);
-    gdk_window_destroy(widget->window);
+    // If the window is preserved, someone else must destroy it.
+    if (!priv->preserve_window)
+      gdk_window_destroy(widget->window);
     widget->window = NULL;
   }
 
@@ -167,6 +170,9 @@ void gtk_preserve_window_set_preserve(GtkPreserveWindow* window,
     attributes_mask = GDK_WA_VISUAL | GDK_WA_COLORMAP;
     widget->window = gdk_window_new(
         gdk_get_default_root_window(), &attributes, attributes_mask);
+  } else if (!value && widget->window && !GTK_WIDGET_REALIZED(widget)) {
+    gdk_window_destroy(widget->window);
+    widget->window = NULL;
   }
 }