Bug 47197 – Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to prevent early deallocation and crash

• Component: New Bugs
• Status: RESOLVED FIXED

[Chris Marrin]

Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to prevent early deallocation and crash

[Chris Marrin]

Created attachment 69812 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 69812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69812&action=review

> WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:59
> -    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?

[Chris Marrin]

Created attachment 69814 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 69814 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69814&action=review

r=me if you remove the FeatureDefines change.

> WebCore/ChangeLog:10
> +        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!)

> WebCore/Configurations/FeatureDefines.xcconfig:39
> -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;

Whoopsie!

[Eric Seidel (no email)]

Was this landed and just not closed?

[Eric Seidel (no email)]

r69127.  Please close bugs after landing or use a tool which does it for you (webkit-patch land for example).