Bug 59212

Summary: [chromium] Regression: r84631 causes crashes on Chromium GPU canvas tests
Product: WebKit Reporter: Adrienne Walker <enne>
Component: CanvasAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, enne, eric, jamesr, mdelaney7, rolandsteiner, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch simon.fraser: review+

Description Adrienne Walker 2011-04-22 10:45:42 PDT
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=canvas%2Fphilip%2Ftests%2F2d.clearRect.negative.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.canvas.destination-over.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.clip.destination-out.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.globalAlpha.default.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.image.destination-over.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.operation.default.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.transparent.lighter.html%2Ccanvas%2Fphilip%2Ftests%2F2d.composite.uncovered.fill.source-out.html%2Ccanvas%2Fphilip%2Ftests%2F2d.drawImage.9arg.sourcepos.html%2Ccanvas%2Fphilip%2Ftests%2F2d.drawImage.floatsource.html%2Ccanvas%2Fphilip%2Ftests%2F2d.drawImage.nonfinite.html%2Ccanvas%2Fphilip%2Ftests%2F2d.drawImage.zerosource.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.get.semitransparent.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.hsl-1.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.hsla-1.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.invalid.hex2.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.invalid.hsl-4.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.invalid.rgb-2.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.invalid.rgba-5.html%2Ccanvas%2Fphilip%2Ftests%2F2d.fillStyle.parse.rgb-percent.html

This only seems to repro locally for me in DRT and only if I run more than single test.
Comment 1 Simon Fraser (smfr) 2011-04-22 10:55:44 PDT
Seems like your code has unmatched save/restore then?
Comment 2 Adrienne Walker 2011-04-22 10:58:35 PDT
It's segfaulting, not asserting.  Not to say we probably don't have mismatched save and restores somewhere too.
Comment 3 Adrienne Walker 2011-04-22 11:04:52 PDT
Simon, I suspect your change just unearthed some preexisting Chromium bug here, but I figured I'd put you on the CC list.  Didn't want you to feel left out.  ;)

I'm still investigating what's going on.  It looks like the m_context.clear() call in HTMLCanvasElement changed the destruction order and some other Chromium-only pointer also needs to get cleaned up.
Comment 4 Adrienne Walker 2011-04-22 13:08:59 PDT
James: this is crashing in PlatformContextSkia (the one owned by the ImageBufferData owned by the ImageBuffer owned by the HTMLCanvasElement).  In the destructor, either m_gpuCanvas or m_gpuCanvas->drawingBuffer() is a stale pointer (or both).

Maybe CanvasRenderingContext2D should clear its shared graphics context in the destructor?
Comment 5 Adrienne Walker 2011-04-22 13:10:45 PDT
Created attachment 90747 [details]
Patch
Comment 6 Simon Fraser (smfr) 2011-04-22 13:37:37 PDT
Comment on attachment 90747 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:162
> +    if (GraphicsContext* c = drawingContext())
> +        c->setSharedGraphicsContext3D(0, 0, IntSize());

Would be nice to use 'context' instead of 'c' as I do above.
Comment 7 Adrienne Walker 2011-04-22 13:40:32 PDT
(In reply to comment #6)
> (From update of attachment 90747 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90747&action=review
> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:162
> > +    if (GraphicsContext* c = drawingContext())
> > +        c->setSharedGraphicsContext3D(0, 0, IntSize());
> 
> Would be nice to use 'context' instead of 'c' as I do above.

Sure.  I'll fix that before landing.
Comment 8 Adrienne Walker 2011-04-22 14:17:59 PDT
Committed r84680: <http://trac.webkit.org/changeset/84680>
Comment 9 WebKit Review Bot 2011-04-22 16:18:49 PDT
http://trac.webkit.org/changeset/84680 might have broken GTK Linux 64-bit Debug