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 a1.9.0.2=ss


git-svn-id: svn://10.0.0.236/trunk@253260 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/config/system-headers

@@ -221,6 +221,7 @@ gdk/gdk.h
 gdk/gdkkeysyms.h
 gdk/gdkpango.h
 gdk/gdkprivate.h
+gdk/gdkscreen.h
 gdk/gdkregion.h
 gdk/gdkwindow.h
 gdk/gdkx.h

mozilla/gfx/thebes/public/gfxXlibNativeRenderer.h

@@ -60,7 +60,8 @@ public:
      * @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;
   
@@ -79,11 +80,12 @@ public:
         // 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
+        // If set, then the Screen 'screen' in the callback can be different
-        // the display passed to 'Draw'
+        // from the default Screen of the display passed to 'Draw' and can be
-        DRAW_SUPPORTS_ALTERNATE_DISPLAY = 0x20
+        // on a different display.
+        DRAW_SUPPORTS_ALTERNATE_SCREEN = 0x20
     };
 
     struct DrawOutput {

mozilla/gfx/thebes/src/cairo-xlib-utils.c

@@ -214,7 +214,7 @@ _draw_with_xlib_direct (cairo_t *cr,
     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;
 
@@ -291,9 +291,9 @@ _draw_with_xlib_direct (cairo_t *cr,
     }
     
     /* Check that the display is supported */  
-    dpy = cairo_xlib_surface_get_display (target);
+    screen = cairo_xlib_surface_get_screen (target);
-    if (!(capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY) &&
+    if (!(capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN) &&
-        dpy != default_display) {
+        screen != DefaultScreenOfDisplay (default_display)) {
         CAIRO_XLIB_DRAWING_NOTE("TAKING SLOW PATH: non-default display\n");
         return False;
     }
@@ -301,7 +301,7 @@ _draw_with_xlib_direct (cairo_t *cr,
     /* 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;
     }
@@ -309,7 +309,7 @@ _draw_with_xlib_direct (cairo_t *cr,
     /* 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;
@@ -338,16 +338,15 @@ _create_temp_xlib_surface (cairo_t *cr, Display *dpy, int width, int height,
     /* 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 ||
+        if ((target_screen == screen ||
-             (capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY)) &&
+             (capabilities & CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN)) &&
-            (target_visual == visual ||
+            (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);
         }
     }
@@ -381,7 +380,7 @@ _draw_onto_temp_xlib_surface (cairo_surface_t *temp_xlib_surface,
                               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);
@@ -394,7 +393,7 @@ _draw_onto_temp_xlib_surface (cairo_surface_t *temp_xlib_surface,
     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;
 }

mozilla/gfx/thebes/src/cairo-xlib-utils.h

@@ -56,7 +56,7 @@ CAIRO_BEGIN_DECLS
  */
 typedef cairo_bool_t (* cairo_xlib_drawing_callback)
     (void *closure,
-     Display *dpy,
+     Screen *screen,
      Drawable drawable,
      Visual *visual,
      short offset_x, short offset_y,
@@ -97,19 +97,19 @@ typedef enum _cairo_xlib_drawing_opacity {
  * 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
+ * If CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN is set, then 'screen' can
- * any display, otherwise it will be the display passed into
+ * be any screen on any display, otherwise it will be the default screen of
- * cairo_draw_with_xlib.
+ * 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;
 

mozilla/gfx/thebes/src/gfxXlibNativeRenderer.cpp

@@ -40,6 +40,61 @@
 
 #include "cairo-xlib-utils.h"
 
+#ifdef MOZ_WIDGET_GTK2
+#include <gdk/gdkscreen.h>
+#include <gdk/gdkx.h>
+#endif
+
+// Look for an existing Colormap that is 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;
@@ -47,17 +102,46 @@ typedef struct {
 
 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);
 }
 
@@ -87,8 +171,8 @@ gfxXlibNativeRenderer::Draw(Display* dpy, gfxContext* ctx, int width, int height
     if (flags & DRAW_SUPPORTS_CLIP_LIST) {
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_CLIP_LIST;
     }
-    if (flags & DRAW_SUPPORTS_ALTERNATE_DISPLAY) {
+    if (flags & DRAW_SUPPORTS_ALTERNATE_SCREEN) {
-        cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_DISPLAY;
+        cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_ALTERNATE_SCREEN;
     }
     if (flags & DRAW_SUPPORTS_NONDEFAULT_VISUAL) {
         cairoFlags |= CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL;

mozilla/layout/generic/nsObjectFrame.cpp

@@ -483,7 +483,8 @@ private:
              const nsIntRect& aDirtyRect)
       : mWindow(aWindow), mInstance(aInstance), 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:
@@ -939,32 +940,7 @@ nsObjectFrame::CallSetWindow()
   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();
-  }
 
   // this will call pi->SetWindow and take care of window subclassing
   // if needed, see bug 132759.
@@ -4054,7 +4030,7 @@ void nsPluginInstanceOwner::Paint(nsIRenderingContext& aRenderingContext,
     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,
@@ -4076,9 +4052,24 @@ void nsPluginInstanceOwner::Paint(nsIRenderingContext& aRenderingContext,
                 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,
+nsPluginInstanceOwner::Renderer::NativeDraw(Screen* screen, Drawable drawable,
-                                            Visual* visual,
+                                            Visual* visual, Colormap colormap,
                                             short offsetX, short offsetY,
                                             XRectangle* clipRects,
                                             PRUint32 numClipRects)
@@ -4120,14 +4111,10 @@ nsPluginInstanceOwner::Renderer::NativeDraw(Display* dpy, Drawable drawable,
 
   NPSetWindowCallbackStruct* ws_info = 
     static_cast<NPSetWindowCallbackStruct*>(mWindow->ws_info);
-  if ( ws_info->visual != visual) {
+  if (ws_info->visual != visual || ws_info->colormap != colormap) {
-    // 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");
     ws_info->visual = visual;
+    ws_info->colormap = colormap;
+    ws_info->depth = DepthOfVisual(screen, visual);
     doupdatewindow = PR_TRUE;
   }
 
@@ -4138,7 +4125,7 @@ nsPluginInstanceOwner::Renderer::NativeDraw(Display* dpy, Drawable drawable,
   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;
@@ -4367,6 +4354,21 @@ NS_IMETHODIMP nsPluginInstanceOwner::CreateWidget(void)
           // 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);

mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp

@@ -625,8 +625,8 @@ public:
                 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,
+  nsresult NativeDraw(Screen* screen, Drawable drawable, Visual* visual,
-                      short offsetX, short offsetY,
+                      Colormap colormap, short offsetX, short offsetY,
                       XRectangle* clipRects, PRUint32 numClipRects);
 private:
   GtkWidgetState mState;
@@ -639,8 +639,8 @@ private:
 };
 
 nsresult
-ThemeRenderer::NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
+ThemeRenderer::NativeDraw(Screen* screen, Drawable drawable, Visual* visual,
-                          short offsetX, short offsetY,
+                          Colormap colormap, short offsetX, short offsetY,
                           XRectangle* clipRects, PRUint32 numClipRects)
 {
   GdkRectangle gdk_rect = mGDKRect;
@@ -651,7 +651,7 @@ ThemeRenderer::NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
   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;
 
@@ -659,17 +659,23 @@ ThemeRenderer::NativeDraw(Display* dpy, Drawable drawable, Visual* visual,
   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);
+      NS_ASSERTION(screen == GDK_SCREEN_XSCREEN(gdkScreen),
-      Colormap cmap = DefaultScreenOfDisplay(dpy)->cmap;
+                   "'screen' should be the default Screen");
-      GdkColormap* colormap =
+      // GDK requires a GdkColormap to be set on the GdkPixmap.
-        gdk_x11_colormap_foreign_new(gdkVisual, cmap);
+      GdkVisual* gdkVisual =
-                                             
+        gdk_x11_screen_lookup_visual(gdkScreen, visual->visualid);
-      gdk_drawable_set_colormap(gdkPixmap, colormap);
+      GdkColormap* gdkColormap =
+        gdk_x11_colormap_foreign_new(gdkVisual, colormap);
+      gdk_drawable_set_colormap(gdkPixmap, gdkColormap);
+      g_object_unref(G_OBJECT(gdkColormap));
     }
   }
 
@@ -800,7 +806,7 @@ nsNativeThemeGTK::DrawWidgetBackground(nsIRenderingContext* aContext,
   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