Merge r184273 - [EGL][X11] XPixmap created in GLContextEGL::createPix…

…mapContext() is leaked

https://bugs.webkit.org/show_bug.cgi?id=144909

Reviewed by Sergio Villar Senin and Žan Doberšek.

The pixmap is created and passed to eglCreatePixmapSurface(), but
never released. eglCreatePixmapSurface() doesn't take the
ownership of the pixmap, so we should explicitly free it when the
GLContextEGL is destroyed.

* platform/graphics/egl/GLContextEGL.cpp:
(WebCore::GLContextEGL::createPixmapContext): Use XUniquePixmap
and transfer the ownership to the context by using the new
constructor that receives a XUniquePixmap&&.
(WebCore::GLContextEGL::createContext): createPixmapContext() is
now only defined for X11.
(WebCore::GLContextEGL::GLContextEGL): New constructor that
receives a XUniquePixmap&&.
* platform/graphics/egl/GLContextEGL.h: Add new constructor and
initialize the cairo device when defined to simplify constructors.

Source/WebCore/ChangeLog

@@ -1,3 +1,26 @@
+2015-05-12  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [EGL][X11] XPixmap created in GLContextEGL::createPixmapContext() is leaked
+        https://bugs.webkit.org/show_bug.cgi?id=144909
+
+        Reviewed by Sergio Villar Senin and Žan Doberšek.
+
+        The pixmap is created and passed to eglCreatePixmapSurface(), but
+        never released. eglCreatePixmapSurface() doesn't take the
+        ownership of the pixmap, so we should explicitly free it when the
+        GLContextEGL is destroyed.
+
+        * platform/graphics/egl/GLContextEGL.cpp:
+        (WebCore::GLContextEGL::createPixmapContext): Use XUniquePixmap
+        and transfer the ownership to the context by using the new
+        constructor that receives a XUniquePixmap&&.
+        (WebCore::GLContextEGL::createContext): createPixmapContext() is
+        now only defined for X11.
+        (WebCore::GLContextEGL::GLContextEGL): New constructor that
+        receives a XUniquePixmap&&.
+        * platform/graphics/egl/GLContextEGL.h: Add new constructor and
+        initialize the cairo device when defined to simplify constructors.
+
 2015-05-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix the build with RESOURCE_TIMING disabled.

Source/WebCore/platform/graphics/egl/GLContextEGL.cpp

@@ -170,9 +170,9 @@ std::unique_ptr<GLContextEGL> GLContextEGL::createPbufferContext(EGLContext shar
     return std::make_unique<GLContextEGL>(context, surface, PbufferSurface);
 }
 
+#if PLATFORM(X11)
 std::unique_ptr<GLContextEGL> GLContextEGL::createPixmapContext(EGLContext sharingContext)
 {
-#if PLATFORM(X11)
     EGLDisplay display = sharedEGLDisplay();
     if (display == EGL_NO_DISPLAY)
         return nullptr;
@@ -198,18 +198,15 @@ std::unique_ptr<GLContextEGL> GLContextEGL::createPixmapContext(EGLContext shari
     }
 
     EGLSurface surface = eglCreatePixmapSurface(display, config, pixmap, 0);
-
     if (surface == EGL_NO_SURFACE) {
         XFreePixmap(sharedX11Display(), pixmap);
         eglDestroyContext(display, context);
         return nullptr;
     }
 
-    return std::make_unique<GLContextEGL>(context, surface, PixmapSurface);
-#else
-    return nullptr;
-#endif
+    return std::make_unique<GLContextEGL>(context, surface, pixmap);
 }
+#endif // PLATFORM(X11)
 
 std::unique_ptr<GLContextEGL> GLContextEGL::createContext(EGLNativeWindowType window, GLContext* sharingContext)
 {
@@ -229,25 +226,34 @@ std::unique_ptr<GLContextEGL> GLContextEGL::createContext(EGLNativeWindowType wi
 
     EGLContext eglSharingContext = sharingContext ? static_cast<GLContextEGL*>(sharingContext)->m_context : 0;
     auto context = window ? createWindowContext(window, sharingContext) : nullptr;
+#if PLATFORM(X11)
     if (!context)
         context = createPixmapContext(eglSharingContext);
-
+#endif
     if (!context)
         context = createPbufferContext(eglSharingContext);
-    
+
     return WTF::move(context);
 }
 
 GLContextEGL::GLContextEGL(EGLContext context, EGLSurface surface, EGLSurfaceType type)
     : m_context(context)
     , m_surface(surface)
     , m_type(type)
-#if USE(CAIRO)
-    , m_cairoDevice(0)
-#endif
 {
+    ASSERT(type != PixmapSurface);
 }
 
+#if PLATFORM(X11)
+GLContextEGL::GLContextEGL(EGLContext context, EGLSurface surface, Pixmap pixmap)
+    : m_context(context)
+    , m_surface(surface)
+    , m_type(PixmapSurface)
+    , m_pixmap(pixmap)
+{
+}
+#endif
+
 GLContextEGL::~GLContextEGL()
 {
 #if USE(CAIRO)
@@ -264,6 +270,11 @@ GLContextEGL::~GLContextEGL()
 
     if (m_surface)
         eglDestroySurface(display, m_surface);
+
+#if PLATFORM(X11)
+    if (m_pixmap)
+        XFreePixmap(sharedX11Display(), m_pixmap);
+#endif
 }
 
 bool GLContextEGL::canRenderToDefaultFramebuffer()

Source/WebCore/platform/graphics/egl/GLContextEGL.h

@@ -23,9 +23,12 @@
 #if USE(EGL)
 
 #include "GLContext.h"
-
 #include <EGL/egl.h>
 
+#if PLATFORM(X11)
+typedef unsigned long Pixmap;
+#endif
+
 namespace WebCore {
 
 class GLContextEGL : public GLContext {
@@ -36,6 +39,9 @@ class GLContextEGL : public GLContext {
     static std::unique_ptr<GLContextEGL> createWindowContext(EGLNativeWindowType, GLContext* sharingContext);
 
     GLContextEGL(EGLContext, EGLSurface, EGLSurfaceType);
+#if PLATFORM(X11)
+    GLContextEGL(EGLContext, EGLSurface, Pixmap);
+#endif
     virtual ~GLContextEGL();
     virtual bool makeContextCurrent();
     virtual void swapBuffers();
@@ -52,16 +58,21 @@ class GLContextEGL : public GLContext {
 
 private:
     static std::unique_ptr<GLContextEGL> createPbufferContext(EGLContext sharingContext);
+#if PLATFORM(X11)
     static std::unique_ptr<GLContextEGL> createPixmapContext(EGLContext sharingContext);
+#endif
 
     static void addActiveContext(GLContextEGL*);
     static void cleanupSharedEGLDisplay(void);
 
     EGLContext m_context;
     EGLSurface m_surface;
     EGLSurfaceType m_type;
+#if PLATFORM(X11)
+    Pixmap m_pixmap;
+#endif
 #if USE(CAIRO)
-    cairo_device_t* m_cairoDevice;
+    cairo_device_t* m_cairoDevice { nullptr };
 #endif
 };