Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to prevent early deallocation and crash
Created attachment 69812 [details]
Comment on attachment 69812 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=69812&action=review
> - OwnPtr<GraphicsContext3D> context = GraphicsContext3D::create(attr, hostWindow);
> + PassOwnPtr<GraphicsContext3D> context = GraphicsContext3D::create(attr, hostWindow);
> if (!context)
> return 0;
> - return adoptRef(new SharedGraphicsContext3D(context.get()));
> + return adoptRef(new SharedGraphicsContext3D(context));
It would be better to keep the context variable an OwnPtr, and pass context.release() to SharedGraphicsContext3D.
Is it possible to make a test?
Created attachment 69814 [details]
Comment on attachment 69814 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=69814&action=review
r=me if you remove the FeatureDefines change.
> + This is work in progress and the crash only happens with ACCELERATED_2D_CANVAS turned on (which is off
> + by default). So no test cases yet.
It would be better to say that many existing test cases will crash if ACCELERATED_2D_CANVAS is turned on. (If that were not the case, you could conceivably write a test case that would crash once it was turned on. But since we already have such test cases, you're off the hook!)
> -ENABLE_ACCELERATED_2D_CANVAS_macosx_1060 = ;
> -ENABLE_ACCELERATED_2D_CANVAS_macosx_1070 = ;
> +ENABLE_ACCELERATED_2D_CANVAS_macosx_1060 = ENABLE_ACCELERATED_2D_CANVAS;
> +ENABLE_ACCELERATED_2D_CANVAS_macosx_1070 = ENABLE_ACCELERATED_2D_CANVAS;
Was this landed and just not closed?
r69127. Please close bugs after landing or use a tool which does it for you (webkit-patch land for example).