linux: new socket/plug code for windowed plugins

This CL reworks the GtkSocket/GtkPlug code for windowed plugins on linux. Instead of having the plugin ask the browser to create a socket to plug into, it simply creates a plug and sends it to the browser. The browser creates a socket and attaches the plug when the socket becomes realized

This fixes 2 main issues:
- we can create windowed plugins in background tabs (Issue 16125)
- we can detach tabs with windowed plugins and reattach them (Issue 17110)

I reworked the IPCs, so it removes some amount of linux-specific things. We also need less synchronous IPCs to create/destroy plugins, so that should be a bit faster. In particular, I removed the plugin pid map, and instead made sure the renderer always destroys the plugin containers if the plugin process crashes - they will be destroyed if the renderer process crashes. Let me know if you have an issue with that.

Also, the intermediate plug/socket creation now happens in webplugin_delegate_impl_gtk. That means test_shell uses it as well. It made the code a lot simpler, and means we're testing it as well, albeit with a bit of extra overhead.

Bonus: I found a big bad bug in the GtkPluginContainer that made its width/height alias with some internal gtk structures. That was certainly causing some amounts of bugs.

Bonus 2: scrolling now looks more in sync with the rest of the page, though I'm not exactly sure which part caused that.

BUG=16125,17110
TEST=a lot of manual testing involving YouTube videos

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


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

13 files changed, 126 insertions, 104 deletions

