Bug 435764 – crash [@ cairo_draw_with_xlib] painting windowless plugins.
authorKarl Tomlinson <karlt+@karlt.net>
Tue, 08 Jul 2008 14:15:40 +1200
changeset 15703 9bbea3b66376
parent 15702 ee8961927fb4
child 15704 aae8e140f735
push id434
push userktomlinson@mozilla.com
push date2008-07-08 02:16 +0000
treeherdermozilla-central@9bbea3b66376 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs435764
milestone1.9.1a1pre
Bug 435764 – crash [@ cairo_draw_with_xlib] painting windowless plugins. Move ws_info set-up from nsObjectFrame::CallSetWindow(). Provide gfxXlibNativeRenderer::NativeDraw with Screen and Colormap. r+sr=roc
gfx/thebes/public/gfxXlibNativeRenderer.h
gfx/thebes/src/cairo-xlib-utils.c
gfx/thebes/src/cairo-xlib-utils.h
gfx/thebes/src/gfxXlibNativeRenderer.cpp
layout/generic/nsObjectFrame.cpp
widget/src/gtk2/nsNativeThemeGTK.cpp
--- a/gfx/thebes/public/gfxXlibNativeRenderer.h
+++ b/gfx/thebes/public/gfxXlibNativeRenderer.h
@@ -55,17 +55,18 @@ public:
     /**
      * Perform the native drawing.
      * @param offsetX draw at this offset into the given drawable
      * @param offsetY draw at this offset into the given drawable
      * @param clipRects an array of rects; clip to the union
      * @param numClipRects the number of rects in the array, or zero if
      * no clipping is required
      */
-    virtual nsresult NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
+    virtual nsresult NativeDraw(Screen* screen, Drawable drawable,
+                                Visual* visual, Colormap colormap,
                                 short offsetX, short offsetY,
                                 XRectangle* clipRects, PRUint32 numClipRects) = 0;
   
     enum {
         // If set, then Draw() is opaque, i.e., every pixel in the intersection
         // of the clipRect and (offset.x,offset.y,bounds.width,bounds.height)
         // will be set and there is no dependence on what the existing pixels
         // in the drawable are set to.
@@ -74,21 +75,22 @@ public:
         // only be called with offset==(0,0)
         DRAW_SUPPORTS_OFFSET = 0x02,
         // If set, then numClipRects can be zero or one
         DRAW_SUPPORTS_CLIP_RECT = 0x04,
         // If set, then numClipRects can be any value. If neither this
         // nor CLIP_RECT are set, then numClipRects will be zero
         DRAW_SUPPORTS_CLIP_LIST = 0x08,
         // If set, then the visual passed in can be any visual, otherwise the
-        // visual passed in must be the default visual for dpy's default screen
+        // visual passed in must be the default visual for 'screen'
         DRAW_SUPPORTS_NONDEFAULT_VISUAL = 0x10,
-        // If set, then the display 'dpy' in the callback can be different from
-        // the display passed to 'Draw'
-        DRAW_SUPPORTS_ALTERNATE_DISPLAY = 0x20
+        // If set, then the Screen 'screen' in the callback can be different
+        // from the default Screen of the display passed to 'Draw' and can be
+        // on a different display.
+        DRAW_SUPPORTS_ALTERNATE_SCREEN = 0x20,
     };
 
     struct DrawOutput {
         nsRefPtr<gfxASurface> mSurface;
         PRPackedBool mUniformAlpha;
         PRPackedBool mUniformColor;
         gfxRGBA      mColor;
     };
--- a/gfx/thebes/src/cairo-xlib-utils.c
+++ b/gfx/thebes/src/cairo-xlib-utils.c
@@ -209,17 +209,17 @@ static cairo_bool_t
     Drawable d;
     cairo_matrix_t matrix;
     short offset_x, offset_y;
     cairo_bool_t needs_clip;
     XRectangle rectangles[MAX_STATIC_CLIP_RECTANGLES];
     int rect_count;
     double device_offset_x, device_offset_y;
     int max_rectangles;
