Linux: use opaque NativeViewIds

Currently we are still passing GtkWidget* into the renderer and
trusting the value on the way out. With this patch we switch to using
opaque values.

These opaque values are handled by a GtkNativeViewIdManger, a
singleton object which maintains the list of currently valid ids and
their current X window ids.

We don't pass the X window ids directly to the renderer because they
are a) guessable and b) possibly variable for a GtkWidget. From a
patch size point of view, the X window isn't current created at the
point where we need it so significant work would be needed to reorder
operations to fix that as well.

This patch also removes the GTK accesses from the BACKGROUND_X11
thread which were a temporary hack.

http://codereview.chromium.org/92110

BUG=9014,9869,10787


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

10 files changed, 362 insertions, 40 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 58601ae..9c259b6 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -480,6 +480,8 @@
       'sources': [
         'gfx/gdi_util.cc',
         'gfx/gdi_util.h',
+        'gfx/gtk_native_view_id_manager.cc',
+        'gfx/gtk_native_view_id_manager.h',
         'gfx/gtk_util.cc',
         'gfx/gtk_util.h',
         'gfx/jpeg_codec.cc',
@@ -487,6 +489,7 @@
         'gfx/native_theme.cc',
         'gfx/native_theme.h',
         'gfx/native_widget_types.h',
+        'gfx/native_widget_types_gtk.cc',
         'gfx/platform_canvas.h',
         'gfx/platform_canvas_linux.h',
         'gfx/platform_canvas_mac.h',
@@ -528,7 +531,9 @@
             ],
         }],
         [ 'OS != "linux"', { 'sources!': [
+            'gfx/gtk_native_view_id_manager.cc',
             'gfx/gtk_util.cc',
+            'gfx/native_widget_types_gtk.cc',
             ],
         }],
       ],
