[X11] Add XUniquePtr and XUniqueResource to automatically free X resources
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 May 2015 11:11:00 +0000 (11:11 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 May 2015 11:11:00 +0000 (11:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144521

Reviewed by Darin Adler.

Source/WebCore:

Add XUniquePtr as a template alias of std:unique_ptr to handle X
resources using pointers and XUniqueResource as a new class to
handle X resources using a long unsigned identifier. This
simplifies the code and makes it more difficult to leak X resources.

* PlatformEfl.cmake: Add new files to compilation.
* PlatformGTK.cmake: Ditto.
* platform/graphics/cairo/BackingStoreBackendCairoX11.cpp:
(WebCore::BackingStoreBackendCairoX11::BackingStoreBackendCairoX11):
Remove the display member, and use XUnique for Pixmap and GC.
(WebCore::BackingStoreBackendCairoX11::~BackingStoreBackendCairoX11):
Remove code to explicitly free Pixmap and GC.
(WebCore::BackingStoreBackendCairoX11::scroll):
* platform/graphics/cairo/BackingStoreBackendCairoX11.h:
* platform/graphics/glx/GLContextGLX.cpp:
(WebCore::GLContextGLX::createWindowContext): Use XUnique and the
new constructor that receives a XID, since there's no longer
conflict with the one receiving a Pbuffer.
(WebCore::GLContextGLX::createPbufferContext): Use XUnique and the
new constructor that receives a XUniqueGLXPbuffer&&.
(WebCore::GLContextGLX::createPixmapContext):
(WebCore::GLContextGLX::createContext):
(WebCore::GLContextGLX::GLContextGLX):
(WebCore::GLContextGLX::~GLContextGLX): Remove code to explicitly
free X resources.
(WebCore::GLContextGLX::makeContextCurrent):
(WebCore::GLContextGLX::cairoDevice):
(WebCore::GLContextGLX::platformContext):
* platform/graphics/glx/GLContextGLX.h:
* platform/graphics/surfaces/egl/EGLXSurface.cpp:
(WebCore::EGLXTransportSurfaceClient::EGLXTransportSurfaceClient):
(WebCore::EGLXTransportSurfaceClient::destroy):
(WebCore::EGLXTransportSurfaceClient::prepareTexture):
* platform/graphics/surfaces/egl/EGLXSurface.h:
* platform/graphics/surfaces/glx/GLXConfigSelector.h:
(WebCore::GLXConfigSelector::findMatchingConfig): Use XUnique
instead of the custom std::unique X11Deleter.
(WebCore::GLXConfigSelector::findMatchingConfigWithVisualId): Ditto.
* platform/graphics/surfaces/glx/GLXSurface.cpp:
(WebCore::GLXTransportSurface::GLXTransportSurface): Ditto.
(WebCore::GLXOffScreenSurface::initialize):
* platform/graphics/surfaces/glx/X11Helper.cpp:
(WebCore::X11Helper::createOffScreenWindow): Ditto.
(WebCore::X11Helper::createPixmap): Ditto.
* platform/graphics/surfaces/glx/X11Helper.h:
* platform/graphics/x11/XUniquePtr.h: Remove X11Deleter.
(WebCore::XPtrDeleter::operator()):
* platform/graphics/x11/XUniqueResource.cpp: Added.
(WebCore::XUniqueResource<XResource::Colormap>::deleteXResource):
(WebCore::XUniqueResource<XResource::Damage>::deleteXResource):
(WebCore::XUniqueResource<XResource::Pixmap>::deleteXResource):
(WebCore::XUniqueResource<XResource::Window>::deleteXResource):
(WebCore::XUniqueResource<XResource::GLXPbuffer>::deleteXResource):
(WebCore::XUniqueResource<XResource::GLXPixmap>::deleteXResource):
* platform/graphics/x11/XUniqueResource.h: Added.
(WebCore::XUniqueResource::XUniqueResource):
(WebCore::XUniqueResource::operator=):
(WebCore::XUniqueResource::~XUniqueResource):
(WebCore::XUniqueResource::get):
(WebCore::XUniqueResource::release):
(WebCore::XUniqueResource::reset):
(WebCore::XUniqueResource::operator!):
(WebCore::XUniqueResource::operator UnspecifiedBoolType*):

Source/WebKit2:

Use XUniquePtr and XUniqueResource to free X resources.

* PlatformEfl.cmake: Add Source/WebCore/platform/graphics/x11 dir
to the include dir list.
* PlatformGTK.cmake: Ditto.
* UIProcess/cairo/BackingStoreCairo.cpp:
(WebKit::BackingStore::createBackend): Do not pass the display to
the BackingStoreBackendCairoX11 constructor.
* UIProcess/gtk/RedirectedXCompositeWindow.cpp:
(WebKit::RedirectedXCompositeWindow::RedirectedXCompositeWindow):
(WebKit::RedirectedXCompositeWindow::~RedirectedXCompositeWindow):
(WebKit::RedirectedXCompositeWindow::resize):
(WebKit::RedirectedXCompositeWindow::cleanupPixmapAndPixmapSurface):
(WebKit::RedirectedXCompositeWindow::surface):
* UIProcess/gtk/RedirectedXCompositeWindow.h:
(WebKit::RedirectedXCompositeWindow::windowID):
* WebProcess/Plugins/Netscape/NetscapePlugin.h:
* WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:
(WebKit::NetscapePlugin::platformPostInitializeWindowless):
(WebKit::NetscapePlugin::platformDestroy):
(WebKit::NetscapePlugin::platformGeometryDidChange):
(WebKit::NetscapePlugin::platformPaint):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@184197 268f45cc-cd09-0410-ab3c-d52691b4dbfc

24 files changed:
Source/WebCore/ChangeLog
Source/WebCore/PlatformEfl.cmake
Source/WebCore/PlatformGTK.cmake
Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoX11.cpp
Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoX11.h
Source/WebCore/platform/graphics/glx/GLContextGLX.cpp
Source/WebCore/platform/graphics/glx/GLContextGLX.h
Source/WebCore/platform/graphics/surfaces/egl/EGLXSurface.cpp
Source/WebCore/platform/graphics/surfaces/egl/EGLXSurface.h
Source/WebCore/platform/graphics/surfaces/glx/GLXConfigSelector.h
Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.cpp
Source/WebCore/platform/graphics/surfaces/glx/X11Helper.cpp
Source/WebCore/platform/graphics/surfaces/glx/X11Helper.h
Source/WebCore/platform/graphics/x11/XUniquePtr.h [new file with mode: 0644]
Source/WebCore/platform/graphics/x11/XUniqueResource.cpp [new file with mode: 0644]
Source/WebCore/platform/graphics/x11/XUniqueResource.h [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/PlatformEfl.cmake
Source/WebKit2/PlatformGTK.cmake
Source/WebKit2/UIProcess/cairo/BackingStoreCairo.cpp
Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.cpp
Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.h
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h
Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp

index f09e96d..ceb0dfb 100644 (file)
@@ -1,3 +1,74 @@
+2015-05-08  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [X11] Add XUniquePtr and XUniqueResource to automatically free X resources
+        https://bugs.webkit.org/show_bug.cgi?id=144521
+
+        Reviewed by Darin Adler.
+
+        Add XUniquePtr as a template alias of std:unique_ptr to handle X
+        resources using pointers and XUniqueResource as a new class to
+        handle X resources using a long unsigned identifier. This
+        simplifies the code and makes it more difficult to leak X resources.
+
+        * PlatformEfl.cmake: Add new files to compilation.
+        * PlatformGTK.cmake: Ditto.
+        * platform/graphics/cairo/BackingStoreBackendCairoX11.cpp:
+        (WebCore::BackingStoreBackendCairoX11::BackingStoreBackendCairoX11):
+        Remove the display member, and use XUnique for Pixmap and GC.
+        (WebCore::BackingStoreBackendCairoX11::~BackingStoreBackendCairoX11):
+        Remove code to explicitly free Pixmap and GC.
+        (WebCore::BackingStoreBackendCairoX11::scroll):
+        * platform/graphics/cairo/BackingStoreBackendCairoX11.h:
+        * platform/graphics/glx/GLContextGLX.cpp:
+        (WebCore::GLContextGLX::createWindowContext): Use XUnique and the
+        new constructor that receives a XID, since there's no longer
+        conflict with the one receiving a Pbuffer.
+        (WebCore::GLContextGLX::createPbufferContext): Use XUnique and the
+        new constructor that receives a XUniqueGLXPbuffer&&.
+        (WebCore::GLContextGLX::createPixmapContext):
+        (WebCore::GLContextGLX::createContext):
+        (WebCore::GLContextGLX::GLContextGLX):
+        (WebCore::GLContextGLX::~GLContextGLX): Remove code to explicitly
+        free X resources.
+        (WebCore::GLContextGLX::makeContextCurrent):
+        (WebCore::GLContextGLX::cairoDevice):
+        (WebCore::GLContextGLX::platformContext):
+        * platform/graphics/glx/GLContextGLX.h:
+        * platform/graphics/surfaces/egl/EGLXSurface.cpp:
+        (WebCore::EGLXTransportSurfaceClient::EGLXTransportSurfaceClient):
+        (WebCore::EGLXTransportSurfaceClient::destroy):
+        (WebCore::EGLXTransportSurfaceClient::prepareTexture):
+        * platform/graphics/surfaces/egl/EGLXSurface.h:
+        * platform/graphics/surfaces/glx/GLXConfigSelector.h:
+        (WebCore::GLXConfigSelector::findMatchingConfig): Use XUnique
+        instead of the custom std::unique X11Deleter.
+        (WebCore::GLXConfigSelector::findMatchingConfigWithVisualId): Ditto.
+        * platform/graphics/surfaces/glx/GLXSurface.cpp:
+        (WebCore::GLXTransportSurface::GLXTransportSurface): Ditto.
+        (WebCore::GLXOffScreenSurface::initialize):
+        * platform/graphics/surfaces/glx/X11Helper.cpp:
+        (WebCore::X11Helper::createOffScreenWindow): Ditto.
+        (WebCore::X11Helper::createPixmap): Ditto.
+        * platform/graphics/surfaces/glx/X11Helper.h:
+        * platform/graphics/x11/XUniquePtr.h: Remove X11Deleter.
+        (WebCore::XPtrDeleter::operator()):
+        * platform/graphics/x11/XUniqueResource.cpp: Added.
+        (WebCore::XUniqueResource<XResource::Colormap>::deleteXResource):
+        (WebCore::XUniqueResource<XResource::Damage>::deleteXResource):
+        (WebCore::XUniqueResource<XResource::Pixmap>::deleteXResource):
+        (WebCore::XUniqueResource<XResource::Window>::deleteXResource):
+        (WebCore::XUniqueResource<XResource::GLXPbuffer>::deleteXResource):
+        (WebCore::XUniqueResource<XResource::GLXPixmap>::deleteXResource):
+        * platform/graphics/x11/XUniqueResource.h: Added.
+        (WebCore::XUniqueResource::XUniqueResource):
+        (WebCore::XUniqueResource::operator=):
+        (WebCore::XUniqueResource::~XUniqueResource):
+        (WebCore::XUniqueResource::get):
+        (WebCore::XUniqueResource::release):
+        (WebCore::XUniqueResource::reset):
+        (WebCore::XUniqueResource::operator!):
+        (WebCore::XUniqueResource::operator UnspecifiedBoolType*):
+
 2015-05-12  Zan Dobersek  <zdobersek@igalia.com>
 
         Move TransformOperation-based classes off of PassRefPtr
index 764b336..b0520f5 100644 (file)
@@ -197,6 +197,7 @@ list(APPEND WebCore_SOURCES
     platform/graphics/texmap/coordinated/UpdateAtlas.cpp
 
     platform/graphics/x11/PlatformDisplayX11.cpp
+    platform/graphics/x11/XUniqueResource.cpp
 
     platform/image-decoders/ImageDecoder.cpp
 
index b5d09b7..1f9f3bc 100644 (file)
@@ -126,6 +126,7 @@ list(APPEND WebCore_SOURCES
     platform/graphics/opentype/OpenTypeVerticalData.cpp
 
     platform/graphics/x11/PlatformDisplayX11.cpp
+    platform/graphics/x11/XUniqueResource.cpp
 
     platform/gtk/ErrorsGtk.cpp
     platform/gtk/EventLoopGtk.cpp
index 4c0872f..7b8631a 100644 (file)
 
 namespace WebCore {
 
-BackingStoreBackendCairoX11::BackingStoreBackendCairoX11(Display* display, unsigned long rootWindowID, Visual* visual, int depth, const IntSize& size, float deviceScaleFactor)
+BackingStoreBackendCairoX11::BackingStoreBackendCairoX11(unsigned long rootWindowID, Visual* visual, int depth, const IntSize& size, float deviceScaleFactor)
     : BackingStoreBackendCairo(size)
-    , m_display(display)
 {
     IntSize scaledSize = size;
     scaledSize.scale(deviceScaleFactor);
 
-    m_pixmap = XCreatePixmap(m_display, rootWindowID, scaledSize.width(), scaledSize.height(), depth);
-    m_gc = XCreateGC(m_display, m_pixmap, 0, nullptr);
+    auto* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
+    m_pixmap = XCreatePixmap(display, rootWindowID, scaledSize.width(), scaledSize.height(), depth);
+    m_gc.reset(XCreateGC(display, m_pixmap.get(), 0, nullptr));
 
-    m_surface = adoptRef(cairo_xlib_surface_create(m_display, m_pixmap, visual, scaledSize.width(), scaledSize.height()));
+    m_surface = adoptRef(cairo_xlib_surface_create(display, m_pixmap.get(), visual, scaledSize.width(), scaledSize.height()));
     cairoSurfaceSetDeviceScale(m_surface.get(), deviceScaleFactor, deviceScaleFactor);
 }
 
@@ -44,8 +44,6 @@ BackingStoreBackendCairoX11::~BackingStoreBackendCairoX11()
 {
     // The pixmap needs to exist when the surface is destroyed, so begin by clearing it.
     m_surface.clear();
-    XFreePixmap(m_display, m_pixmap);
-    XFreeGC(m_display, m_gc);
 }
 
 void BackingStoreBackendCairoX11::scroll(const IntRect& scrollRect, const IntSize& scrollOffset)
@@ -65,7 +63,7 @@ void BackingStoreBackendCairoX11::scroll(const IntRect& scrollRect, const IntSiz
     scaledScrollOffset.scale(xScale, yScale);
 
     cairo_surface_flush(m_surface.get());
-    XCopyArea(m_display, m_pixmap, m_pixmap, m_gc,
+    XCopyArea(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native(), m_pixmap.get(), m_pixmap.get(), m_gc.get(),
         targetRect.x() - scaledScrollOffset.width(), targetRect.y() - scaledScrollOffset.height(),
         targetRect.width(), targetRect.height(), targetRect.x(), targetRect.y());
     cairo_surface_mark_dirty_rectangle(m_surface.get(), targetRect.x(), targetRect.y(), targetRect.width(), targetRect.height());
index 1de0d84..3441809 100644 (file)
 #include "BackingStoreBackendCairo.h"
 
 #if USE(CAIRO) && PLATFORM(X11)
-#include <X11/Xlib.h>
+#include "XUniquePtr.h"
+#include "XUniqueResource.h"
 
 namespace WebCore {
 
 class BackingStoreBackendCairoX11 final : public BackingStoreBackendCairo {
 public:
-    BackingStoreBackendCairoX11(Display*, unsigned long rootWindowID, Visual*, int depth, const IntSize&, float deviceScaleFactor);
+    BackingStoreBackendCairoX11(unsigned long rootWindowID, Visual*, int depth, const IntSize&, float deviceScaleFactor);
     virtual ~BackingStoreBackendCairoX11();
 
     void scroll(const IntRect& scrollRect, const IntSize& scrollOffset) override;
 
 private:
-    Display* m_display;
-    Pixmap m_pixmap;
-    GC m_gc;
+    XUniquePixmap m_pixmap;
+    XUniqueGC m_gc;
 };
 
 } // namespace WebCore
index f7467a7..f05b6cf 100644 (file)
@@ -43,25 +43,19 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createWindowContext(XID window, GLCo
     visualInfo.visualid = XVisualIDFromVisual(attributes.visual);
 
     int numReturned = 0;
-    XVisualInfo* visualInfoList = XGetVisualInfo(display, VisualIDMask, &visualInfo, &numReturned);
-
-    GLXContext glxSharingContext = sharingContext ? static_cast<GLContextGLX*>(sharingContext)->m_context : 0;
-    GLXContext context = glXCreateContext(display, visualInfoList, glxSharingContext, True);
-    XFree(visualInfoList);
+    XUniquePtr<XVisualInfo> visualInfoList(XGetVisualInfo(display, VisualIDMask, &visualInfo, &numReturned));
 
+    GLXContext glxSharingContext = sharingContext ? static_cast<GLContextGLX*>(sharingContext)->m_context.get() : nullptr;
+    XUniqueGLXContext context(glXCreateContext(display, visualInfoList.get(), glxSharingContext, True));
     if (!context)
         return nullptr;
 
-    // GLXPbuffer and XID are both the same types underneath, so we have to share
-    // a constructor here with the window path.
-    auto contextWrapper = std::make_unique<GLContextGLX>(context);
-    contextWrapper->m_window = window;
-    return WTF::move(contextWrapper);
+    return std::make_unique<GLContextGLX>(WTF::move(context), window);
 }
 
 std::unique_ptr<GLContextGLX> GLContextGLX::createPbufferContext(GLXContext sharingContext)
 {
-    int fbConfigAttributes[] = {
+    static const int fbConfigAttributes[] = {
         GLX_DRAWABLE_TYPE, GLX_PBUFFER_BIT,
         GLX_RENDER_TYPE, GLX_RGBA_BIT,
         GLX_RED_SIZE, 1,
@@ -74,32 +68,21 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createPbufferContext(GLXContext shar
 
     int returnedElements;
     Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
-    GLXFBConfig* configs = glXChooseFBConfig(display, 0, fbConfigAttributes, &returnedElements);
-    if (!returnedElements) {
-        XFree(configs);
+    XUniquePtr<GLXFBConfig> configs(glXChooseFBConfig(display, 0, fbConfigAttributes, &returnedElements));
+    if (!returnedElements)
         return nullptr;
-    }
 
     // We will be rendering to a texture, so our pbuffer does not need to be large.
     static const int pbufferAttributes[] = { GLX_PBUFFER_WIDTH, 1, GLX_PBUFFER_HEIGHT, 1, 0 };
-    GLXPbuffer pbuffer = glXCreatePbuffer(display, configs[0], pbufferAttributes);
-    if (!pbuffer) {
-        XFree(configs);
+    XUniqueGLXPbuffer pbuffer(glXCreatePbuffer(display, configs.get()[0], pbufferAttributes));
+    if (!pbuffer)
         return nullptr;
-    }
 
-    GLXContext context = glXCreateNewContext(display, configs[0], GLX_RGBA_TYPE, sharingContext, GL_TRUE);
-    XFree(configs);
-    if (!context) {
-        glXDestroyPbuffer(display, pbuffer);
+    XUniqueGLXContext context(glXCreateNewContext(display, configs.get()[0], GLX_RGBA_TYPE, sharingContext, GL_TRUE));
+    if (!context)
         return nullptr;
-    }
 
-    // GLXPbuffer and XID are both the same types underneath, so we have to share
-    // a constructor here with the window path.
-    auto contextWrapper = std::make_unique<GLContextGLX>(context);
-    contextWrapper->m_pbuffer = pbuffer;
-    return WTF::move(contextWrapper);
+    return std::make_unique<GLContextGLX>(WTF::move(context), WTF::move(pbuffer));
 }
 
 std::unique_ptr<GLContextGLX> GLContextGLX::createPixmapContext(GLXContext sharingContext)
@@ -114,31 +97,23 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createPixmapContext(GLXContext shari
     };
 
     Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
-    XVisualInfo* visualInfo = glXChooseVisual(display, DefaultScreen(display), visualAttributes);
+    XUniquePtr<XVisualInfo> visualInfo(glXChooseVisual(display, DefaultScreen(display), visualAttributes));
     if (!visualInfo)
         return nullptr;
 
-    GLXContext context = glXCreateContext(display, visualInfo, sharingContext, GL_TRUE);
-    if (!context) {
-        XFree(visualInfo);
+    XUniqueGLXContext context(glXCreateContext(display, visualInfo.get(), sharingContext, GL_TRUE));
+    if (!context)
         return nullptr;
-    }
 
-    Pixmap pixmap = XCreatePixmap(display, DefaultRootWindow(display), 1, 1, visualInfo->depth);
-    if (!pixmap) {
-        XFree(visualInfo);
+    XUniquePixmap pixmap(XCreatePixmap(display, DefaultRootWindow(display), 1, 1, visualInfo->depth));
+    if (!pixmap)
         return nullptr;
-    }
 
-    GLXPixmap glxPixmap = glXCreateGLXPixmap(display, visualInfo, pixmap);
-    if (!glxPixmap) {
-        XFreePixmap(display, pixmap);
-        XFree(visualInfo);
+    XUniqueGLXPixmap glxPixmap(glXCreateGLXPixmap(display, visualInfo.get(), pixmap.get()));
+    if (!glxPixmap)
         return nullptr;
-    }
 
-    XFree(visualInfo);
-    return std::make_unique<GLContextGLX>(context, pixmap, glxPixmap);
+    return std::make_unique<GLContextGLX>(WTF::move(context), WTF::move(pixmap), WTF::move(glxPixmap));
 }
 
 std::unique_ptr<GLContextGLX> GLContextGLX::createContext(XID window, GLContext* sharingContext)
@@ -152,7 +127,7 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createContext(XID window, GLContext*
     if (!success)
         return nullptr;
 
-    GLXContext glxSharingContext = sharingContext ? static_cast<GLContextGLX*>(sharingContext)->m_context : 0;
+    GLXContext glxSharingContext = sharingContext ? static_cast<GLContextGLX*>(sharingContext)->m_context.get() : nullptr;
     auto context = window ? createWindowContext(window, sharingContext) : nullptr;
     if (!context)
         context = createPbufferContext(glxSharingContext);
@@ -164,23 +139,22 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createContext(XID window, GLContext*
     return WTF::move(context);
 }
 
-GLContextGLX::GLContextGLX(GLXContext context)
-    : m_context(context)
-    , m_window(0)
-    , m_pbuffer(0)
-    , m_pixmap(0)
-    , m_glxPixmap(0)
-    , m_cairoDevice(0)
+GLContextGLX::GLContextGLX(XUniqueGLXContext&& context, XID window)
+    : m_context(WTF::move(context))
+    , m_window(window)
 {
 }
 
-GLContextGLX::GLContextGLX(GLXContext context, Pixmap pixmap, GLXPixmap glxPixmap)
-    : m_context(context)
-    , m_window(0)
-    , m_pbuffer(0)
-    , m_pixmap(pixmap)
-    , m_glxPixmap(glxPixmap)
-    , m_cairoDevice(0)
+GLContextGLX::GLContextGLX(XUniqueGLXContext&& context, XUniqueGLXPbuffer&& pbuffer)
+    : m_context(WTF::move(context))
+    , m_pbuffer(WTF::move(pbuffer))
+{
+}
+
+GLContextGLX::GLContextGLX(XUniqueGLXContext&& context, XUniquePixmap&& pixmap, XUniqueGLXPixmap&& glxPixmap)
+    : m_context(WTF::move(context))
+    , m_pixmap(WTF::move(pixmap))
+    , m_glxPixmap(WTF::move(glxPixmap))
 {
 }
 
@@ -189,26 +163,12 @@ GLContextGLX::~GLContextGLX()
     if (m_cairoDevice)
         cairo_device_destroy(m_cairoDevice);
 
-    Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
     if (m_context) {
         // This may be necessary to prevent crashes with NVidia's closed source drivers. Originally
         // from Mozilla's 3D canvas implementation at: http://bitbucket.org/ilmari/canvas3d/
+        Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
         glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
         glXMakeCurrent(display, None, None);
-        glXDestroyContext(display, m_context);
-    }
-
-    if (m_pbuffer) {
-        glXDestroyPbuffer(display, m_pbuffer);
-        m_pbuffer = 0;
-    }
-    if (m_glxPixmap) {
-        glXDestroyGLXPixmap(display, m_glxPixmap);
-        m_glxPixmap = 0;
-    }
-    if (m_pixmap) {
-        XFreePixmap(display, m_pixmap);
-        m_pixmap = 0;
     }
 }
 
@@ -237,17 +197,17 @@ bool GLContextGLX::makeContextCurrent()
     ASSERT(m_context && (m_window || m_pbuffer || m_glxPixmap));
 
     GLContext::makeContextCurrent();
-    if (glXGetCurrentContext() == m_context)
+    if (glXGetCurrentContext() == m_context.get())
         return true;
 
     Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
     if (m_window)
-        return glXMakeCurrent(display, m_window, m_context);
+        return glXMakeCurrent(display, m_window, m_context.get());
 
     if (m_pbuffer)
-        return glXMakeCurrent(display, m_pbuffer, m_context);
+        return glXMakeCurrent(display, m_pbuffer.get(), m_context.get());
 
-    return ::glXMakeCurrent(display, m_glxPixmap, m_context);
+    return ::glXMakeCurrent(display, m_glxPixmap.get(), m_context.get());
 }
 
 void GLContextGLX::swapBuffers()
@@ -267,7 +227,7 @@ cairo_device_t* GLContextGLX::cairoDevice()
         return m_cairoDevice;
 
 #if ENABLE(ACCELERATED_2D_CANVAS)
-    m_cairoDevice = cairo_glx_device_create(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native(), m_context);
+    m_cairoDevice = cairo_glx_device_create(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native(), m_context.get());
 #endif
 
     return m_cairoDevice;
@@ -276,7 +236,7 @@ cairo_device_t* GLContextGLX::cairoDevice()
 #if ENABLE(GRAPHICS_CONTEXT_3D)
 PlatformGraphicsContext3D GLContextGLX::platformContext()
 {
-    return m_context;
+    return m_context.get();
 }
 #endif
 
index 6934f24..647f998 100644 (file)
 #if USE(GLX)
 
 #include "GLContext.h"
+#include "XUniquePtr.h"
+#include "XUniqueResource.h"
 
-typedef struct __GLXcontextRec* GLXContext;
-typedef unsigned long GLXPbuffer;
-typedef unsigned long GLXPixmap;
 typedef unsigned char GLubyte;
-typedef unsigned long Pixmap;
 typedef unsigned long XID;
 typedef void* ContextKeyType;
 
@@ -40,8 +38,9 @@ public:
     static std::unique_ptr<GLContextGLX> createContext(XID window, GLContext* sharingContext);
     static std::unique_ptr<GLContextGLX> createWindowContext(XID window, GLContext* sharingContext);
 
-    GLContextGLX(GLXContext);
-    GLContextGLX(GLXContext, Pixmap, GLXPixmap);
+    GLContextGLX(XUniqueGLXContext&&, XID);
+    GLContextGLX(XUniqueGLXContext&&, XUniqueGLXPbuffer&&);
+    GLContextGLX(XUniqueGLXContext&&, XUniquePixmap&&, XUniqueGLXPixmap&&);
     virtual ~GLContextGLX();
     virtual bool makeContextCurrent();
     virtual void swapBuffers();
@@ -59,12 +58,12 @@ private:
     static std::unique_ptr<GLContextGLX> createPbufferContext(GLXContext sharingContext);
     static std::unique_ptr<GLContextGLX> createPixmapContext(GLXContext sharingContext);
 
-    GLXContext m_context;
-    XID m_window;
-    GLXPbuffer m_pbuffer;
-    Pixmap m_pixmap;
-    GLXPixmap m_glxPixmap;
-    cairo_device_t* m_cairoDevice;
+    XUniqueGLXContext m_context;
+    XID m_window { 0 };
+    XUniqueGLXPbuffer m_pbuffer;
+    XUniquePixmap m_pixmap;
+    XUniqueGLXPixmap m_glxPixmap;
+    cairo_device_t* m_cairoDevice { nullptr };
 };
 
 } // namespace WebCore
index 618fdc3..cb77568 100644 (file)
@@ -141,7 +141,6 @@ void EGLPixmapSurface::destroy()
 
 EGLXTransportSurfaceClient::EGLXTransportSurfaceClient(const PlatformBufferHandle handle, const IntSize& size, bool hasAlpha)
     : GLTransportSurfaceClient()
-    , m_image(0)
     , m_size(size)
     , m_totalBytes(0)
 {
@@ -197,11 +196,7 @@ void EGLXTransportSurfaceClient::destroy()
     }
 
     eglWaitGL();
-
-    if (m_image) {
-        XDestroyImage(m_image);
-        m_image = 0;
-    }
+    m_image = nullptr;
 }
 
 void EGLXTransportSurfaceClient::prepareTexture()
@@ -214,7 +209,7 @@ void EGLXTransportSurfaceClient::prepareTexture()
     }
 
     // Fallback to use XImage in case EGLImage and TextureToPixmap are not supported.
-    m_image = XGetImage(NativeWrapper::nativeDisplay(), m_handle, 0, 0, m_size.width(), m_size.height(), AllPlanes, ZPixmap);
+    m_image.reset(XGetImage(NativeWrapper::nativeDisplay(), m_handle, 0, 0, m_size.width(), m_size.height(), AllPlanes, ZPixmap));
 
 #if USE(OPENGL_ES_2)
     if (m_format != GraphicsContext3D::BGRA) {
@@ -225,10 +220,7 @@ void EGLXTransportSurfaceClient::prepareTexture()
 
     glTexImage2D(GL_TEXTURE_2D, 0, m_format, m_size.width(), m_size.height(), 0, m_format, GL_UNSIGNED_BYTE, m_image->data);
 
-    if (m_image) {
-        XDestroyImage(m_image);
-        m_image = 0;
-    }
+    m_image = nullptr;
 }
 
 EGLTextureFromPixmap::EGLTextureFromPixmap(const NativePixmap handle, bool hasAlpha, EGLConfig config)
index 96c8a66..b421e17 100644 (file)
 #if PLATFORM(X11) && USE(EGL) && USE(GRAPHICS_SURFACE)
 
 #include "EGLSurface.h"
+#include "XUniquePtr.h"
 #include <glx/X11Helper.h>
 
 namespace WebCore {
 
-typedef X11Helper NativeWrapper;
 typedef Pixmap NativePixmap;
 
 // Contents of the surface are backed by native window.
@@ -74,7 +74,7 @@ public:
     virtual void destroy() override;
 
 private:
-    XImage* m_image;
+    XUniquePtr<XImage> m_image;
     IntSize m_size;
     PlatformBufferHandle m_handle;
     GLuint m_format;
index 5cdede5..145831d 100644 (file)
@@ -29,6 +29,7 @@
 #if USE(GLX)
 
 #include "X11Helper.h"
+#include "XUniquePtr.h"
 #include <opengl/GLDefs.h>
 #include <opengl/GLPlatformSurface.h>
 
@@ -140,13 +141,13 @@ private:
     GLXFBConfig findMatchingConfig(const int attributes[], int depth = 32)
     {
         int numAvailableConfigs;
-        std::unique_ptr<GLXFBConfig[], X11Deleter> temp(glXChooseFBConfig(X11Helper::nativeDisplay(), DefaultScreen(X11Helper::nativeDisplay()), attributes, &numAvailableConfigs));
+        XUniquePtr<GLXFBConfig> temp(glXChooseFBConfig(X11Helper::nativeDisplay(), DefaultScreen(X11Helper::nativeDisplay()), attributes, &numAvailableConfigs));
 
         if (!numAvailableConfigs || !temp.get())
             return 0;
 
         for (int i = 0; i < numAvailableConfigs; ++i) {
-            std::unique_ptr<XVisualInfo, X11Deleter> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp[i]) };
+            XUniquePtr<XVisualInfo> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp.get()[i]) };
             if (!scopedVisualInfo.get())
                 continue;
 
@@ -157,40 +158,40 @@ private:
                 if (format) {
                     if (m_attributes & GLPlatformSurface::SupportAlpha) {
                         if (scopedVisualInfo->depth == depth && format->direct.alphaMask > 0)
-                            return temp[i];
+                            return temp.get()[i];
                     } else if (!format->direct.alphaMask)
-                        return temp[i];
+                        return temp.get()[i];
                 }
             }
 #endif
             if (scopedVisualInfo->depth == depth)
-                return temp[i];
+                return temp.get()[i];
         }
 
         // Did not find any visual supporting alpha, select the first available config.
-        std::unique_ptr<XVisualInfo, X11Deleter> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp[0]) };
+        XUniquePtr<XVisualInfo> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp.get()[0]) };
 
         if ((m_attributes & GLPlatformSurface::SupportAlpha) && (scopedVisualInfo->depth != 32))
             m_attributes &= ~GLPlatformSurface::SupportAlpha;
 
