Defer window destruction until GPU finished drawing.

gfx/gtk_native_view_id_manager.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gfx/gtk_native_view_id_manager.h"
 
 #include <gtk/gtk.h>
 #include <gdk/gdkx.h>
 
 #include "base/logging.h"
@@ skipping 46 matching lines @@
   gfx::NativeViewId new_id =
       static_cast<gfx::NativeViewId>(base::RandUint64());
   while (id_to_info_.find(new_id) != id_to_info_.end())
     new_id = static_cast<gfx::NativeViewId>(base::RandUint64());
 
   NativeViewInfo info;
   info.widget = widget;
   if (GTK_WIDGET_REALIZED(widget)) {
     GdkWindow *gdk_window = widget->window;
     CHECK(gdk_window);
-    info.x_window_id = GDK_WINDOW_XID(gdk_window);
+    XID xid = GDK_WINDOW_XID(gdk_window);
+    info.x_window_id = xid;
+    xid_to_native_view_[xid] = widget;
   }
 
   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);
 
   return new_id;
@@ skipping 17 matching lines @@
   AutoLock locked(lock_);
 
   std::map<gfx::NativeViewId, NativeViewInfo>::iterator i =
       id_to_info_.find(id);
 
   if (i == id_to_info_.end())
     return false;
 
   // 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;
+  CHECK(GTK_IS_PRESERVE_WINDOW(i->second.widget));

[piman, 2010/11/29 21:59:01]

I'm not sure that this CHECK (and other below) really need to be present in
release builds, you probably should change them to DCHECKs.

[jonathan.backer, 2010/11/30 13:09:31]

Done.

 
   GtkPreserveWindow* widget =
       reinterpret_cast<GtkPreserveWindow*>(i->second.widget);
   gtk_preserve_window_set_preserve(widget, TRUE);
 
   *output = GDK_WINDOW_XID(i->second.widget->window);
+
+  // It's possible that we were not realized before this call, in which
+  // case we must place an entry in the lookup table.
+  xid_to_native_view_[*output] = i->second.widget;
+
   return true;
 }
 
+void GtkNativeViewManager::ReleasePermanentXID(XID xid) {
+  AutoLock locked(lock_);
+
+  std::map<XID, gfx::NativeView>::iterator i =
+    xid_to_native_view_.find(xid);
+
+  if (i == xid_to_native_view_.end())
+    return;
+
+  if (i->second) {
+    CHECK(GTK_IS_PRESERVE_WINDOW(i->second));
+    GtkPreserveWindow* preserve_window =
+      reinterpret_cast<GtkPreserveWindow*>(i->second);
+    gtk_preserve_window_set_preserve(preserve_window, FALSE);
+  } else {
+    GdkWindow* window = reinterpret_cast<GdkWindow*>(gdk_xid_table_lookup(xid));
+    CHECK(window);
+    gdk_window_destroy(window);
+  }
+}
+
 // -----------------------------------------------------------------------------
 
 
 // -----------------------------------------------------------------------------
 // Private functions...
 
 gfx::NativeViewId GtkNativeViewManager::GetWidgetId(gfx::NativeView widget) {
   lock_.AssertAcquired();
 
   std::map<gfx::NativeView, gfx::NativeViewId>::const_iterator i =
     native_view_to_id_.find(widget);
 
   CHECK(i != native_view_to_id_.end());
   return i->second;
 }
 
 void GtkNativeViewManager::OnRealize(gfx::NativeView widget) {
   AutoLock locked(lock_);
 
   const gfx::NativeViewId id = GetWidgetId(widget);
   std::map<gfx::NativeViewId, NativeViewInfo>::iterator i =
     id_to_info_.find(id);
 
   CHECK(i != id_to_info_.end());
   CHECK(widget->window);
 
   i->second.x_window_id = GDK_WINDOW_XID(widget->window);
+
+  // See if the XID is already associated with a widget.
+  // If so, there had better be a lock on that window.
+  XID xid = GDK_WINDOW_XID(widget->window);
+  std::map<XID, gfx::NativeView>::iterator j =
+    xid_to_native_view_.find(xid);
+
+  if (j != xid_to_native_view_.end()) {
+    CHECK(GTK_IS_PRESERVE_WINDOW(j->second));
+    GtkPreserveWindow* preserve_window =
+      reinterpret_cast<GtkPreserveWindow*>(j->second);
+    CHECK(gtk_preserve_window_get_preserve(preserve_window));
+  } else {
+    xid_to_native_view_[xid] = widget;

[piman, 2010/11/29 21:59:01]

We're effectively doing the lookup into the map twice, only for checks that
should always be true.
How about using xid_to_native_view_.insert(std::make_pair(xid, widget)) and
testing the return value for the checks ?

[jonathan.backer, 2010/11/30 13:09:31]

Done.

+  }
 }
 
 void GtkNativeViewManager::OnUnrealize(gfx::NativeView widget) {
   AutoLock unrealize_locked(unrealize_lock_);
   AutoLock locked(lock_);
 
   const gfx::NativeViewId id = GetWidgetId(widget);
   std::map<gfx::NativeViewId, NativeViewInfo>::iterator i =
     id_to_info_.find(id);
 
   CHECK(i != id_to_info_.end());
 
+  XID xid = i->second.x_window_id;
   i->second.x_window_id = 0;
+
+  // Remove XID from lookup table unless it will be preserved.
+  std::map<XID, gfx::NativeView>::iterator j =
+    xid_to_native_view_.find(xid);
+
+  CHECK(j != xid_to_native_view_.end());
+  if (!GTK_IS_PRESERVE_WINDOW(j->second) ||
+      !gtk_preserve_window_get_preserve(
+          reinterpret_cast<GtkPreserveWindow*>(j->second))) {
+    xid_to_native_view_.erase(j);
+  }
 }
 
 void GtkNativeViewManager::OnDestroy(gfx::NativeView widget) {
   AutoLock locked(lock_);
 
   std::map<gfx::NativeView, gfx::NativeViewId>::iterator i =
     native_view_to_id_.find(widget);
   CHECK(i != native_view_to_id_.end());
 
   std::map<gfx::NativeViewId, NativeViewInfo>::iterator j =
     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)))
+    xid_to_native_view_[GDK_WINDOW_XID(widget->window)] = NULL;
+
   native_view_to_id_.erase(i);
   id_to_info_.erase(j);
 }
 
 // -----------------------------------------------------------------------------