[webkit-reviews] review granted: [Bug 47197] Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to prevent early deallocation and crash : [Attachment 69814] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 47197: Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to
prevent early deallocation and crash
https://bugs.webkit.org/show_bug.cgi?id=47197

Attachment 69814: Patch
https://bugs.webkit.org/attachment.cgi?id=69814&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
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!