[Apple] WebGL layer may use GC3D after free with remote layer hosting
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Jun 2014 00:00:03 +0000 (00:00 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Jun 2014 00:00:03 +0000 (00:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=134179
<rdar://problem/17412931>

Reviewed by Tim Horton.

With remote layer hosting, a WebGLLayer may attempt to draw after
the GraphicsContext3D has been destroyed. We need to make sure
the GC3D tells the WebGLLayer that it is no longer valid.

While here, I changed some return 0 to return nullptr, the
name of the ObjC property member from m_context to _context,
and removed some unnecessary .get() calls.

This is tested by run-webkit-tests with the --remote-layer-tree
option.

* platform/graphics/mac/GraphicsContext3DMac.mm:
(WebCore::GraphicsContext3D::GraphicsContext3D): No need for .get().
(WebCore::GraphicsContext3D::~GraphicsContext3D): Set the context reference on WebGLLayer
to be null.
(WebCore::GraphicsContext3D::setRenderbufferStorageFromDrawable): No need for .get().
* platform/graphics/mac/WebGLLayer.h: Set the context reference on WebGLLayer
to be null.
* platform/graphics/mac/WebGLLayer.mm:
(-[WebGLLayer initWithGraphicsContext3D:]): Rename m_context to _context.
(-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Check for null.
(-[WebGLLayer copyCGLContextForPixelFormat:]): Rename to _context.
(-[WebGLLayer drawInCGLContext:pixelFormat:forLayerTime:displayTime:]): Check for null.
(-[WebGLLayer copyImageSnapshotWithColorSpace:]): Ditto.
(-[WebGLLayer display]): Ditto.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm
Source/WebCore/platform/graphics/mac/WebGLLayer.h
Source/WebCore/platform/graphics/mac/WebGLLayer.mm

index ef9c4cf..a1587e4 100644 (file)
@@ -1,3 +1,37 @@
+2014-06-22  Dean Jackson  <dino@apple.com>
+
+        [Apple] WebGL layer may use GC3D after free with remote layer hosting
+        https://bugs.webkit.org/show_bug.cgi?id=134179
+        <rdar://problem/17412931>
+
+        Reviewed by Tim Horton.
+
+        With remote layer hosting, a WebGLLayer may attempt to draw after
+        the GraphicsContext3D has been destroyed. We need to make sure
+        the GC3D tells the WebGLLayer that it is no longer valid.
+
+        While here, I changed some return 0 to return nullptr, the
+        name of the ObjC property member from m_context to _context,
+        and removed some unnecessary .get() calls.
+
+        This is tested by run-webkit-tests with the --remote-layer-tree
+        option.
+
+        * platform/graphics/mac/GraphicsContext3DMac.mm:
+        (WebCore::GraphicsContext3D::GraphicsContext3D): No need for .get().
+        (WebCore::GraphicsContext3D::~GraphicsContext3D): Set the context reference on WebGLLayer
+        to be null.
+        (WebCore::GraphicsContext3D::setRenderbufferStorageFromDrawable): No need for .get().
+        * platform/graphics/mac/WebGLLayer.h: Set the context reference on WebGLLayer
+        to be null.
+        * platform/graphics/mac/WebGLLayer.mm:
+        (-[WebGLLayer initWithGraphicsContext3D:]): Rename m_context to _context.
+        (-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Check for null.
+        (-[WebGLLayer copyCGLContextForPixelFormat:]): Rename to _context.
+        (-[WebGLLayer drawInCGLContext:pixelFormat:forLayerTime:displayTime:]): Check for null.
+        (-[WebGLLayer copyImageSnapshotWithColorSpace:]): Ditto.
+        (-[WebGLLayer display]): Ditto.
+
 2014-06-20  Simon Fraser  <simon.fraser@apple.com>
 
         [WK2] Frameset frames are not scrollable after loading (and should be)
index 1e9f953..d25b67d 100644 (file)
@@ -223,11 +223,11 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3D::Attributes attrs, HostWi
     BEGIN_BLOCK_OBJC_EXCEPTIONS
         m_webGLLayer = adoptNS([[WebGLLayer alloc] initWithGraphicsContext3D:this]);
 #if PLATFORM(IOS)
-        [m_webGLLayer.get() setOpaque:0];
+        [m_webGLLayer setOpaque:0];
 #endif
 #ifndef NDEBUG
-        [m_webGLLayer.get() setName:@"WebGL Layer"];
-#endif    
+        [m_webGLLayer setName:@"WebGL Layer"];
+#endif
     END_BLOCK_OBJC_EXCEPTIONS
 
 #if !PLATFORM(IOS)
@@ -332,6 +332,7 @@ GraphicsContext3D::~GraphicsContext3D()
         CGLSetCurrentContext(0);
         CGLDestroyContext(m_contextObj);
 #endif
+        [m_webGLLayer setContext:nullptr];
         numActiveContexts--;
     }
 }
@@ -339,7 +340,7 @@ GraphicsContext3D::~GraphicsContext3D()
 #if PLATFORM(IOS)
 bool GraphicsContext3D::setRenderbufferStorageFromDrawable(GC3Dsizei width, GC3Dsizei height)
 {
-    [m_webGLLayer.get() setBounds:CGRectMake(0, 0, width, height)];
+    [m_webGLLayer setBounds:CGRectMake(0, 0, width, height)];
     return [m_contextObj renderbufferStorage:GL_RENDERBUFFER fromDrawable:static_cast<NSObject<EAGLDrawable>*>(m_webGLLayer.get())];
 }
 #endif
index 011aaf7..96871a8 100644 (file)
@@ -39,9 +39,11 @@ namespace WebCore {
 @interface WebGLLayer : CAOpenGLLayer
 #endif
 {
-    WebCore::GraphicsContext3D* m_context;
+    WebCore::GraphicsContext3D* _context;
 }
 
+@property (nonatomic) WebCore::GraphicsContext3D* context;
+
 - (id)initWithGraphicsContext3D:(WebCore::GraphicsContext3D*)context;
 
 - (CGImageRef)copyImageSnapshotWithColorSpace:(CGColorSpaceRef)colorSpace;
index 2fd7c46..55de04e 100644 (file)
@@ -42,9 +42,11 @@ using namespace WebCore;
 
 @implementation WebGLLayer
 
+@synthesize context=_context;
+
 -(id)initWithGraphicsContext3D:(GraphicsContext3D*)context
 {
-    m_context = context;
+    _context = context;
     self = [super init];
     return self;
 }
@@ -59,19 +61,22 @@ using namespace WebCore;
     // If needed we will have to set the display mask in the Canvas CGLContext and
     // make sure it matches.
     UNUSED_PARAM(mask);
-    return CGLRetainPixelFormat(CGLGetPixelFormat(m_context->platformGraphicsContext3D()));
+    return CGLRetainPixelFormat(CGLGetPixelFormat(_context->platformGraphicsContext3D()));
 }
 
 -(CGLContextObj)copyCGLContextForPixelFormat:(CGLPixelFormatObj)pixelFormat
 {
     CGLContextObj contextObj;
-    CGLCreateContext(pixelFormat, m_context->platformGraphicsContext3D(), &contextObj);
+    CGLCreateContext(pixelFormat, _context->platformGraphicsContext3D(), &contextObj);
     return contextObj;
 }
 
 -(void)drawInCGLContext:(CGLContextObj)glContext pixelFormat:(CGLPixelFormatObj)pixelFormat forLayerTime:(CFTimeInterval)timeInterval displayTime:(const CVTimeStamp *)timeStamp
 {
-    m_context->prepareTexture();
+    if (!_context)
+        return;
+
+    _context->prepareTexture();
 
     CGLSetCurrentContext(glContext);
 
@@ -86,7 +91,7 @@ using namespace WebCore;
     glLoadIdentity();
 
     glEnable(GL_TEXTURE_2D);
-    glBindTexture(GL_TEXTURE_2D, m_context->platformTexture());
+    glBindTexture(GL_TEXTURE_2D, _context->platformTexture());
     
     glBegin(GL_TRIANGLE_FAN);
         glTexCoord2f(0, 0);
@@ -114,11 +119,14 @@ static void freeData(void *, const void *data, size_t /* size */)
 
 -(CGImageRef)copyImageSnapshotWithColorSpace:(CGColorSpaceRef)colorSpace
 {
+    if (!_context)
+        return nullptr;
+
 #if PLATFORM(IOS)
     UNUSED_PARAM(colorSpace);
-    return 0;
+    return nullptr;
 #else
-    CGLSetCurrentContext(m_context->platformGraphicsContext3D());
+    CGLSetCurrentContext(_context->platformGraphicsContext3D());
 
     RetainPtr<CGColorSpaceRef> imageColorSpace = colorSpace;
     if (!imageColorSpace)
@@ -133,7 +141,7 @@ static void freeData(void *, const void *data, size_t /* size */)
     size_t dataSize = rowBytes * height;
     void* data = fastMalloc(dataSize);
     if (!data)
-        return 0;
+        return nullptr;
 
     glPixelStorei(GL_PACK_ROW_LENGTH, rowBytes / 4);
     glReadPixels(0, 0, width, height, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, data);
@@ -150,12 +158,15 @@ static void freeData(void *, const void *data, size_t /* size */)
 
 - (void)display
 {
+    if (!_context)
+        return;
+
 #if PLATFORM(IOS)
-    m_context->endPaint();
+    _context->endPaint();
 #else
     [super display];
 #endif
-    m_context->markLayerComposited();
+    _context->markLayerComposited();
 }
 
 @end