-        return temp[0];
+        return temp.get()[0];
     }
 
     GLXFBConfig findMatchingConfigWithVisualId(const int attributes[], int depth, VisualID id)
     {
         int numAvailableConfigs;
-        std::unique_ptr<GLXFBConfig[], X11Deleter> temp(glXChooseFBConfig(X11Helper::nativeDisplay(), DefaultScreen(X11Helper::nativeDisplay()), attributes, &numAvailableConfigs));
+        XUniquePtr<GLXFBConfig> temp(glXChooseFBConfig(X11Helper::nativeDisplay(), DefaultScreen(X11Helper::nativeDisplay()), attributes, &numAvailableConfigs));
 
         if (!numAvailableConfigs || !temp.get())
             return 0;
 
         for (int i = 0; i < numAvailableConfigs; ++i) {
-            std::unique_ptr<XVisualInfo, X11Deleter> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp[i]) };
+            XUniquePtr<XVisualInfo> scopedVisualInfo { glXGetVisualFromFBConfig(X11Helper::nativeDisplay(), temp.get()[i]) };
             if (!scopedVisualInfo.get())
                 continue;
 
             if (id && scopedVisualInfo->depth == depth && scopedVisualInfo->visualid == id)
-                return temp[i];
+                return temp.get()[i];
         }
 
         return 0;
