[Webkit-unassigned] [Bug 66710] Assertion fires if canvas is resized while save() active

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=66710





--- Comment #2 from Stephen White <senorblanco at chromium.org>  2011-08-23 07:05:10 PST ---
(In reply to comment #1)
> (In reply to comment #0)
> > Created an attachment (id=104734)
 --> (https://bugs.webkit.org/attachment.cgi?id=104734&action=review) [details] [details]
> > Test case
> > 
> > If a 2D canvas is resized (via canvas.width = foo) while a save() is active, GraphicsContext's destructor asserts:
> > 
> > ASSERT(m_stack.isEmpty());
> 
> IIRC, this ASSERT was added primarily for helping with an issue Simon discovered in the layout tests. Certain canvas tests would use save() without a corresponding restore() - which in general, is totally fine for a page's script to do - but it had some side effect in the layout test environment that caused other subsequent tests in the chain of layout tests to act up or fail altogether. So, adding the assert helped to find those canvas tests that didn't restore, so that we could add restores to them, and avoid the issue.
> 
> Surprisingly, I haven't run into many (if any at all - can't think of any at the moment) real world examples of saves not matched with restores.

This assertion fires in Chrome if I visit http://ie.microsoft.com/testdrive/Performance/Fireflies/Default.html.

 I assume this is because almost all use cases for save() and restore() are animations, and it's crucial for them to match saves and restores in order to work properly. Though, I always thought that I'd find more pages that would have behavior that might bail out in the middle of some animation, recreate the canvas, and hit this assert. I suppose this bug is pretty much that case. OR even more simply: have an extra save or two in the beginning that wouldn't affect them but would hit the assert when we tear down the page.
> 
> Do we have anything instead of ASSERT like TESTING that would only enable certain "testing" asserts when we're running tests? I always considered ASSERT to be a way to declare an invariant in the code, such that if it fails, it signifies a bug in our code or our understanding of our code. This ASSERT is not an invariant since it's perfectly acceptable for a context to die with a non-zero size state stack.

If we did so, it would be impossible to add the attached test case as a layout test, for example.

It should be fairly straightforward to fix this bug by copying the #if !ASSERT_DISABLED code that's currently in the CanvasRenderingContext2D destructor to HTMLCanvasElement::setSurfaceSize(), just before m_imageBuffer is cleared.   Or alternatively, to the ImageBuffer destructor, although that would have to be done in every port.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.