diff --git a/webkit/glue/plugins/gtk_plugin_container.cc b/webkit/glue/plugins/gtk_plugin_container.cc
index c487fa0..c80bbf1 100644
--- a/webkit/glue/plugins/gtk_plugin_container.cc
+++ b/webkit/glue/plugins/gtk_plugin_container.cc
@@ -35,7 +35,7 @@ class GtkPluginContainer : public GtkSocket {
         NULL, NULL,
         static_cast<GClassInitFunc>(&ClassInit),
         NULL, NULL,
-        sizeof(GtkSocket),  // We are identical to a GtkSocket.
+        sizeof(GtkPluginContainer),
         0, &InstanceInit,
       };
       type = g_type_register_static(GTK_TYPE_SOCKET,
diff --git a/webkit/glue/plugins/gtk_plugin_container_manager.cc b/webkit/glue/plugins/gtk_plugin_container_manager.cc
index 266687b..4683e08 100644
--- a/webkit/glue/plugins/gtk_plugin_container_manager.cc
+++ b/webkit/glue/plugins/gtk_plugin_container_manager.cc
@@ -11,54 +11,44 @@
 #include "webkit/glue/plugins/gtk_plugin_container.h"
 #include "webkit/glue/webplugin.h"
 
-// Helper function that always returns true. Used to prevent Gtk from
-// destroying our socket when the plug goes away: we manage it ourselves.
-static gboolean AlwaysTrue(void* unused) {
-  return TRUE;
-}
-
-gfx::PluginWindowHandle GtkPluginContainerManager::CreatePluginContainer() {
+GtkWidget* GtkPluginContainerManager::CreatePluginContainer(
+    gfx::PluginWindowHandle id) {
   DCHECK(host_widget_);
-  // If the current view hasn't been attached to a top-level window (e.g. it is
-  // loaded in a background tab), it can't be realized without asserting in
-  // Gtk, so we can't get the XID for the socket. Instead, don't create one.
-  // We'll never see the plugin but it's better than crashing.
-  // TODO(piman@google.com): figure out how to add the background tab to the
-  // widget hierarchy, so that it can be realized. It doesn't have to be
-  // visible.
-  if (!gtk_widget_get_ancestor(host_widget_, GTK_TYPE_WINDOW)) {
-    NOTIMPLEMENTED() << "Can't create plugins in background tabs.";
-    return 0;
-  }
-
-  GtkWidget* plugin_container = gtk_plugin_container_new();
-  g_signal_connect(G_OBJECT(plugin_container), "plug-removed",
-                   G_CALLBACK(AlwaysTrue), NULL);
-  // Add a connection to the "unrealize" signal so that if the parent widget
-  // gets destroyed before the DestroyPluginContainer gets called, bad things
-  // don't happen.
-  g_signal_connect(G_OBJECT(plugin_container), "unrealize",
-                   G_CALLBACK(UnrealizeCallback), this);
-  gtk_container_add(GTK_CONTAINER(host_widget_), plugin_container);
-  gtk_widget_show(plugin_container);
-  gtk_widget_realize(plugin_container);
-
-  gfx::PluginWindowHandle id = gtk_socket_get_id(GTK_SOCKET(plugin_container));
-
-  plugin_window_to_widget_map_.insert(std::make_pair(id, plugin_container));
-
-  return id;
+  GtkWidget *widget = gtk_plugin_container_new();
+  plugin_window_to_widget_map_.insert(std::make_pair(id, widget));
+
+  // The Realize callback is responsible for adding the plug into the socket.
+  // The reason is 2-fold:
+  // - the plug can't be added until the socket is realized, but this may not
+  // happen until the socket is attached to a top-level window, which isn't the
+  // case for background tabs.
+  // - when dragging tabs, the socket gets unrealized, which breaks the XEMBED
+  // connection. We need to make it again when the tab is reattached, and the
+  // socket gets realized again.
+  //
+  // Note, the RealizeCallback relies on the plugin_window_to_widget_map_ to
+  // have the mapping.
+  g_signal_connect(G_OBJECT(widget), "realize",
+                   G_CALLBACK(RealizeCallback), this);
+
+  // Don't destroy the widget when the plug is removed.
+  g_signal_connect(G_OBJECT(widget), "plug-removed",
+                   G_CALLBACK(gtk_true), NULL);
+
+  gtk_container_add(GTK_CONTAINER(host_widget_), widget);
+  gtk_widget_show(widget);
+
+  return widget;
 }
 
 void GtkPluginContainerManager::DestroyPluginContainer(
-    gfx::PluginWindowHandle container) {
-  GtkWidget* plugin_container = MapIDToWidget(container);
-  if (!plugin_container)
-    return;
+    gfx::PluginWindowHandle id) {
+  DCHECK(host_widget_);
+  GtkWidget* widget = MapIDToWidget(id);
+  if (widget)
+    gtk_widget_destroy(widget);
 
-  // This will call the UnrealizeCallback that will remove plugin_container
-  // from the map.
-  gtk_widget_destroy(plugin_container);
+  plugin_window_to_widget_map_.erase(id);
 }
 
 void GtkPluginContainerManager::MovePluginContainer(
@@ -117,7 +107,8 @@ void GtkPluginContainerManager::MovePluginContainer(
                                 move.window_rect.height());
 }
 
-GtkWidget* GtkPluginContainerManager::MapIDToWidget(gfx::PluginWindowHandle id) {
+GtkWidget* GtkPluginContainerManager::MapIDToWidget(
+    gfx::PluginWindowHandle id) {
   PluginWindowToWidgetMap::const_iterator i =
       plugin_window_to_widget_map_.find(id);
   if (i != plugin_window_to_widget_map_.end())
@@ -128,12 +119,26 @@ GtkWidget* GtkPluginContainerManager::MapIDToWidget(gfx::PluginWindowHandle id)
   return NULL;
 }
 
-void GtkPluginContainerManager::UnrealizeCallback(GtkWidget* widget,
-                                                  void* user_data) {
-  // This is the last chance to get the XID for the widget. Remove it from the
-  // map here.
+gfx::PluginWindowHandle GtkPluginContainerManager::MapWidgetToID(
+     GtkWidget* widget) {
+  for (PluginWindowToWidgetMap::const_iterator i =
+          plugin_window_to_widget_map_.begin();
+       i != plugin_window_to_widget_map_.end(); ++i) {
+    if (i->second == widget)
+      return i->first;
+  }
+
+  LOG(ERROR) << "Request for id for unknown widget";
+  return 0;
+}
+
+// static
+void GtkPluginContainerManager::RealizeCallback(GtkWidget* widget,
+                                                void* user_data) {
   GtkPluginContainerManager* plugin_container_manager =
       static_cast<GtkPluginContainerManager*>(user_data);
-  gfx::PluginWindowHandle id = gtk_socket_get_id(GTK_SOCKET(widget));
-  plugin_container_manager->plugin_window_to_widget_map_.erase(id);
+
+  gfx::PluginWindowHandle id = plugin_container_manager->MapWidgetToID(widget);
+  if (id)
+    gtk_socket_add_id(GTK_SOCKET(widget), id);
 }
diff --git a/webkit/glue/plugins/gtk_plugin_container_manager.h b/webkit/glue/plugins/gtk_plugin_container_manager.h
index 648e963..30a6b1a 100644
--- a/webkit/glue/plugins/gtk_plugin_container_manager.h
+++ b/webkit/glue/plugins/gtk_plugin_container_manager.h
@@ -21,28 +21,31 @@ class GtkPluginContainerManager {
   // Sets the widget that will host the plugin containers. Must be a GtkFixed.
   void set_host_widget(GtkWidget *widget) { host_widget_ = widget; }
 
-  // Creates a new plugin container, returning its XID.
-  gfx::PluginWindowHandle CreatePluginContainer();
+  // Creates a new plugin container, for a given plugin XID.
+  GtkWidget* CreatePluginContainer(gfx::PluginWindowHandle id);
 
-  // Destroys a plugin container, given its XID.
-  void DestroyPluginContainer(gfx::PluginWindowHandle container);
+  // Destroys a plugin container, given the plugin XID.
+  void DestroyPluginContainer(gfx::PluginWindowHandle id);
 
   // Takes an update from WebKit about a plugin's position and side and moves
   // the plugin accordingly.
   void MovePluginContainer(const WebPluginGeometry& move);
 
  private:
-  // Maps a plugin container XID to the corresponding widget.
+  // Maps a plugin XID to the corresponding container widget.
   GtkWidget* MapIDToWidget(gfx::PluginWindowHandle id);
 
-  // Callback for when the plugin container loses its XID, so that it can be
-  // removed from plugin_window_to_widget_map_.
-  static void UnrealizeCallback(GtkWidget *widget, void *user_data);
+  // Maps a container widget to the corresponding plugin XID.
+  gfx::PluginWindowHandle MapWidgetToID(GtkWidget* widget);
+
+  // Callback for when the plugin container gets realized, at which point it
+  // plugs the plugin XID.
+  static void RealizeCallback(GtkWidget *widget, void *user_data);
 
   // Parent of the plugin containers.
   GtkWidget* host_widget_;
 
-  // A map that associates plugin containers to their XID.
+  // A map that associates plugin containers to the plugin XID.
   typedef std::map<gfx::PluginWindowHandle, GtkWidget*> PluginWindowToWidgetMap;
   PluginWindowToWidgetMap plugin_window_to_widget_map_;
 };
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.h b/webkit/glue/plugins/webplugin_delegate_impl.h
index 106a040..90d8203 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.h
+++ b/webkit/glue/plugins/webplugin_delegate_impl.h
@@ -196,6 +196,13 @@ class WebPluginDelegateImpl : public WebPluginDelegate {
   GdkPixmap* pixmap_;
   double first_event_time_;
 
+  // On Linux some plugins assume that the GtkSocket container is in the same
+  // process. So we create a GtkPlug to plug into the browser's container, and
+  // a GtkSocket to hold the plugin. We then send the GtkPlug to the browser
+  // process.
+  GtkWidget* plug_;
+  GtkWidget* socket_;
+
   // Ensure pixmap_ exists and is at least width by height pixels.
   void EnsurePixmapAtLeastSize(int width, int height);
 #endif
diff --git a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
index da236b4..83d348b 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
@@ -68,6 +68,8 @@ WebPluginDelegateImpl::WebPluginDelegateImpl(
       instance_(instance),
       pixmap_(NULL),
       first_event_time_(-1.0),
+      plug_(NULL),
+      socket_(NULL),
       parent_(containing_view),
       quirks_(0) {
   memset(&window_, 0, sizeof(window_));
@@ -134,8 +136,10 @@ bool WebPluginDelegateImpl::Initialize(const GURL& url,
     if (!WindowedCreatePlugin())
       return false;
   }
+  gfx::PluginWindowHandle handle =
+      windowless_ ? 0 : gtk_plug_get_id(GTK_PLUG(plug_));
+  plugin->SetWindow(handle);
 
-  plugin->SetWindow(windowed_handle_);
   plugin_url_ = url.spec();
 
   return true;
@@ -258,6 +262,7 @@ void WebPluginDelegateImpl::WindowedUpdateGeometry(
 
 bool WebPluginDelegateImpl::WindowedCreatePlugin() {
   DCHECK(!windowed_handle_);
+  DCHECK(!plug_);
 
   bool xembed;
   NPError err = instance_->NPP_GetValue(NPPVpluginNeedsXEmbed, &xembed);
@@ -267,11 +272,22 @@ bool WebPluginDelegateImpl::WindowedCreatePlugin() {
     return false;
   }
 
-  // Xembed plugins need a window created for them browser-side.
-  // Do that now.
-  windowed_handle_ = plugin_->CreatePluginContainer();
-  if (!windowed_handle_)
-    return false;
+  // Passing 0 as the socket XID creates a plug without plugging it in a socket
+  // yet, so that it can be latter added with gtk_socket_add_id().
+  plug_ = gtk_plug_new(0);
+  gtk_widget_show(plug_);
+  socket_ = gtk_socket_new();
+  gtk_widget_show(socket_);
+  gtk_container_add(GTK_CONTAINER(plug_), socket_);
+  gtk_widget_show_all(plug_);
+
+  // Prevent the plug from being destroyed if the browser kills the container
+  // window.
+  g_signal_connect(plug_, "delete-event", G_CALLBACK(gtk_true), NULL);
+  // Prevent the socket from being destroyed when the plugin removes itself.
+  g_signal_connect(socket_, "plug_removed", G_CALLBACK(gtk_true), NULL);
+
+  windowed_handle_ = gtk_socket_get_id(GTK_SOCKET(socket_));
 
   window_.window = reinterpret_cast<void*>(windowed_handle_);
 
@@ -288,9 +304,12 @@ bool WebPluginDelegateImpl::WindowedCreatePlugin() {
 }
 
 void WebPluginDelegateImpl::WindowedDestroyWindow() {
-  if (windowed_handle_) {
-    plugin_->WillDestroyWindow(windowed_handle_);
+  if (plug_) {
+    plugin_->WillDestroyWindow(gtk_plug_get_id(GTK_PLUG(plug_)));
 
+    gtk_widget_destroy(plug_);
+    plug_ = NULL;
+    socket_ = NULL;
     windowed_handle_ = 0;
   }
 }
diff --git a/webkit/glue/webplugin.h b/webkit/glue/webplugin.h
index d5f0cf6..886b031 100644
--- a/webkit/glue/webplugin.h
+++ b/webkit/glue/webplugin.h
@@ -25,7 +25,8 @@ struct NPObject;
 // Describes the new location for a plugin window.
 struct WebPluginGeometry {
   // On Windows, this is the plugin window in the plugin process.
-  // On X11, this is the browser process's hosting window (the GtkSocket).
+  // On X11, this is the XID of the plugin-side GtkPlug containing the
+  // GtkSocket hosting the actual plugin window.
   gfx::PluginWindowHandle window;
   gfx::Rect window_rect;
   // Clip rect (include) and cutouts (excludes), relative to
@@ -51,13 +52,6 @@ class WebPlugin {
   WebPlugin() { }
   virtual ~WebPlugin() { }
 
-#if defined(OS_LINUX)
-  // Called by the plugin delegate to request a container for a new
-  // windowed plugin.  This handle will later get destroyed with
-  // WillDestroyWindow.
-  virtual gfx::PluginWindowHandle CreatePluginContainer() = 0;
-#endif
-
   // Called by the plugin delegate to let the WebPlugin know if the plugin is
   // windowed (i.e. handle is not NULL) or windowless (handle is NULL).  This
   // tells the WebPlugin to send mouse/keyboard events to the plugin delegate,
diff --git a/webkit/glue/webplugin_impl.cc b/webkit/glue/webplugin_impl.cc
index 3fe6b58..397fbd2 100644
--- a/webkit/glue/webplugin_impl.cc
+++ b/webkit/glue/webplugin_impl.cc
@@ -396,19 +396,17 @@ void WebPluginImpl::didFailLoading(const WebURLError& error) {
 WebPluginImpl::~WebPluginImpl() {
 }
 
-#if defined(OS_LINUX)
-gfx::PluginWindowHandle WebPluginImpl::CreatePluginContainer() {
-  WebViewDelegate* view_delegate = GetWebViewDelegate();
-  if (!view_delegate)
-    return 0;
-  return view_delegate->CreatePluginContainer();
-}
-#endif
 
 void WebPluginImpl::SetWindow(gfx::PluginWindowHandle window) {
   if (window) {
     DCHECK(!windowless_);  // Make sure not called twice.
     window_ = window;
+    WebViewDelegate* view_delegate = GetWebViewDelegate();
+    if (view_delegate) {
+      // Tell the view delegate that the plugin window was created, so that it
+      // can create necessary container widgets.
+      view_delegate->CreatedPluginWindow(window);
+    }
   } else {
     DCHECK(!window_);  // Make sure not called twice.
     windowless_ = true;
diff --git a/webkit/glue/webplugin_impl.h b/webkit/glue/webplugin_impl.h
index fd83ba9..7f98759 100644
--- a/webkit/glue/webplugin_impl.h
+++ b/webkit/glue/webplugin_impl.h
@@ -100,9 +100,6 @@ class WebPluginImpl : public WebPlugin,
   virtual void didFailLoading(const WebKit::WebURLError& error);
 
   // WebPlugin implementation:
-#if defined(OS_LINUX)
-  gfx::PluginWindowHandle CreatePluginContainer();
-#endif
   void SetWindow(gfx::PluginWindowHandle window);
   void WillDestroyWindow(gfx::PluginWindowHandle window);
 #if defined(OS_WIN)
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 826d1a7..6bf918c 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -149,9 +149,9 @@ class WebViewDelegate : virtual public WebKit::WebWidgetClient {
     return NULL;
   }
 
-  // Called when a windowed plugin is initializing, to request a container
-  // for the plugin.  Only used on Linux.
-  virtual gfx::PluginWindowHandle CreatePluginContainer() { return 0; }
+  // Called when a windowed plugin is created.
+  // Lets the view delegate create anything it is using to wrap the plugin.
+  virtual void CreatedPluginWindow(gfx::PluginWindowHandle handle) { }
 
   // Called when a windowed plugin is closing.
   // Lets the view delegate shut down anything it is using to wrap the plugin.
diff --git a/webkit/tools/test_shell/test_webview_delegate.h b/webkit/tools/test_shell/test_webview_delegate.h
index 6224b9c..971f199 100644
--- a/webkit/tools/test_shell/test_webview_delegate.h
+++ b/webkit/tools/test_shell/test_webview_delegate.h
@@ -77,8 +77,8 @@ class TestWebViewDelegate : public WebViewDelegate {
       const std::string& clsid,
       std::string* actual_mime_type);
 #if defined(OS_LINUX)
-  virtual gfx::PluginWindowHandle CreatePluginContainer();
-  virtual void WillDestroyPluginWindow(gfx::PluginWindowHandle handle);
+  virtual void CreatedPluginWindow(gfx::PluginWindowHandle id);
+  virtual void WillDestroyPluginWindow(gfx::PluginWindowHandle id);
 #endif
   virtual WebKit::WebMediaPlayer* CreateWebMediaPlayer(
       WebKit::WebMediaPlayerClient* client);
diff --git a/webkit/tools/test_shell/test_webview_delegate_gtk.cc b/webkit/tools/test_shell/test_webview_delegate_gtk.cc
index 460dceb..461d1d3 100644
--- a/webkit/tools/test_shell/test_webview_delegate_gtk.cc
+++ b/webkit/tools/test_shell/test_webview_delegate_gtk.cc
@@ -108,12 +108,12 @@ WebPluginDelegate* TestWebViewDelegate::CreatePluginDelegate(
   return WebPluginDelegateImpl::Create(info.path, mtype, plugin_parent);
 }
 
-gfx::PluginWindowHandle TestWebViewDelegate::CreatePluginContainer() {
-  return shell_->webViewHost()->CreatePluginContainer();
+void TestWebViewDelegate::CreatedPluginWindow(gfx::PluginWindowHandle id) {
+  shell_->webViewHost()->CreatePluginContainer(id);
 }
 
-void TestWebViewDelegate::WillDestroyPluginWindow(unsigned long id) {
-  shell_->webViewHost()->OnPluginWindowDestroyed(id);
+void TestWebViewDelegate::WillDestroyPluginWindow(gfx::PluginWindowHandle id) {
+  shell_->webViewHost()->DestroyPluginContainer(id);
 }
 
 void TestWebViewDelegate::ShowJavaScriptAlert(const std::wstring& message) {
diff --git a/webkit/tools/test_shell/webview_host.h b/webkit/tools/test_shell/webview_host.h
index 962633b..3c95e1d 100644
--- a/webkit/tools/test_shell/webview_host.h
+++ b/webkit/tools/test_shell/webview_host.h
@@ -32,12 +32,11 @@ class WebViewHost : public WebWidgetHost {
   WebView* webview() const;
 
 #if defined(OS_LINUX)
-  // Create a new plugin parent container, returning its X window id for
-  // embedders to use.
-  GdkNativeWindow CreatePluginContainer();
+  // Create a new plugin parent container for a given plugin XID.
+  void CreatePluginContainer(gfx::PluginWindowHandle id);
 
-  // Called when a plugin has been destroyed.  Lets us clean up our side.
-  void OnPluginWindowDestroyed(GdkNativeWindow id);
+  // Destroy the plugin parent container when a plugin has been destroyed.
+  void DestroyPluginContainer(gfx::PluginWindowHandle id);
 
   GtkPluginContainerManager* plugin_container_manager() {
     return &plugin_container_manager_;
diff --git a/webkit/tools/test_shell/webview_host_gtk.cc b/webkit/tools/test_shell/webview_host_gtk.cc
index 39a88ab..6554cc5 100644
--- a/webkit/tools/test_shell/webview_host_gtk.cc
+++ b/webkit/tools/test_shell/webview_host_gtk.cc
@@ -35,10 +35,10 @@ WebView* WebViewHost::webview() const {
   return static_cast<WebView*>(webwidget_);
 }
 
-GdkNativeWindow WebViewHost::CreatePluginContainer() {
-  return plugin_container_manager_.CreatePluginContainer();
+void WebViewHost::CreatePluginContainer(gfx::PluginWindowHandle id) {
+  plugin_container_manager_.CreatePluginContainer(id);
 }
 
-void WebViewHost::OnPluginWindowDestroyed(GdkNativeWindow id) {
+void WebViewHost::DestroyPluginContainer(gfx::PluginWindowHandle id) {
   plugin_container_manager_.DestroyPluginContainer(id);
 }