Changeset 283299 in webkit

Timestamp:

Sep 29, 2021, 10:17:38 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Cocoa GraphicsContextGLOpenGL should be more robust in destruction
​https://bugs.webkit.org/show_bug.cgi?id=230940

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2021-09-29
Reviewed by Antti Koivisto.

Delete resources based on checking if they exist, not based on
a flag that should cause them to exist. Currently the constructor can return early,
so various resources might not exist even if their flag condition would
indicate they should.

No new tests, refactor.

• platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:

(WebCore::GraphicsContextGLOpenGL::~GraphicsContextGLOpenGL):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-29  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        Cocoa GraphicsContextGLOpenGL should be more robust in destruction
+        https://bugs.webkit.org/show_bug.cgi?id=230940
+
+        Reviewed by Antti Koivisto.
+
+        Delete resources based on checking if they exist, not based on
+        a flag that should cause them to exist. Currently the constructor can return early,
+        so various resources might not exist even if their flag condition would
+        indicate they should.
+
+        No new tests, refactor.
+
+        * platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:
+        (WebCore::GraphicsContextGLOpenGL::~GraphicsContextGLOpenGL):
+
 2021-09-29  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm

@@ -430 +430 @@
     GraphicsContextGLOpenGLManager::sharedManager().removeContext(this);
     if (makeContextCurrent()) {
-        GraphicsContextGLAttributes attrs = contextAttributes();
-        gl::DeleteTextures(1, &m_texture);
-
-        if (attrs.antialias) {
+        if (m_texture)
+            gl::DeleteTextures(1, &m_texture);
+        if (m_multisampleColorBuffer)
             gl::DeleteRenderbuffers(1, &m_multisampleColorBuffer);
-            if (attrs.stencil || attrs.depth)
-                gl::DeleteRenderbuffers(1, &m_multisampleDepthStencilBuffer);
+        if (m_multisampleDepthStencilBuffer)
+            gl::DeleteRenderbuffers(1, &m_multisampleDepthStencilBuffer);
+        if (m_multisampleFBO)
             gl::DeleteFramebuffers(1, &m_multisampleFBO);
-        } else {
-            if (attrs.stencil || attrs.depth)
-                gl::DeleteRenderbuffers(1, &m_depthStencilBuffer);
-        }
-        gl::DeleteFramebuffers(1, &m_fbo);
+        if (m_depthStencilBuffer)
+            gl::DeleteRenderbuffers(1, &m_depthStencilBuffer);
+        if (m_fbo)
+            gl::DeleteFramebuffers(1, &m_fbo);
         if (m_preserveDrawingBufferTexture)
             gl::DeleteTextures(1, &m_preserveDrawingBufferTexture);
@@ -454 +453 @@
             fence.abandon();
     }
-    if (m_displayBufferPbuffer) {
+    if (m_displayBufferPbuffer)
         EGL_DestroySurface(m_displayObj, m_displayBufferPbuffer);
+    if (m_swapChain) {
         auto recycledBuffer = m_swapChain->recycleBuffer();
         if (recycledBuffer.handle)
@@ -466 +466 @@
         clearCurrentContext();
         EGL_DestroyContext(m_displayObj, m_contextObj);
-    } else
-        ASSERT(currentContext != this);
+    }
+    ASSERT(currentContext != this);
     LOG(WebGL, "Destroyed a GraphicsContextGLOpenGL (%p).", this);
 }