index ba493b0..541e703 100644 (file)
@@ -28,6 +28,8 @@
 
 #if USE(GLX)
 
+#include "XUniquePtr.h"
+
 namespace WebCore {
 
 static PFNGLXBINDTEXIMAGEEXTPROC pGlXBindTexImageEXT = 0;
@@ -66,7 +68,7 @@ GLXTransportSurface::GLXTransportSurface(const IntSize& size, SurfaceAttributes
 {
     attributes |= GLPlatformSurface::DoubleBuffered;
     m_configSelector = std::make_unique<GLXConfigSelector>(attributes);
-    std::unique_ptr<XVisualInfo, X11Deleter> visInfo(m_configSelector->visualInfo(m_configSelector->surfaceContextConfig()));
+    XUniquePtr<XVisualInfo> visInfo(m_configSelector->visualInfo(m_configSelector->surfaceContextConfig()));
 
     if (!visInfo.get()) {
         destroy();
@@ -142,7 +144,7 @@ void GLXOffScreenSurface::initialize(SurfaceAttributes attributes)
 {
     m_configSelector = std::make_unique<GLXConfigSelector>(attributes);
 
-    std::unique_ptr<XVisualInfo, X11Deleter> visualInfo(m_configSelector->visualInfo(m_configSelector->pixmapContextConfig()));
+    XUniquePtr<XVisualInfo> visualInfo(m_configSelector->visualInfo(m_configSelector->pixmapContextConfig()));
     X11Helper::createPixmap(&m_pixmap, *visualInfo.get());
 
     if (!m_pixmap) {
index 8010b57..fa8c4c0 100644 (file)
@@ -27,6 +27,7 @@
 #include "X11Helper.h"
 
 #include "PlatformDisplayX11.h"
+#include "XUniquePtr.h"
 
 namespace WebCore {
 
@@ -223,7 +224,7 @@ void X11Helper::createOffScreenWindow(uint32_t* handleId, const EGLint id, bool
     memset(&visualInfoTemplate, 0, sizeof(XVisualInfo));
     visualInfoTemplate.visualid = visualId;
     int matchingCount = 0;
-    std::unique_ptr<XVisualInfo, X11Deleter> matchingVisuals(XGetVisualInfo(nativeDisplay(), VisualIDMask, &visualInfoTemplate, &matchingCount));
+    XUniquePtr<XVisualInfo> matchingVisuals(XGetVisualInfo(nativeDisplay(), VisualIDMask, &visualInfoTemplate, &matchingCount));
     XVisualInfo* foundVisual = 0;
 
     if (matchingVisuals) {
@@ -260,7 +261,7 @@ void X11Helper::createPixmap(Pixmap* handleId, const EGLint id, bool hasAlpha, c
     memset(&visualInfoTemplate, 0, sizeof(XVisualInfo));
     visualInfoTemplate.visualid = visualId;
     int matchingCount = 0;
-    std::unique_ptr<XVisualInfo, X11Deleter> matchingVisuals(XGetVisualInfo(nativeDisplay(), VisualIDMask, &visualInfoTemplate, &matchingCount));
+    XUniquePtr<XVisualInfo> matchingVisuals(XGetVisualInfo(nativeDisplay(), VisualIDMask, &visualInfoTemplate, &matchingCount));
     XVisualInfo* foundVisual = 0;
     int requiredDepth = hasAlpha ? 32 : 24;
 
index 103f001..7f5e4c0 100644 (file)
 
 namespace WebCore {
 
-class X11Deleter {
-public:
-    template<typename T> void operator()(T* resource)
-    {
-        if (resource)
-            XFree(resource);
-    }
-};
-
 class X11Helper {
 public:
     static void createPixmap(Pixmap*, const XVisualInfo&, const IntSize& = IntSize(1, 1));
diff --git a/Source/WebCore/platform/graphics/x11/XUniquePtr.h b/Source/WebCore/platform/graphics/x11/XUniquePtr.h
new file mode 100644 (file)
index 0000000..245096f
--- /dev/null
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2015 Igalia S.L
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef XUniquePtr_h
+#define XUniquePtr_h
+
+#if PLATFORM(X11)
+#include "PlatformDisplayX11.h"
+#include <X11/Xutil.h>
+
+#if USE(GLX)
+typedef struct __GLXcontextRec* GLXContext;
+extern "C" void glXDestroyContext(Display*, GLXContext);
+#endif
+
+namespace WebCore {
+
+template<typename T>
+struct XPtrDeleter {
+    void operator()(T* ptr) const
+    {
+        if (ptr)
+            XFree(ptr);
+    }
+};
+
+template<typename T>
+using XUniquePtr = std::unique_ptr<T, XPtrDeleter<T>>;
+
+template<> struct XPtrDeleter<XImage> {
+    void operator() (XImage* ptr) const
+    {
+        if (ptr)
+            XDestroyImage(ptr);
+    }
+};
+
+template<> struct XPtrDeleter<_XGC> {
+    void operator() (_XGC* ptr) const
+    {
+        if (ptr)
+            XFreeGC(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native(), ptr);
+    }
+};
+// Give a name to this to avoid having to use the internal struct name.
+using XUniqueGC = XUniquePtr<_XGC>;
+
+#if USE(GLX)
+template<> struct XPtrDeleter<__GLXcontextRec> {
+    void operator() (__GLXcontextRec* ptr)
+    {
+        if (ptr)
+            glXDestroyContext(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native(), ptr);
+    }
+};
+// Give a name to this to avoid having to use the internal struct name.
+using XUniqueGLXContext = XUniquePtr<__GLXcontextRec>;
+#endif
+
+} // namespace WebCore
+
+#endif // PLATFORM(X11)
+
+#endif // XUniquePtr_h
diff --git a/Source/WebCore/platform/graphics/x11/XUniqueResource.cpp b/Source/WebCore/platform/graphics/x11/XUniqueResource.cpp
new file mode 100644 (file)
index 0000000..f300ee4
--- /dev/null
@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2015 Igalia S.L
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "XUniqueResource.h"
+
+#if PLATFORM(X11)
+#include "PlatformDisplayX11.h"
+#include <X11/Xlib.h>
+#include <X11/Xutil.h>
+
+#if PLATFORM(GTK)
+#include <X11/extensions/Xdamage.h>
+#endif
+
+#if USE(GLX)
+#include <GL/glx.h>
+#endif
+
+namespace WebCore {
+
+static inline Display* sharedDisplay()
+{
+    return downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
+}
+
+template<> void XUniqueResource<XResource::Colormap>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        XFreeColormap(sharedDisplay(), resource);
+}
+
+#if PLATFORM(GTK)
+template<> void XUniqueResource<XResource::Damage>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        XDamageDestroy(sharedDisplay(), resource);
+}
+#endif
+
+template<> void XUniqueResource<XResource::Pixmap>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        XFreePixmap(sharedDisplay(), resource);
+}
+
+template<> void XUniqueResource<XResource::Window>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        XDestroyWindow(sharedDisplay(), resource);
+}
+
+#if USE(GLX)
+template<> void XUniqueResource<XResource::GLXPbuffer>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        glXDestroyPbuffer(sharedDisplay(), resource);
+}
+
+template<> void XUniqueResource<XResource::GLXPixmap>::deleteXResource(unsigned long resource)
+{
+    if (resource)
+        glXDestroyGLXPixmap(sharedDisplay(), resource);
+}
+#endif // USE(GLX)
+
+} // namespace WebCore
+
+#endif // PLATFORM(X11)
diff --git a/Source/WebCore/platform/graphics/x11/XUniqueResource.h b/Source/WebCore/platform/graphics/x11/XUniqueResource.h
new file mode 100644 (file)
index 0000000..0da8b0c
--- /dev/null
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2015 Igalia S.L
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef XUniqueResource_h
+#define XUniqueResource_h
+
+#if PLATFORM(X11)
+
+#if USE(GLX)
+typedef unsigned long GLXPbuffer;
+typedef unsigned long GLXPixmap;
+#endif
+
+namespace WebCore {
+
+enum class XResource {
+    Colormap,
+#if PLATFORM(GTK)
+    Damage,
+#endif
+    Pixmap,
+    Window,
+#if USE(GLX)
+    GLXPbuffer,
+    GLXPixmap,
+#endif
+};
+
+template <XResource T> class XUniqueResource {
+public:
+    XUniqueResource()
+    {
+    }
+
+    XUniqueResource(unsigned long resource)
+        : m_resource(resource)
+    {
+    }
+
+    XUniqueResource(XUniqueResource&& uniqueResource)
+        : m_resource(uniqueResource.release())
+    {
+    }
+
+    XUniqueResource& operator=(XUniqueResource&& uniqueResource)
+    {
+        reset(uniqueResource.release());
+        return *this;
+    }
+
+    ~XUniqueResource()
+    {
+        reset();
+    }
+
+    unsigned long get() const { return m_resource; }
+    unsigned long release() { return std::exchange(m_resource, 0); }
+
+    void reset(unsigned long resource = 0)
+    {
+        std::swap(m_resource, resource);
+        deleteXResource(resource);
+    }
+
+    explicit operator bool() const { return m_resource; }
+
+private:
+    static void deleteXResource(unsigned long resource);
+
+    unsigned long m_resource { 0 };
+};
+
+using XUniqueColormap = XUniqueResource<XResource::Colormap>;
+#if PLATFORM(GTK)
+using XUniqueDamage = XUniqueResource<XResource::Damage>;
+#endif
+using XUniquePixmap = XUniqueResource<XResource::Pixmap>;
+using XUniqueWindow = XUniqueResource<XResource::Window>;
+#if USE(GLX)
+using XUniqueGLXPbuffer = XUniqueResource<XResource::GLXPbuffer>;
+using XUniqueGLXPixmap = XUniqueResource<XResource::GLXPixmap>;
+#endif
+
+} // namespace WebCore
+
+#endif // PLATFORM(X11)
+
+#endif // XUniqueResource_h
index 1920beb..83d64c7 100644 (file)
@@ -1,3 +1,33 @@
+2015-05-08  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [X11] Add XUniquePtr and XUniqueResource to automatically free X resources
+        https://bugs.webkit.org/show_bug.cgi?id=144521
+
+        Reviewed by Darin Adler.
+
+        Use XUniquePtr and XUniqueResource to free X resources.
+
+        * PlatformEfl.cmake: Add Source/WebCore/platform/graphics/x11 dir
+        to the include dir list.
+        * PlatformGTK.cmake: Ditto.
+        * UIProcess/cairo/BackingStoreCairo.cpp:
+        (WebKit::BackingStore::createBackend): Do not pass the display to
+        the BackingStoreBackendCairoX11 constructor.
+        * UIProcess/gtk/RedirectedXCompositeWindow.cpp:
+        (WebKit::RedirectedXCompositeWindow::RedirectedXCompositeWindow):
+        (WebKit::RedirectedXCompositeWindow::~RedirectedXCompositeWindow):
+        (WebKit::RedirectedXCompositeWindow::resize):
+        (WebKit::RedirectedXCompositeWindow::cleanupPixmapAndPixmapSurface):
+        (WebKit::RedirectedXCompositeWindow::surface):
+        * UIProcess/gtk/RedirectedXCompositeWindow.h:
+        (WebKit::RedirectedXCompositeWindow::windowID):
+        * WebProcess/Plugins/Netscape/NetscapePlugin.h:
+        * WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:
+        (WebKit::NetscapePlugin::platformPostInitializeWindowless):
+        (WebKit::NetscapePlugin::platformDestroy):
+        (WebKit::NetscapePlugin::platformGeometryDidChange):
+        (WebKit::NetscapePlugin::platformPaint):
+
 2015-05-11  Dan Bernstein  <mitz@apple.com>
 
         WebKit2 part of <rdar://problem/20878075> Trying to navigate to an invalid URL loads about:blank, but -[WKWebView URL] returns the invalid URL
index 5a02ea2..efefebe 100644 (file)
@@ -226,6 +226,7 @@ list(APPEND WebKit2_INCLUDE_DIRECTORIES
     "${WEBCORE_DIR}/platform/graphics/cairo"
     "${WEBCORE_DIR}/platform/graphics/efl"
     "${WEBCORE_DIR}/platform/graphics/opentype"
+    "${WEBCORE_DIR}/platform/graphics/x11"
     "${WEBCORE_DIR}/platform/network/soup"
     "${WEBCORE_DIR}/platform/text/enchant"
     "${WEBKIT2_DIR}/NetworkProcess/efl"
index 8bbaa4e..79b42b2 100644 (file)
@@ -455,6 +455,7 @@ list(APPEND WebKit2_INCLUDE_DIRECTORIES
     "${WEBCORE_DIR}/platform/gtk"
     "${WEBCORE_DIR}/platform/graphics/cairo"
     "${WEBCORE_DIR}/platform/graphics/opentype"
+    "${WEBCORE_DIR}/platform/graphics/x11"
     "${WEBCORE_DIR}/platform/network/soup"
     "${WEBCORE_DIR}/platform/text/enchant"
     "${WEBKIT2_DIR}/DatabaseProcess/unix"
index bd80b37..2fb30b5 100644 (file)
@@ -54,7 +54,7 @@ std::unique_ptr<BackingStoreBackendCairo> BackingStore::createBackend()
         GdkVisual* visual = gtk_widget_get_visual(m_webPageProxy.viewWidget());
         GdkScreen* screen = gdk_visual_get_screen(visual);
         ASSERT(downcast<PlatformDisplayX11>(sharedDisplay).native() == GDK_SCREEN_XDISPLAY(screen));
-        return std::make_unique<BackingStoreBackendCairoX11>(downcast<PlatformDisplayX11>(sharedDisplay).native(), GDK_WINDOW_XID(gdk_screen_get_root_window(screen)),
+        return std::make_unique<BackingStoreBackendCairoX11>(GDK_WINDOW_XID(gdk_screen_get_root_window(screen)),
             GDK_VISUAL_XVISUAL(visual), gdk_visual_get_depth(visual), m_size, m_deviceScaleFactor);
     }
 #endif
index 444f864..63092a6 100644 (file)
@@ -29,6 +29,7 @@
 
 #if USE(REDIRECTED_XCOMPOSITE_WINDOW)
 
+#include <X11/Xlib.h>
 #include <X11/extensions/Xcomposite.h>
 #include <X11/extensions/Xdamage.h>
 #include <cairo-xlib.h>
@@ -135,21 +136,18 @@ std::unique_ptr<RedirectedXCompositeWindow> RedirectedXCompositeWindow::create(G
 
 RedirectedXCompositeWindow::RedirectedXCompositeWindow(GdkWindow* parentWindow, std::function<void()> damageNotify)
     : m_display(GDK_DISPLAY_XDISPLAY(gdk_window_get_display(parentWindow)))
-    , m_window(0)
-    , m_parentWindow(0)
-    , m_pixmap(0)
-    , m_damage(0)
     , m_needsNewPixmapAfterResize(false)
 {
+    ASSERT(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native() == m_display);
     Screen* screen = DefaultScreenOfDisplay(m_display);
 
     GdkVisual* visual = gdk_window_get_visual(parentWindow);
-    Colormap colormap = XCreateColormap(m_display, RootWindowOfScreen(screen), GDK_VISUAL_XVISUAL(visual), AllocNone);
+    XUniqueColormap colormap(XCreateColormap(m_display, RootWindowOfScreen(screen), GDK_VISUAL_XVISUAL(visual), AllocNone));
 
     // This is based on code from Chromium: src/content/common/gpu/image_transport_surface_linux.cc
     XSetWindowAttributes windowAttributes;
     windowAttributes.override_redirect = True;
-    windowAttributes.colormap = colormap;
+    windowAttributes.colormap = colormap.get();
 
     // CWBorderPixel must be present when the depth doesn't match the parent's one.
     // See http://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c?id=xorg-server-1.16.0#n703.
@@ -164,13 +162,13 @@ RedirectedXCompositeWindow::RedirectedXCompositeWindow(GdkWindow* parentWindow,
         GDK_VISUAL_XVISUAL(visual),
         CWOverrideRedirect | CWColormap | CWBorderPixel,
         &windowAttributes);
-    XMapWindow(m_display, m_parentWindow);
+    XMapWindow(m_display, m_parentWindow.get());
 
     windowAttributes.event_mask = StructureNotifyMask;
     windowAttributes.override_redirect = False;
     // Create the window of at last 1x1 since X doesn't allow to create empty windows.
     m_window = XCreateWindow(m_display,
-        m_parentWindow,
+        m_parentWindow.get(),
         0, 0,
         std::max(1, m_size.width()),
         std::max(1, m_size.height()),
@@ -180,21 +178,19 @@ RedirectedXCompositeWindow::RedirectedXCompositeWindow(GdkWindow* parentWindow,
         CopyFromParent,
         CWEventMask,
         &windowAttributes);
-    XMapWindow(m_display, m_window);
+    XMapWindow(m_display, m_window.get());
 
-    XFreeColormap(m_display, colormap);
-
-    xDamageNotifier().add(m_window, WTF::move(damageNotify));
+    xDamageNotifier().add(m_window.get(), WTF::move(damageNotify));
 
     while (1) {
         XEvent event;
-        XWindowEvent(m_display, m_window, StructureNotifyMask, &event);
-        if (event.type == MapNotify && event.xmap.window == m_window)
+        XWindowEvent(m_display, m_window.get(), StructureNotifyMask, &event);
+        if (event.type == MapNotify && event.xmap.window == m_window.get())
             break;
     }
-    XSelectInput(m_display, m_window, NoEventMask);
-    XCompositeRedirectWindow(m_display, m_window, CompositeRedirectManual);
-    m_damage = XDamageCreate(m_display, m_window, XDamageReportNonEmpty);
+    XSelectInput(m_display, m_window.get(), NoEventMask);
+    XCompositeRedirectWindow(m_display, m_window.get(), CompositeRedirectManual);
+    m_damage = XDamageCreate(m_display, m_window.get(), XDamageReportNonEmpty);
 }
 
 RedirectedXCompositeWindow::~RedirectedXCompositeWindow()
@@ -204,12 +200,12 @@ RedirectedXCompositeWindow::~RedirectedXCompositeWindow()
     ASSERT(m_window);
     ASSERT(m_parentWindow);
 
-    xDamageNotifier().remove(m_window);
+    xDamageNotifier().remove(m_window.get());
 
-    XDamageDestroy(m_display, m_damage);
-    XDestroyWindow(m_display, m_window);
-    XDestroyWindow(m_display, m_parentWindow);
-    cleanupPixmapAndPixmapSurface();
+    // Explicitly reset these because we need to ensure it happens in this order.
+    m_damage.reset();
+    m_window.reset();
+    m_parentWindow.reset();
 }
 
 void RedirectedXCompositeWindow::resize(const IntSize& size)
@@ -218,7 +214,7 @@ void RedirectedXCompositeWindow::resize(const IntSize& size)
         return;
 
     // Resize the window to at last 1x1 since X doesn't allow to create empty windows.
-    XResizeWindow(m_display, m_window, std::max(1, size.width()), std::max(1, size.height()));
+    XResizeWindow(m_display, m_window.get(), std::max(1, size.width()), std::max(1, size.height()));
     XFlush(m_display);
 
     m_size = size;
@@ -232,9 +228,8 @@ void RedirectedXCompositeWindow::cleanupPixmapAndPixmapSurface()
     if (!m_pixmap)
         return;
 
-    XFreePixmap(m_display, m_pixmap);
-    m_pixmap = 0;
     m_surface = nullptr;
+    m_pixmap.reset();
 }
 
 cairo_surface_t* RedirectedXCompositeWindow::surface()
@@ -247,20 +242,19 @@ cairo_surface_t* RedirectedXCompositeWindow::surface()
 
     m_needsNewPixmapAfterResize = false;
 
-    Pixmap newPixmap = XCompositeNameWindowPixmap(m_display, m_window);
+    XUniquePixmap newPixmap(XCompositeNameWindowPixmap(m_display, m_window.get()));
     if (!newPixmap) {
         cleanupPixmapAndPixmapSurface();
         return nullptr;
     }
 
     XWindowAttributes windowAttributes;
-    if (!XGetWindowAttributes(m_display, m_window, &windowAttributes)) {
+    if (!XGetWindowAttributes(m_display, m_window.get(), &windowAttributes)) {
         cleanupPixmapAndPixmapSurface();
-        XFreePixmap(m_display, newPixmap);
         return nullptr;
     }
 
-    RefPtr<cairo_surface_t> newSurface = adoptRef(cairo_xlib_surface_create(m_display, newPixmap, windowAttributes.visual, m_size.width(), m_size.height()));
+    RefPtr<cairo_surface_t> newSurface = adoptRef(cairo_xlib_surface_create(m_display, newPixmap.get(), windowAttributes.visual, m_size.width(), m_size.height()));
 
     // Nvidia drivers seem to prepare their redirected window pixmap asynchronously, so for a few fractions
     // of a second after each resize, while doing continuous resizing (which constantly destroys and creates
@@ -275,7 +269,7 @@ cairo_surface_t* RedirectedXCompositeWindow::surface()
     }
 
     cleanupPixmapAndPixmapSurface();
-    m_pixmap = newPixmap;
+    m_pixmap = WTF::move(newPixmap);
     m_surface = newSurface;
     return m_surface.get();
 }
index 3aa4ff5..88a7e08 100644 (file)
 
 #include <WebCore/IntSize.h>
 #include <WebCore/RefPtrCairo.h>
-#include <X11/Xlib.h>
+#include <WebCore/XUniqueResource.h>
 #include <functional>
 
 typedef struct _GdkWindow GdkWindow;
-typedef unsigned long Damage;
+typedef struct _XDisplay Display;
+typedef unsigned long Window;
 
 namespace WebKit {
 
@@ -44,7 +45,7 @@ public:
     static std::unique_ptr<RedirectedXCompositeWindow> create(GdkWindow*, std::function<void()> damageNotify);
     ~RedirectedXCompositeWindow();
 
-    Window windowID() const { return m_window; }
+    Window windowID() const { return m_window.get(); }
     void resize(const WebCore::IntSize&);
     cairo_surface_t* surface();
 
@@ -54,10 +55,10 @@ private:
 
     Display* m_display;
     WebCore::IntSize m_size;
-    Window m_window;
-    Window m_parentWindow;
-    Pixmap m_pixmap;
-    Damage m_damage;
+    WebCore::XUniqueWindow m_window;
+    WebCore::XUniqueWindow m_parentWindow;
+    WebCore::XUniquePixmap m_pixmap;
+    WebCore::XUniqueDamage m_damage;
     RefPtr<cairo_surface_t> m_surface;
     bool m_needsNewPixmapAfterResize;
 };
index be11add..2c63721 100644 (file)
 #include "WebHitTestResult.h"
 #endif
 
+#if PLUGIN_ARCHITECTURE(X11)
+#include <WebCore/XUniqueResource.h>
+#endif
+
 namespace WebCore {
 class MachSendRight;
 class HTTPHeaderMap;
@@ -388,7 +392,7 @@ private:
     NP_CGContext m_npCGContext;
 #endif
 #elif PLUGIN_ARCHITECTURE(X11)
-    Pixmap m_drawable;
+    WebCore::XUniquePixmap m_drawable;
     Display* m_pluginDisplay;
 #if PLATFORM(GTK)
     GtkWidget* m_platformPluginWidget;
index 37f4793..8adca8a 100644 (file)
@@ -34,6 +34,7 @@
 #include <WebCore/GraphicsContext.h>
 #include <WebCore/NotImplemented.h>
 #include <WebCore/PlatformDisplayX11.h>
+#include <WebCore/XUniquePtr.h>
 
 #if PLATFORM(GTK)
 #include <gtk/gtk.h>
@@ -175,12 +176,10 @@ bool NetscapePlugin::platformPostInitializeWindowless()
     visualTemplate.depth = depth;
     visualTemplate.c_class = TrueColor;
     int numMatching;
-    XVisualInfo* visualInfo = XGetVisualInfo(display, VisualScreenMask | VisualDepthMask | VisualClassMask,
-                                             &visualTemplate, &numMatching);
+    XUniquePtr<XVisualInfo> visualInfo(XGetVisualInfo(display, VisualScreenMask | VisualDepthMask | VisualClassMask, &visualTemplate, &numMatching));
     ASSERT(visualInfo);
-    Visual* visual = visualInfo[0].visual;
+    Visual* visual = visualInfo.get()[0].visual;
     ASSERT(visual);
-    XFree(visualInfo);
 
     callbackStruct->visual = visual;
     callbackStruct->colormap = XCreateColormap(display, rootWindowID(), visual, AllocNone);
@@ -232,10 +231,7 @@ void NetscapePlugin::platformDestroy()
     XFreeColormap(hostDisplay, callbackStruct->colormap);
     delete callbackStruct;
 
-    if (m_drawable) {
-        XFreePixmap(hostDisplay, m_drawable);
-        m_drawable = 0;
-    }
+    m_drawable.reset();
 
 #if PLATFORM(GTK)
     if (m_platformPluginWidget) {
@@ -262,18 +258,12 @@ void NetscapePlugin::platformGeometryDidChange()
         return;
     }
 
-    Display* display = x11HostDisplay();
-    if (m_drawable)
-        XFreePixmap(display, m_drawable);
-
-    if (m_pluginSize.isEmpty()) {
-        m_drawable = 0;
+    m_drawable.reset();
+    if (m_pluginSize.isEmpty())
         return;
-    }
-
-    m_drawable = XCreatePixmap(display, rootWindowID(), m_pluginSize.width(), m_pluginSize.height(), displayDepth());
 
-    XSync(display, false); // Make sure that the server knows about the Drawable.
+    m_drawable = XCreatePixmap(x11HostDisplay(), rootWindowID(), m_pluginSize.width(), m_pluginSize.height(), displayDepth());
+    XSync(x11HostDisplay(), false); // Make sure that the server knows about the Drawable.
 }
 
 void NetscapePlugin::platformVisibilityDidChange()
@@ -307,7 +297,7 @@ void NetscapePlugin::platformPaint(GraphicsContext* context, const IntRect& dirt
     XGraphicsExposeEvent& exposeEvent = xevent.xgraphicsexpose;
     exposeEvent.type = GraphicsExpose;
     exposeEvent.display = x11HostDisplay();
-    exposeEvent.drawable = m_drawable;
+    exposeEvent.drawable = m_drawable.get();
 
     IntRect exposedRect(dirtyRect);
     exposeEvent.x = exposedRect.x();
@@ -324,11 +314,8 @@ void NetscapePlugin::platformPaint(GraphicsContext* context, const IntRect& dirt
         XSync(m_pluginDisplay, false);
 
 #if PLATFORM(GTK) || (PLATFORM(EFL) && USE(CAIRO))
-    RefPtr<cairo_surface_t> drawableSurface = adoptRef(cairo_xlib_surface_create(m_pluginDisplay,
-                                                                                 m_drawable,
-                                                                                 static_cast<NPSetWindowCallbackStruct*>(m_npWindow.ws_info)->visual,
-                                                                                 m_pluginSize.width(),
-                                                                                 m_pluginSize.height()));
+    RefPtr<cairo_surface_t> drawableSurface = adoptRef(cairo_xlib_surface_create(m_pluginDisplay, m_drawable.get(),
+        static_cast<NPSetWindowCallbackStruct*>(m_npWindow.ws_info)->visual, m_pluginSize.width(), m_pluginSize.height()));
     cairo_t* cr = context->platformContext()->cr();
     cairo_save(cr);