-    Display *dpy;
+    Screen *screen;
     Visual *visual;
     cairo_bool_t have_rectangular_clip;
 
     target = cairo_get_group_target (cr);
     cairo_surface_get_device_offset (target, &device_offset_x, &device_offset_y);
     d = cairo_xlib_surface_get_drawable (target);
 
     cairo_get_matrix (cr, &matrix);
@@ -286,35 +286,35 @@ static cairo_bool_t
        we might complete early above when when the object to be drawn is
        completely clipped out. */
     if (!d) {
         CAIRO_XLIB_DRAWING_NOTE("TAKING SLOW PATH: non-X surface\n");
         return False;
     }
     
     /* Check that the display is supported */  
-    dpy = cairo_xlib_surface_get_display (target);
-    if (!(capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY) &&
-        dpy != default_display) {
+    screen = cairo_xlib_surface_get_screen (target);
+    if (!(capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN) &&
+        screen != DefaultScreenOfDisplay (default_display)) {
         CAIRO_XLIB_DRAWING_NOTE("TAKING SLOW PATH: non-default display\n");
         return False;
     }
         
     /* Check that the visual is supported */  
     visual = cairo_xlib_surface_get_visual (target);
     if (!(capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL) &&
-        DefaultVisual (dpy, DefaultScreen (dpy)) != visual) {
+        DefaultVisualOfScreen (screen) != visual) {
         CAIRO_XLIB_DRAWING_NOTE("TAKING SLOW PATH: non-default visual\n");
         return False;
     }
   
     /* we're good to go! */
     CAIRO_XLIB_DRAWING_NOTE("TAKING FAST PATH\n");
     cairo_surface_flush (target);
-    callback (closure, dpy, d, visual, offset_x, offset_y, rectangles,
+    callback (closure, screen, d, visual, offset_x, offset_y, rectangles,
               needs_clip ? rect_count : 0);
     cairo_surface_mark_dirty (target);
     return True;
 }
 
 static cairo_surface_t *
 _create_temp_xlib_surface (cairo_t *cr, Display *dpy, int width, int height,
                            cairo_xlib_drawing_support_t capabilities)
@@ -333,26 +333,25 @@ static cairo_surface_t *
 
     Pixmap pixmap;
     cairo_surface_t *result;
     pixmap_free_struct *pfs;
     
     /* make the temporary surface match target_drawable to the extent supported
        by the native rendering code */
     if (target_drawable) {
-        Display *target_display = cairo_xlib_surface_get_display (target);
+        Screen *target_screen = cairo_xlib_surface_get_screen (target);
         Visual *target_visual = cairo_xlib_surface_get_visual (target);
-        if ((target_display == dpy ||
-             (capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY)) &&
-            (target_visual == visual ||
+        if ((target_screen == screen ||
+             (capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN)) &&
+            (target_visual == DefaultVisualOfScreen (target_screen) ||
              (capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL))) {
             drawable = target_drawable;
-            dpy = target_display;
+            dpy = cairo_xlib_surface_get_display (target);
             visual = target_visual;
-            screen = cairo_xlib_surface_get_screen (target);
             depth = cairo_xlib_surface_get_depth (target);
         }
     }
     
     pfs = malloc (sizeof(pixmap_free_struct));
     if (pfs == NULL)
         return NULL;
         
@@ -376,30 +375,30 @@ static cairo_surface_t *
 
 static cairo_bool_t
 _draw_onto_temp_xlib_surface (cairo_surface_t *temp_xlib_surface,
                               cairo_xlib_drawing_callback callback,
                               void *closure,
                               double background_gray_value)
 {
     Drawable d = cairo_xlib_surface_get_drawable (temp_xlib_surface);
-    Display *dpy = cairo_xlib_surface_get_display (temp_xlib_surface);
+    Screen *screen = cairo_xlib_surface_get_screen (temp_xlib_surface);
     Visual *visual = cairo_xlib_surface_get_visual (temp_xlib_surface);
     cairo_bool_t result;
     cairo_t *cr = cairo_create (temp_xlib_surface);
     cairo_set_source_rgb (cr, background_gray_value, background_gray_value,
                           background_gray_value);
     cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
     cairo_paint (cr);
     cairo_destroy (cr);
     
     cairo_surface_flush (temp_xlib_surface);
     /* no clipping is needed because the callback can't draw outside the native
        surface anyway */
-    result = callback (closure, dpy, d, visual, 0, 0, NULL, 0);
+    result = callback (closure, screen, d, visual, 0, 0, NULL, 0);
     cairo_surface_mark_dirty (temp_xlib_surface);
     return result;
 }
 
 static cairo_surface_t *
 _copy_xlib_surface_to_image (cairo_surface_t *temp_xlib_surface,
                              cairo_format_t format,
                              int width, int height,
--- a/gfx/thebes/src/cairo-xlib-utils.h
+++ b/gfx/thebes/src/cairo-xlib-utils.h
@@ -51,17 +51,17 @@ CAIRO_BEGIN_DECLS
  * than zero. This includes the assumption that the same RGBA image
  * is composited if you call the callback multiple times with the same closure,
  * display and visual during a single cairo_draw_with_xlib call.
  * 
  * @return True on success, False on non-recoverable error
  */
 typedef cairo_bool_t (* cairo_xlib_drawing_callback)
     (void *closure,
-     Display *dpy,
+     Screen *screen,
      Drawable drawable,
      Visual *visual,
      short offset_x, short offset_y,
      XRectangle* clip_rects, unsigned int num_rects);
 
 /**
  * This structure captures the result of the native drawing, in case the
  * caller may wish to reapply the drawing efficiently later.
@@ -92,29 +92,29 @@ typedef enum _cairo_xlib_drawing_opacity
  * can be nonzero in the call to the callback; otherwise they will be zero.
  * 
  * If CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_RECT is set, then 'num_rects' can be
  * zero or one in the call to the callback. If
  * CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_LIST is set, then 'num_rects' can be
  * anything in the call to the callback. Otherwise 'num_rects' will be zero.
  * Do not set both of these values.
  * 
- * If CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY is set, then 'dpy' can be
- * any display, otherwise it will be the display passed into
- * cairo_draw_with_xlib.
+ * If CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN is set, then 'screen' can
+ * be any screen on any display, otherwise it will be the default screen of
+ * the display passed into cairo_draw_with_xlib.
  * 
  * If CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL is set, then 'visual' can be
  * any visual, otherwise it will be equal to
- * DefaultVisual (dpy, DefaultScreen (dpy)).
+ * DefaultVisualOfScreen (screen).
  */
 typedef enum {
     CAIRO_XLIB_DRAWING_SUPPORTS_OFFSET = 0x01,
     CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_RECT = 0x02,
     CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_LIST = 0x04,
-    CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY = 0x08,
+    CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN = 0x08,
     CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL = 0x10
 } cairo_xlib_drawing_support_t;
 
 /**
  * Draw Xlib output into any cairo context. All cairo transforms and effects
  * are honored, including the current operator. This is equivalent to a
  * cairo_set_source_surface and then cairo_paint.
  * @param cr the context to draw into
--- a/gfx/thebes/src/gfxXlibNativeRenderer.cpp
+++ b/gfx/thebes/src/gfxXlibNativeRenderer.cpp
@@ -35,34 +35,118 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "gfxXlibNativeRenderer.h"
 #include "gfxContext.h"
 
 #include "cairo-xlib-utils.h"
 
+#ifdef MOZ_WIDGET_GTK2
+#include <gdk/gdkscreen.h>
+#include <gdk/gdkx.h>
+#endif
+
+// Look for an existing Colormap that known to be associated with visual.
+static Colormap
+LookupColormapForVisual(const Screen* screen, const Visual* visual)
+{
+    // common case
+    if (visual == DefaultVisualOfScreen(screen))
+        return DefaultColormapOfScreen(screen);
+
+#ifdef MOZ_WIDGET_GTK2
+    // I wish there were a gdk_x11_display_lookup_screen.
+    Display* dpy = DisplayOfScreen(screen);
+    GdkDisplay* gdkDpy = gdk_x11_lookup_xdisplay(dpy);
+    if (gdkDpy) {
+        gint screen_num = 0;
+        for (int s = 0; s < ScreenCount(dpy); ++s) {
+            if (ScreenOfDisplay(dpy, s) == screen) {
+                screen_num = s;
+                break;
+            }
+        }
+        GdkScreen* gdkScreen = gdk_display_get_screen(gdkDpy, screen_num);
+
+        GdkColormap* gdkColormap = NULL;
+        if (visual ==
+            GDK_VISUAL_XVISUAL(gdk_screen_get_rgb_visual(gdkScreen))) {
+            // widget/src/gtk2/mozcontainer.c uses gdk_rgb_get_colormap()
+            // which is inherited by child widgets, so this is the visual
+            // expected when drawing directly to widget surfaces or surfaces
+            // created using cairo_surface_create_similar with
+            // CAIRO_CONTENT_COLOR.
+            // gdk_screen_get_rgb_colormap is the generalization of
+            // gdk_rgb_get_colormap for any screen.
+            gdkColormap = gdk_screen_get_rgb_colormap(gdkScreen);
+        }
+        else if (visual ==
+             GDK_VISUAL_XVISUAL(gdk_screen_get_rgba_visual(gdkScreen))) {
+            // This is the visual expected on displays with the Composite
+            // extension enabled when the surface has been created using
+            // cairo_surface_create_similar with CAIRO_CONTENT_COLOR_ALPHA,
+            // as happens with non-unit opacity.
+            gdkColormap = gdk_screen_get_rgba_colormap(gdkScreen);
+        }
+        if (gdkColormap != NULL)
+            return GDK_COLORMAP_XCOLORMAP(gdkColormap);
+    }
+#endif
+
+    return None;
+}
+
 typedef struct {
     gfxXlibNativeRenderer* mRenderer;
     nsresult               mRV;
 } NativeRenderingClosure;
 
 static cairo_bool_t
 NativeRendering(void *closure,
-                Display *dpy,
+                Screen *screen,
                 Drawable drawable,
                 Visual *visual,
                 short offset_x, short offset_y,
                 XRectangle* rectangles, unsigned int num_rects)
 {
+    // Cairo doesn't provide a Colormap.
+    // See if a suitable existing Colormap is known.
+    Colormap colormap = LookupColormapForVisual(screen, visual);
+    PRBool allocColormap = colormap == None;
+    if (allocColormap) {
+        // No existing colormap found.
+        // This case is not expected with MOZ_WIDGET_GTK2.
+        // Create a Colormap for the Visual.
+        // This is only really useful for Visual classes with predefined
+        // Colormap entries, but cairo would be all confused with
+        // non-default non-TrueColor colormaps anyway.
+        NS_ASSERTION(visual->c_class == TrueColor ||
+                     visual->c_class == StaticColor ||
+                     visual->c_class == StaticGray,
+                     "Creating empty colormap");
+        // If this case were expected then it might be worth considering
+        // using a service that maintains a set of Colormaps for associated
+        // Visuals so as to avoid repeating the LockDisplay required in
+        // XCreateColormap, but then it's no worse than the XCreatePixmap
+        // that produced the Drawable here.
+        colormap = XCreateColormap(DisplayOfScreen(screen),
+                                   RootWindowOfScreen(screen),
+                                   visual, AllocNone);
+    }
+
     NativeRenderingClosure* cl = (NativeRenderingClosure*)closure;
     nsresult rv = cl->mRenderer->
-        NativeDraw(dpy, drawable, visual, offset_x, offset_y,
+        NativeDraw(screen, drawable, visual, colormap, offset_x, offset_y,
                    rectangles, num_rects);
     cl->mRV = rv;
+
+    if (allocColormap) {
+        XFreeColormap(DisplayOfScreen(screen), colormap);
+    }
     return NS_SUCCEEDED(rv);
 }
 
 nsresult
 gfxXlibNativeRenderer::Draw(Display* dpy, gfxContext* ctx, int width, int height,
                             PRUint32 flags, DrawOutput* output)
 {
     NativeRenderingClosure closure = { this, NS_OK };
@@ -82,18 +166,18 @@ gfxXlibNativeRenderer::Draw(Display* dpy
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_OFFSET;
     }
     if (flags & DRAW_SUPPORTS_CLIP_RECT) {
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_RECT;
     }
     if (flags & DRAW_SUPPORTS_CLIP_LIST) {
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_LIST;
     }
-    if (flags & DRAW_SUPPORTS_ALTERNATE_DISPLAY) {
-        cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY;
+    if (flags & DRAW_SUPPORTS_ALTERNATE_SCREEN) {
+        cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN;
     }
     if (flags & DRAW_SUPPORTS_NONDEFAULT_VISUAL) {
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL;
     }
     cairo_draw_with_xlib(ctx->GetCairo(), NativeRendering, &closure, dpy,
                          width, height,
                          (flags & DRAW_IS_OPAQUE) ? CAIRO_XLIB_DRAWING_OPAQUE
                                                   : CAIRO_XLIB_DRAWING_TRANSPARENT,
--- a/layout/generic/nsObjectFrame.cpp
+++ b/layout/generic/nsObjectFrame.cpp
@@ -480,17 +480,18 @@ private:
 #ifdef MOZ_X11
   class Renderer : public gfxXlibNativeRenderer {
   public:
     Renderer(nsPluginWindow* aWindow, nsIPluginInstance* aInstance,
              const nsIntSize& aPluginSize, const nsIntRect& aDirtyRect)
       : mWindow(aWindow), mInstance(aInstance),
         mPluginSize(aPluginSize), mDirtyRect(aDirtyRect)
     {}
-    virtual nsresult NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
+    virtual nsresult NativeDraw(Screen* screen, Drawable drawable,
+                                Visual* visual, Colormap colormap,
                                 short offsetX, short offsetY,
                                 XRectangle* clipRects, PRUint32 numClipRects);
   private:
     nsPluginWindow* mWindow;
     nsIPluginInstance* mInstance;
     const nsIntSize& mPluginSize;
     const nsIntRect& mDirtyRect;
   };
@@ -939,42 +940,17 @@ nsObjectFrame::CallSetWindow()
   PRBool windowless = (window->type == nsPluginWindowType_Drawable);
 
   nsPoint origin = GetWindowOriginInPixels(windowless);
 
   window->x = origin.x;
   window->y = origin.y;
 
   // refresh the plugin port as well
-#ifdef MOZ_X11
-  if(windowless) {
-    // There is no plugin port window but there are some extra fields to
-    // fill in.
-    nsIWidget* widget = GetWindow();
-    if (widget) {
-      NPSetWindowCallbackStruct* ws_info = 
-        static_cast<NPSetWindowCallbackStruct*>(window->ws_info);
-      ws_info->display =
-        static_cast<Display*>(widget->GetNativeData(NS_NATIVE_DISPLAY));
-#ifdef MOZ_WIDGET_GTK2
-      GdkWindow* gdkWindow =
-        static_cast<GdkWindow*>(widget->GetNativeData(NS_NATIVE_WINDOW));
-      GdkColormap* gdkColormap = gdk_drawable_get_colormap(gdkWindow);
-      ws_info->colormap = gdk_x11_colormap_get_xcolormap(gdkColormap);
-      GdkVisual* gdkVisual = gdk_colormap_get_visual(gdkColormap);
-      ws_info->visual = gdk_x11_visual_get_xvisual(gdkVisual);
-      ws_info->depth = gdkVisual->depth;
-#endif
-    }
-  }
-  else
-#endif
-  {
-    window->window = mInstanceOwner->GetPluginPort();
-  }
+  window->window = mInstanceOwner->GetPluginPort();
 
   // this will call pi->SetWindow and take care of window subclassing
   // if needed, see bug 132759.
   window->CallSetWindow(pi);
 
   mInstanceOwner->ReleasePluginPort((nsPluginPort *)window->window);
 }
 
@@ -4114,17 +4090,17 @@ void nsPluginInstanceOwner::Paint(gfxCon
   nsPluginWindow* window;
   GetWindow(window);
 
   Renderer renderer(window, mInstance, pluginSize, pluginDirtyRect);
   PRUint32 rendererFlags =
     Renderer::DRAW_SUPPORTS_OFFSET |
     Renderer::DRAW_SUPPORTS_CLIP_RECT |
     Renderer::DRAW_SUPPORTS_NONDEFAULT_VISUAL |
-    Renderer::DRAW_SUPPORTS_ALTERNATE_DISPLAY;
+    Renderer::DRAW_SUPPORTS_ALTERNATE_SCREEN;
 
   PRBool transparent = PR_TRUE;
   mInstance->GetValue(nsPluginInstanceVariable_TransparentBool,
                       (void *)&transparent);
   if (!transparent)
     rendererFlags |= Renderer::DRAW_IS_OPAQUE;
 
   // Renderer::Draw() draws a rectangle with top-left at the aContext origin.
@@ -4136,19 +4112,34 @@ void nsPluginInstanceOwner::Paint(gfxCon
   // here needs to be non-NULL for cairo_draw_with_xlib ->
   // _create_temp_xlib_surface -> DefaultScreen(dpy).
   NPSetWindowCallbackStruct* ws_info = 
     static_cast<NPSetWindowCallbackStruct*>(window->ws_info);
   renderer.Draw(ws_info->display, aContext, pluginSize.width, pluginSize.height,
                 rendererFlags, nsnull);
 }
 
+static int
+DepthOfVisual(const Screen* screen, const Visual* visual)
+{
+  for (int d = 0; d < screen->ndepths; d++) {
+    Depth *d_info = &screen->depths[d];
+    for (int v = 0; v < d_info->nvisuals; v++) {
+      if (&d_info->visuals[v] == visual)
+        return d_info->depth;
+    }
+  }
+
+  NS_ERROR("Visual not on Screen.");
+  return 0;
+}
+
 nsresult
-nsPluginInstanceOwner::Renderer::NativeDraw(Display* dpy, Drawable drawable,
-                                            Visual* visual,
+nsPluginInstanceOwner::Renderer::NativeDraw(Screen* screen, Drawable drawable,
+                                            Visual* visual, Colormap colormap,
                                             short offsetX, short offsetY,
                                             XRectangle* clipRects,
                                             PRUint32 numClipRects)
 {
   // See if the plugin must be notified of new window parameters.
   PRBool doupdatewindow = PR_FALSE;
 
   if (mWindow->x != offsetX || mWindow->y != offsetY) {
@@ -4187,35 +4178,31 @@ nsPluginInstanceOwner::Renderer::NativeD
       mWindow->clipRect.right   != newClipRect.right  ||
       mWindow->clipRect.bottom  != newClipRect.bottom) {
     mWindow->clipRect = newClipRect;
     doupdatewindow = PR_TRUE;
   }
 
   NPSetWindowCallbackStruct* ws_info = 
     static_cast<NPSetWindowCallbackStruct*>(mWindow->ws_info);
-  if ( ws_info->visual != visual) {
-    // NPAPI needs a colormap but the surface doesn't provide a colormap.  If
-    // gfxContent::CurrentSurface is a gfxXlibSurface then the visual here
-    // should be derived from that of the window and so the colormap of the
-    // window should be fine.  For other surfaces I don't know what to use.
-    NS_ASSERTION(ws_info->visual == visual,
-                 "Visual changed: colormap may not match");
+  if (ws_info->visual != visual || ws_info->colormap != colormap) {
     ws_info->visual = visual;
+    ws_info->colormap = colormap;
+    ws_info->depth = DepthOfVisual(screen, visual);
     doupdatewindow = PR_TRUE;
   }
 
   if (doupdatewindow)
       mInstance->SetWindow(mWindow);
 
   nsPluginEvent pluginEvent;
   XGraphicsExposeEvent& exposeEvent = pluginEvent.event.xgraphicsexpose;
   // set the drawing info
   exposeEvent.type = GraphicsExpose;
-  exposeEvent.display = dpy;
+  exposeEvent.display = DisplayOfScreen(screen);
   exposeEvent.drawable = drawable;
   exposeEvent.x = mDirtyRect.x + offsetX;
   exposeEvent.y = mDirtyRect.y + offsetY;
   exposeEvent.width  = mDirtyRect.width;
   exposeEvent.height = mDirtyRect.height;
   exposeEvent.count = 0;
   // information not set:
   exposeEvent.serial = 0;
@@ -4434,16 +4421,31 @@ NS_IMETHODIMP nsPluginInstanceOwner::Cre
         if (PR_TRUE == windowless) {
           mPluginWindow->type = nsPluginWindowType_Drawable;
 
           // this needs to be a HDC according to the spec, but I do
           // not see the right way to release it so let's postpone
           // passing HDC till paint event when it is really
           // needed. Change spec?
           mPluginWindow->window = nsnull;
+#ifdef MOZ_X11
+          // Fill in the display field.
+          nsIWidget* win = mOwner->GetWindow();
+          NPSetWindowCallbackStruct* ws_info = 
+            static_cast<NPSetWindowCallbackStruct*>(mPluginWindow->ws_info);
+          if (win) {
+            ws_info->display =
+              static_cast<Display*>(win->GetNativeData(NS_NATIVE_DISPLAY));
+          }
+#ifdef MOZ_WIDGET_GTK2
+          else {
+            ws_info->display = GDK_DISPLAY();
+          }
+#endif
+#endif
         } else if (mWidget) {
           mWidget->Resize(mPluginWindow->width, mPluginWindow->height,
                           PR_FALSE);
 
           // mPluginWindow->type is used in |GetPluginPort| so it must
           // be initialized first
           mPluginWindow->type = nsPluginWindowType_Window;
           mPluginWindow->window = GetPluginPort();
--- a/widget/src/gtk2/nsNativeThemeGTK.cpp
+++ b/widget/src/gtk2/nsNativeThemeGTK.cpp
@@ -623,61 +623,67 @@ NativeThemeErrorHandler(Display* dpy, XE
 
 class ThemeRenderer : public gfxXlibNativeRenderer {
 public:
   ThemeRenderer(GtkWidgetState aState, GtkThemeWidgetType aGTKWidgetType,
                 gint aFlags, GtkTextDirection aDirection,
                 const GdkRectangle& aGDKRect, const GdkRectangle& aGDKClip)
     : mState(aState), mGTKWidgetType(aGTKWidgetType), mFlags(aFlags),
       mDirection(aDirection), mGDKRect(aGDKRect), mGDKClip(aGDKClip) {}
-  nsresult NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
-                      short offsetX, short offsetY,
+  nsresult NativeDraw(Screen* screen, Drawable drawable, Visual* visual,
+                      Colormap colormap, short offsetX, short offsetY,
                       XRectangle* clipRects, PRUint32 numClipRects);
 private:
   GtkWidgetState mState;
   GtkThemeWidgetType mGTKWidgetType;
   gint mFlags;
   GtkTextDirection mDirection;
   GdkWindow* mWindow;
   const GdkRectangle& mGDKRect;
   const GdkRectangle& mGDKClip;
 };
 
 nsresult
-ThemeRenderer::NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
-                          short offsetX, short offsetY,
+ThemeRenderer::NativeDraw(Screen* screen, Drawable drawable, Visual* visual,
+                          Colormap colormap, short offsetX, short offsetY,
                           XRectangle* clipRects, PRUint32 numClipRects)
 {
   GdkRectangle gdk_rect = mGDKRect;
   gdk_rect.x += offsetX;
   gdk_rect.y += offsetY;
 
   GdkRectangle gdk_clip = mGDKClip;
   gdk_clip.x += offsetX;
   gdk_clip.y += offsetY;
   
-  GdkDisplay* gdkDpy = gdk_x11_lookup_xdisplay(dpy);
+  GdkDisplay* gdkDpy = gdk_x11_lookup_xdisplay(DisplayOfScreen(screen));
   if (!gdkDpy)
     return NS_ERROR_FAILURE;
 
   GdkPixmap* gdkPixmap = gdk_pixmap_lookup_for_display(gdkDpy, drawable);
   if (gdkPixmap) {
     g_object_ref(G_OBJECT(gdkPixmap));
   } else {
+    // XXX gtk+2.10 has gdk_pixmap_foreign_new_for_screen which would not
+    // use XGetGeometry.
     gdkPixmap = gdk_pixmap_foreign_new_for_display(gdkDpy, drawable);
     if (!gdkPixmap)
       return NS_ERROR_FAILURE;
     if (visual) {
+      // We requested that gfxXlibNativeRenderer give us the default screen
       GdkScreen* gdkScreen = gdk_display_get_default_screen(gdkDpy);
-      GdkVisual* gdkVisual = gdk_x11_screen_lookup_visual(gdkScreen, visual->visualid);
-      Colormap cmap = DefaultScreenOfDisplay(dpy)->cmap;
-      GdkColormap* colormap =
-        gdk_x11_colormap_foreign_new(gdkVisual, cmap);
-                                             
-      gdk_drawable_set_colormap(gdkPixmap, colormap);
+      NS_ASSERTION(screen == GDK_SCREEN_XSCREEN(gdkScreen),
+                   "'screen' should be the default Screen");
+      // GDK requires a GdkColormap to be set on the GdkPixmap.
+      GdkVisual* gdkVisual =
+        gdk_x11_screen_lookup_visual(gdkScreen, visual->visualid);
+      GdkColormap* gdkColormap =
+        gdk_x11_colormap_foreign_new(gdkVisual, colormap);
+      gdk_drawable_set_colormap(gdkPixmap, gdkColormap);
+      g_object_unref(G_OBJECT(gdkColormap));
     }
   }
 
   NS_ASSERTION(numClipRects == 0, "We don't support clipping!!!");
   moz_gtk_widget_paint(mGTKWidgetType, gdkPixmap, &gdk_rect, &gdk_clip, &mState,
                        mFlags, mDirection);
 
   g_object_unref(G_OBJECT(gdkPixmap));
@@ -798,17 +804,17 @@ nsNativeThemeGTK::DrawWidgetBackground(n
   if (!safeState) {
     gLastXError = 0;
     oldHandler = XSetErrorHandler(NativeThemeErrorHandler);
   }
 
   gfxContext* ctx = aContext->ThebesContext();
   gfxMatrix current = ctx->CurrentMatrix();
 
-  // We require the use of the default display and visual
+  // We require the use of the default screen and visual
   // because I'm afraid that otherwise the GTK theme may explode.
   // Some themes (e.g. Clearlooks) just don't clip properly to any
   // clip rect we provide, so we cannot advertise support for clipping within the
   // widget bounds. The gdk_clip is just advisory here, meanining "you don't
   // need to draw outside this rect if you don't feel like it!"
   GdkRectangle gdk_rect, gdk_clip;
   gfxRect gfx_rect = ConvertToGfxRect(aRect - drawingRect.TopLeft(), p2a);
   gfxRect gfx_clip = ConvertToGfxRect(drawingRect - drawingRect.TopLeft(), p2a);