diff --git a/base/gfx/gtk_native_view_id_manager.cc b/base/gfx/gtk_native_view_id_manager.cc
new file mode 100644
index 0000000..848e1eb
--- /dev/null
+++ b/base/gfx/gtk_native_view_id_manager.cc
@@ -0,0 +1,141 @@
+// 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 "base/gfx/gtk_native_view_id_manager.h"
+
+#include "base/gfx/rect.h"
+#include "base/logging.h"
+#include "base/rand_util.h"
+
+#include <gtk/gtk.h>
+#include <gdk/gdkx.h>
+
+// -----------------------------------------------------------------------------
+// Bounce functions for GTK to callback into a C++ object...
+
+static void OnRealize(gfx::NativeView widget, void* arg) {
+  GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
+  manager->OnRealize(widget);
+}
+
+static void OnUnrealize(gfx::NativeView widget, void *arg) {
+  GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
+  manager->OnUnrealize(widget);
+}
+
+static void OnDestroy(GtkObject* obj, void* arg) {
+  GtkNativeViewManager* manager = reinterpret_cast<GtkNativeViewManager*>(arg);
+  manager->OnDestroy(reinterpret_cast<GtkWidget*>(obj));
+}
+
+// -----------------------------------------------------------------------------
+
+
+// -----------------------------------------------------------------------------
+// Public functions...
+
+GtkNativeViewManager::GtkNativeViewManager() {
+}
+
+gfx::NativeViewId GtkNativeViewManager::GetIdForWidget(gfx::NativeView widget) {
+  AutoLock locked(lock_);
+
+  std::map<gfx::NativeView, gfx::NativeViewId>::const_iterator i =
+    native_view_to_id_.find(widget);
+
+  if (i != native_view_to_id_.end())
+    return i->second;
+
+  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;
+  if (GTK_WIDGET_REALIZED(widget)) {
+    GdkWindow *gdk_window = widget->window;
+    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);
+
+  return new_id;
+}
+
+bool GtkNativeViewManager::GetXIDForId(XID* output, gfx::NativeViewId id) {
+  AutoLock locked(lock_);
+
+  std::map<gfx::NativeViewId, NativeViewInfo>::const_iterator i =
+    id_to_info_.find(id);
+
+  if (i == id_to_info_.end())
+    return false;
+
+  *output = i->second.x_window_id;
+  return true;
+}
+
+// -----------------------------------------------------------------------------
+
+
+// -----------------------------------------------------------------------------
+// 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);
+}
+
+void GtkNativeViewManager::OnUnrealize(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());
+
+  i->second.x_window_id = 0;
+}
+
+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());
+
+  native_view_to_id_.erase(i);
+  id_to_info_.erase(j);
+}
+
+// -----------------------------------------------------------------------------
diff --git a/base/gfx/gtk_native_view_id_manager.h b/base/gfx/gtk_native_view_id_manager.h
new file mode 100644
index 0000000..5b34baa
--- /dev/null
+++ b/base/gfx/gtk_native_view_id_manager.h
@@ -0,0 +1,91 @@
+// 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.
+
+#ifndef BASE_GFX_GTK_NATIVE_VIEW_ID_MANAGER_H_
+#define BASE_GFX_GTK_NATIVE_VIEW_ID_MANAGER_H_
+
+#include <map>
+
+#include "base/singleton.h"
+#include "base/gfx/native_widget_types.h"
+
+typedef unsigned long XID;
+
+// 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
+// one cannot terminate sync calls on the UI thread as that can lead to
+// deadlocks.
+//
+// Because of this, we have the BACKGROUND_X11 thread for these calls and this
+// thread has a separate X connection in order to answer them. But one cannot
+// use GTK on multiple threads, so the BACKGROUND_X11 thread deals only in Xlib
+// calls and, thus, XIDs.
+//
+// So we could make NativeViewIds be the X id of the window. However, at the
+// time when we need to tell the renderer about its NativeViewId, an XID isn't
+// availible and it goes very much against the grain of the code to make it so.
+// Also, we worry that GTK might choose to change the underlying X window id
+// when, say, the widget is hidden or repacked. Finally, if we used XIDs then a
+// compromised renderer could start asking questions about any X windows on the
+// system.
+//
+// Thus, we have this object. It produces random NativeViewIds from GtkWidget
+// pointers and observes the various signals from the widget for when an X
+// window is created, destroyed etc. Thus it provides a thread safe mapping
+// from NativeViewIds to the current XID for that widget.
+//
+// You get a reference to the global instance with:
+//   Singleton<GtkNativeViewManager>()
+class GtkNativeViewManager {
+ public:
+  // Must be called from the UI thread:
+  //
+  // Return a NativeViewId for the given widget and attach to the various
+  // signals emitted by that widget. The NativeViewId is pseudo-randomly
+  // allocated so that a compromised renderer trying to guess values will fail
+  // with high probability. The NativeViewId will not be reused for the
+  // lifetime of the GtkWidget.
+  gfx::NativeViewId GetIdForWidget(gfx::NativeView widget);
+
+  // May be called from any thread:
+  //
+  // xid: (output) the resulting X window ID, or 0
+  // id: a value previously returned from GetIdForWidget
+  // returns: true if |id| is a valid id, false otherwise.
+  //
+  // If the widget referenced by |id| does not current have an X window id,
+  // |*xid| is set to 0.
+  bool GetXIDForId(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);
+
+ private:
+  // This object is a singleton:
+  GtkNativeViewManager();
+  friend struct DefaultSingletonTraits<GtkNativeViewManager>;
+
+  struct NativeViewInfo {
+    NativeViewInfo()
+        : x_window_id(0) {
+    }
+
+    XID x_window_id;
+  };
+
+  gfx::NativeViewId GetWidgetId(gfx::NativeView id);
+
+  // protects native_view_to_id_ and id_to_info_
+  Lock lock_;
+    // If asked for an id for the same widget twice, we want to return the same
+    // id. So this records the current mapping.
+    std::map<gfx::NativeView, gfx::NativeViewId> native_view_to_id_;
+    std::map<gfx::NativeViewId, NativeViewInfo> id_to_info_;
+
+  DISALLOW_COPY_AND_ASSIGN(GtkNativeViewManager);
+};
+
+#endif  // BASE_GFX_GTK_NATIVE_VIEW_ID_MANAGER_H_
diff --git a/base/gfx/native_widget_types.h b/base/gfx/native_widget_types.h
index 32d6012..d43db5c 100644
--- a/base/gfx/native_widget_types.h
+++ b/base/gfx/native_widget_types.h
@@ -78,23 +78,34 @@ typedef cairo_surface_t* NativeDrawingContext;
 // See comment at the top of the file for usage.
 typedef intptr_t NativeViewId;
 
-// Convert a NativeViewId to a NativeView. At the moment, we assume that the
-// ids are the same as the NativeViews. This is correct on Windows (where
-// NativeView == HWND).
-// TODO(port): figure out what ids are going to be and implement this function
-// This is only to be called in the browser process.
+// Convert a NativeViewId to a NativeView.
+// On Windows, these are both HWNDS so it's just a cast.
+// On Mac, for now, we pass the NSView pointer into the renderer
+// On Linux we use an opaque id
+#if defined(OS_WIN) || defined(OS_MACOSX)
 static inline NativeView NativeViewFromId(NativeViewId id) {
   return reinterpret_cast<NativeView>(id);
 }
