| Summary: | Crash in RenderBox::paintMaskImages due to a mask without an associated image | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Berend-Jan Wever <skylined> |
| Component: | DOM | Assignee: | Julien Chaffraix <jchaffraix> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | eric, hyatt, inferno, jamesr, jchaffraix, simon.fraser, webkit.review.bot |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | http://code.google.com/p/chromium/issues/detail?id=64628 | ||
| Attachments: | |||
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. |
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.