[chromium] Regression: r84631 causes crashes on Chromium GPU canvas tests

[Adrienne Walker]

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.

[Simon Fraser (smfr)]

Seems like your code has unmatched save/restore then?

[Adrienne Walker]

It's segfaulting, not asserting.  Not to say we probably don't have mismatched save and restores somewhere too.

[Adrienne Walker]

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.

[Adrienne Walker]

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?

[Adrienne Walker]

Created attachment 90747 [details]
Patch

[Simon Fraser (smfr)]

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.

[Adrienne Walker]

(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.

[Adrienne Walker]

Committed r84680: <http://trac.webkit.org/changeset/84680>

[WebKit Review Bot]

http://trac.webkit.org/changeset/84680 might have broken GTK Linux 64-bit Debug