+#elif defined(OS_LINUX)
+// A NativeView on Linux is a GtkWidget*. However, we can't go directly from an
+// X window ID to a GtkWidget. Thus, functions which handle NativeViewIds from
+// the renderer have to use Xlib. This is fine since these functions are
+// generally performed on the BACKGROUND_X thread which can't use GTK anyway.
+
+#define NativeViewFromId(x) NATIVE_VIEW_FROM_ID_NOT_AVAILIBLE_ON_LINUX
 
-// Convert a NativeView to a NativeViewId. At the moment, we assume that the
-// ids are the same as the NativeViews. This is correct on Windows (where
-// NativeView == HWND).
-// TODO(port): figure out what ids are going to be and implement this function
-// This is only to be called in the browser process.
+#endif  // defined(OS_LINUX)
+
+// Convert a NativeView to a NativeViewId. See the comments above
+// NativeViewFromId.
+#if defined(OS_WIN) || defined(OS_MACOSX)
 static inline NativeViewId IdFromNativeView(NativeView view) {
   return reinterpret_cast<NativeViewId>(view);
 }
+#elif defined(OS_LINUX)
+// Not inlined because it involves pulling too many headers.
+NativeViewId IdFromNativeView(NativeView view);
+#endif  // defined(OS_LINUX)
 
 }  // namespace gfx
 
diff --git a/base/gfx/native_widget_types_gtk.cc b/base/gfx/native_widget_types_gtk.cc
new file mode 100644
index 0000000..2deb46c
--- /dev/null
+++ b/base/gfx/native_widget_types_gtk.cc
@@ -0,0 +1,16 @@
+// 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 "base/gfx/native_widget_types.h"
+
+#include "base/gfx/gtk_native_view_id_manager.h"
+#include "base/logging.h"
+
+namespace gfx {
+
+NativeViewId IdFromNativeView(NativeView view) {
+  return Singleton<GtkNativeViewManager>()->GetIdForWidget(view);
+}
+
+}  // namespace gfx
diff --git a/chrome/browser/renderer_host/resource_message_filter_gtk.cc b/chrome/browser/renderer_host/resource_message_filter_gtk.cc
index 750356a..70173ad 100644
--- a/chrome/browser/renderer_host/resource_message_filter_gtk.cc
+++ b/chrome/browser/renderer_host/resource_message_filter_gtk.cc
@@ -4,8 +4,7 @@
 
 #include "chrome/browser/renderer_host/resource_message_filter.h"
 
-#include <gtk/gtk.h>
-
+#include "base/gfx/gtk_native_view_id_manager.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/x11_util.h"
@@ -40,16 +39,18 @@ void ResourceMessageFilter::DoOnGetScreenInfo(gfx::NativeViewId view,
 // Called on the BACKGROUND_X11 thread.
 void ResourceMessageFilter::DoOnGetWindowRect(gfx::NativeViewId view,
                                               IPC::Message* reply_msg) {
+  // This is called to get the x, y offset (in screen coordinates) of the given
+  // view and its width and height.
   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);
+  XID window;
+
+  if (Singleton<GtkNativeViewManager>()->GetXIDForId(&window, view)) {
+    if (window) {
+      int x, y;
+      unsigned width, height;
+      if (x11_util::GetWindowGeometry(&x, &y, &width, &height, window))
+        rect = gfx::Rect(x, y, width, height);
+    }
   }
 
   ViewHostMsg_GetWindowRect::WriteReplyParams(reply_msg, rect);
@@ -59,24 +60,36 @@ void ResourceMessageFilter::DoOnGetWindowRect(gfx::NativeViewId view,
         this, &ResourceMessageFilter::SendBackgroundX11Reply, reply_msg));
 }
 
+// Return the top-level parent of the given window. Called on the
+// BACKGROUND_X11 thread.
+static XID GetTopLevelWindow(XID window) {
+  bool parent_is_root;
+  XID parent_window;
+
+  if (!x11_util::GetWindowParent(&parent_window, &parent_is_root, window))
+    return 0;
+  if (parent_is_root)
+    return window;
+
+  return GetTopLevelWindow(parent_window);
+}
+
 // Called on the BACKGROUND_X11 thread.
 void ResourceMessageFilter::DoOnGetRootWindowRect(gfx::NativeViewId view,
                                                   IPC::Message* reply_msg) {
+  // This is called to get the screen coordinates and size of the browser
+  // window itself.
   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);
+  XID window;
+
+  if (Singleton<GtkNativeViewManager>()->GetXIDForId(&window, view)) {
+    if (window) {
+      const XID toplevel = GetTopLevelWindow(toplevel);
+      int x, y;
+      unsigned width, height;
+      if (x11_util::GetWindowGeometry(&x, &y, &width, &height, window))
+        rect = gfx::Rect(x, y, width, height);
+    }
   }
 
   ViewHostMsg_GetRootWindowRect::WriteReplyParams(reply_msg, rect);
