Created attachment 75020 [details] Repro Repro: <style> *{ -webkit-mask-image:none,none,url(x); } </style> id: chrome.dll!WebCore::RenderBox::paintMaskImages ReadAV@NULL (f12ffd1e9345f0da79fdc0164c287064) description: Attempt to read from unallocated NULL pointer in chrome.dll!WebCore::RenderBox::paintMaskImages application: Chromium 9.0.596.0 stack: chrome.dll!WebCore::RenderBox::paintMaskImages chrome.dll!WebCore::RenderBox::paintMask chrome.dll!WebCore::RenderBlock::paintObject chrome.dll!WebCore::RenderBlock::paint chrome.dll!WebCore::RenderLayer::paintLayer chrome.dll!WebCore::RenderLayer::paintList chrome.dll!WebCore::RenderLayer::paintLayer chrome.dll!WebCore::RenderLayer::paintList chrome.dll!WebCore::RenderLayer::paintLayer chrome.dll!WebCore::RenderLayer::paint chrome.dll!WebCore::FrameView::paintContents chrome.dll!WebCore::ScrollView::paint chrome.dll!WebKit::WebFrameImpl::paintWithContext chrome.dll!WebKit::WebFrameImpl::paint chrome.dll!RenderWidget::PaintRect chrome.dll!RenderWidget::DoDeferredUpdate chrome.dll!RenderWidget::CallDoDeferredUpdate chrome.dll!MessageLoop::RunTask chrome.dll!MessageLoop::DoWork chrome.dll!base::MessagePumpDefault::Run chrome.dll!MessageLoop::RunInternal ... Exact stack can differ: it appears that if you put element tags in the repro, they vary depending on what element gets rendered first.
FYI, the test case does not crash under DRT but definitely does under the browser (tested on Qt/Linux and Chromium/Linux)
Testcase1: <style> .f { -webkit-mask:-webkit-gradient(linear, left top, left bottom, from(#E7E7E7), to(#CFCFCF));</style> <table <tr class="f"> <td> Testcase2: <style> td { border-spacing : use-script; -webkit-mask-image : url(red_transparent.gif);</style> AA000A00AAA00<table onmousewheel="entrybar" <tr><td> Testcase3: <style> .box { display: table-footer-group; -webkit-mask-box-image: url("bogus.png");</style> <div class="box"><figure>
(In reply to comment #1) > FYI, the test case does not crash under DRT but definitely does under the browser (tested on Qt/Linux and Chromium/Linux) I was wrong here: you need to dump the pixels if you want the test to crash (which it does). Sorry for the noise. I have a WIP fix, taking the bug.
There is actually 2 bugs covered by this one (which explains the different stacktrace you were getting). One is related to FillLayer usage in paintMaskImage (and is covered by Berend-Jan's test case (at least on my machine)). The other one is that paintMaskImage could be called with a GraphicsContext with paintingDisabled() set (called from FrameView::updateControlTints). This bug is partly covered by Abhishek's test cases. I will fix the first one as part of this bug and file a follow-up bug for the second one as it will need to add a reliable way to test under DRT.
Created attachment 107362 [details] Proposed fix: patch paintMaskImages to properly check the image(), also avoid calling hasImage() in a loop as it is already looping.
Comment on attachment 107362 [details] Proposed fix: patch paintMaskImages to properly check the image(), also avoid calling hasImage() in a loop as it is already looping. View in context: https://bugs.webkit.org/attachment.cgi?id=107362&action=review > LayoutTests/fast/css/empty-webkit-mask-crash.html:4 > + // We need to dump the image, we don't care about the layout information. It would be nice if the text content said "This test should not crash". > Source/WebCore/ChangeLog:3 > + chrome.dll!WebCore::RenderBox::paintMaskImages ReadAV@NULL (various hashes) Retitle the bug to reflect the actual issue, please. > Source/WebCore/ChangeLog:10 > + The crash stems from the fact that FillLayer::hasImage would over the linked list missing verb (would __ over)
Comment on attachment 107362 [details] Proposed fix: patch paintMaskImages to properly check the image(), also avoid calling hasImage() in a loop as it is already looping. View in context: https://bugs.webkit.org/attachment.cgi?id=107362&action=review >> LayoutTests/fast/css/empty-webkit-mask-crash.html:4 >> + // We need to dump the image, we don't care about the layout information. > > It would be nice if the text content said "This test should not crash". There is a comment above that mention the passing condition. I did not want to have a platform-specific output as we really don't care about it and I don't see how we can do what you want in a cross-platform fashion. If you see one, I am all for it too! >> Source/WebCore/ChangeLog:10 >> + The crash stems from the fact that FillLayer::hasImage would over the linked list > > missing verb (would __ over) Oh! __ = walk :-)
(In reply to comment #7) > There is a comment above that mention the passing condition. I did not want to have a platform-specific output as we really don't care about it There isn't a problem with platform-specific output, since this is a dumpAsText test.
Created attachment 107531 [details] Patch for landing
Created attachment 107547 [details] Patch for landing
(In reply to comment #8) > (In reply to comment #7) > > There is a comment above that mention the passing condition. I did not want to have a platform-specific output as we really don't care about it > > There isn't a problem with platform-specific output, since this is a dumpAsText test. FYI, I was confused with what dumpAsText(true) would dump. After I was corrected, I added a mention of the bug and condition to pass in text output for clarity.
Comment on attachment 107547 [details] Patch for landing Clearing flags on attachment: 107547 Committed r95235: <http://trac.webkit.org/changeset/95235>
All reviewed patches have been landed. Closing bug.