| Summary: | Fix ownership of GraphicsContext3D in SharedGraphicsContext3D to prevent early deallocation and crash | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Marrin <cmarrin> | ||||||
| Component: | New Bugs | Assignee: | Chris Marrin <cmarrin> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric, jamesr, kbr, senorblanco | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Marrin
2010-10-05 11:05:35 PDT
Created attachment 69812 [details]
Patch
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? Created attachment 69814 [details]
Patch
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! Was this landed and just not closed? |