diff --git a/chrome/common/x11_util.cc b/chrome/common/x11_util.cc
index 83fc02a..5371573 100644
--- a/chrome/common/x11_util.cc
+++ b/chrome/common/x11_util.cc
@@ -240,13 +240,38 @@ Display* GetSecondaryDisplay() {
 }
 
 // Called on BACKGROUND_X11 thread.
-void GetWindowGeometry(int* x, int* y, unsigned* width, unsigned* height,
+bool GetWindowGeometry(int* x, int* y, unsigned* width, unsigned* height,
                        XID window) {
-  Window root_window;
+  Window root_window, child_window;
   unsigned border_width, depth;
+  int temp;
 
-  CHECK(XGetGeometry(GetSecondaryDisplay(), window,
-                     &root_window, x, y, width, height, &border_width, &depth));
+  if (!XGetGeometry(GetSecondaryDisplay(), window, &root_window, &temp, &temp,
+                    width, height, &border_width, &depth))
+    return false;
+  if (!XTranslateCoordinates(GetSecondaryDisplay(), window, root_window,
+                             0, 0 /* input x, y */, x, y /* output x, y */,
+                             &child_window))
+    return false;
+
+  return true;
+}
+
+// Called on BACKGROUND_X11 thread.
+bool GetWindowParent(XID* parent_window, bool* parent_is_root, XID window) {
+  XID root_window, *children;
+  unsigned num_children;
+
+  Status s = XQueryTree(GetSecondaryDisplay(), window, &root_window,
+                        parent_window, &children, &num_children);
+  if (!s)
+    return false;
+
+  if (children)
+    XFree(children);
+
+  *parent_is_root = root_window == *parent_window;
+  return true;
 }
 
 }  // namespace x11_util
diff --git a/chrome/common/x11_util.h b/chrome/common/x11_util.h
index aef133f..dddd40c 100644
--- a/chrome/common/x11_util.h
+++ b/chrome/common/x11_util.h
@@ -77,9 +77,17 @@ Display* GetSecondaryDisplay();
 // These functions must be called on the BACKGROUND_X11 thread since they
 // reference GetSecondaryDisplay().
 
-void GetWindowGeometry(int* x, int* y, unsigned* width, unsigned* height,
+// Get the position of the given window in screen coordinates as well as its
+// current size.
+bool GetWindowGeometry(int* x, int* y, unsigned* width, unsigned* height,
                        XID window);
 
+// Find the immediate parent of an X window.
+//
+// parent_window: (output) the parent window of |window|, or 0.
+// parent_is_root: (output) true iff the parent of |window| is the root window.
+bool GetWindowParent(XID* parent_window, bool* parent_is_root, XID window);
+
 }  // namespace x11_util
 
 #endif  // CHROME_COMMON_X11_UTIL_H_
diff --git a/chrome/plugin/webplugin_delegate_stub.cc b/chrome/plugin/webplugin_delegate_stub.cc
index 73f5b79..a782b65 100644
--- a/chrome/plugin/webplugin_delegate_stub.cc
+++ b/chrome/plugin/webplugin_delegate_stub.cc
@@ -131,8 +131,16 @@ void WebPluginDelegateStub::OnInit(const PluginMsg_Init_Params& params,
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   FilePath path = FilePath::FromWStringHack(
       command_line.GetSwitchValue(switches::kPluginPath));
+
+#if defined(OS_WIN)
   delegate_ = WebPluginDelegate::Create(
       path, mime_type_, gfx::NativeViewFromId(params.containing_window));
+#else
+  // We don't have gfx::NativeViewFromId on Linux
+  NOTIMPLEMENTED();
+  delegate_ = NULL;
+#endif
+
   if (delegate_) {
     webplugin_ = new WebPluginProxy(channel_, instance_id_, delegate_);
 #if defined(OS_WIN)
diff --git a/chrome/renderer/webplugin_delegate_proxy.cc b/chrome/renderer/webplugin_delegate_proxy.cc
index 9c47852b..2a35b3c 100644
--- a/chrome/renderer/webplugin_delegate_proxy.cc
+++ b/chrome/renderer/webplugin_delegate_proxy.cc
@@ -630,10 +630,14 @@ int WebPluginDelegateProxy::GetProcessId() {
 }
 
 void WebPluginDelegateProxy::OnSetWindow(gfx::NativeViewId window_id) {
+#if defined(OS_WIN)
   gfx::NativeView window = gfx::NativeViewFromId(window_id);
   windowless_ = window == NULL;
   if (plugin_)
     plugin_->SetWindow(window);
+#else
+  NOTIMPLEMENTED();
+#endif
 }
 
 #if defined(OS_WIN)