Bug 12052 - Assertion failure in WebCore::RenderHTMLCanvas::layout
Summary: Assertion failure in WebCore::RenderHTMLCanvas::layout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Major
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2006-12-31 16:32 PST by Mark Rowe (bdash)
Modified: 2009-08-18 00:33 PDT (History)
3 users (show)

See Also:


Attachments
Patch v1 (3.64 KB, patch)
2009-08-13 04:42 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v2 (3.20 KB, patch)
2009-08-13 14:03 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v3 (3.23 KB, patch)
2009-08-13 14:32 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v4 (3.22 KB, patch)
2009-08-13 18:08 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Qt test fix (1.11 KB, patch)
2009-08-17 23:11 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2006-12-31 16:32:08 PST
<html>
<head>
    <title>Test HTML Page</title>
    <style type="text/css">
    canvas { display: run-in; }
    </style>
</head>
<body>
    <canvas>canvas</canvas>

    <p>Die when canvas isn't last tag in the document.</p>
</body>
</html>



Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0xbbadbeef
0x01169cd2 in WebCore::RenderHTMLCanvas::layout (this=0x1705e76c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderHTMLCanvas.cpp:81
81          ASSERT(minMaxKnown());
(gdb) bt
#0  0x01169cd2 in WebCore::RenderHTMLCanvas::layout (this=0x1705e76c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderHTMLCanvas.cpp:81
#1  0x015093dd in WebCore::RenderObject::layoutIfNeeded (this=0x1705e76c) at RenderObject.h:509
#2  0x011499b6 in WebCore::RenderBlock::layoutInlineChildren (this=0x1705de1c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/bidi.cpp:1529
#3  0x0115c454 in WebCore::RenderBlock::layoutBlock (this=0x1705de1c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:493
#4  0x01150ac2 in WebCore::RenderBlock::layout (this=0x1705de1c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:421
#5  0x015093dd in WebCore::RenderObject::layoutIfNeeded (this=0x1705de1c) at RenderObject.h:509
#6  0x0115bb94 in WebCore::RenderBlock::layoutBlockChildren (this=0x1706327c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:1102
#7  0x0115c493 in WebCore::RenderBlock::layoutBlock (this=0x1706327c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:495
#8  0x01150ac2 in WebCore::RenderBlock::layout (this=0x1706327c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:421
#9  0x015093dd in WebCore::RenderObject::layoutIfNeeded (this=0x1706327c) at RenderObject.h:509
#10 0x0115bb94 in WebCore::RenderBlock::layoutBlockChildren (this=0x17060f8c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:1102
#11 0x0115c493 in WebCore::RenderBlock::layoutBlock (this=0x17060f8c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:495
#12 0x01150ac2 in WebCore::RenderBlock::layout (this=0x17060f8c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:421
#13 0x015093dd in WebCore::RenderObject::layoutIfNeeded (this=0x17060f8c) at RenderObject.h:509
#14 0x0115bb94 in WebCore::RenderBlock::layoutBlockChildren (this=0x1704cb2c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:1102
#15 0x0115c493 in WebCore::RenderBlock::layoutBlock (this=0x1704cb2c, relayoutChildren=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:495
#16 0x01150ac2 in WebCore::RenderBlock::layout (this=0x1704cb2c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderBlock.cpp:421
#17 0x011674ab in WebCore::RenderView::layout (this=0x1704cb2c) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/rendering/RenderView.cpp:111
#18 0x010eb87b in WebCore::FrameView::layout (this=0x17035d00, allowSubtree=true) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/page/FrameView.cpp:424
#19 0x010f2d63 in WebCore::Document::implicitClose (this=0x20c8c00) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/dom/Document.cpp:1359
#20 0x01394669 in WebCore::FrameLoader::checkEmitLoadEvent (this=0x2043800) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/FrameLoader.cpp:1074
#21 0x01397fb9 in WebCore::FrameLoader::checkCompleted (this=0x2043800) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/FrameLoader.cpp:1042
#22 0x013980bb in WebCore::FrameLoader::loadDone (this=0x2043800) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/FrameLoader.cpp:1016
#23 0x0110b4ee in WebCore::DocLoader::setLoadInProgress (this=0x17044e90, load=false) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/DocLoader.cpp:176
#24 0x0110cd7d in WebCore::Loader::receivedAllData (this=0x1640bb8, loader=0x17058480, allData=0x17061760) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/loader.cpp:110
#25 0x0137c65c in WebCore::SubresourceLoader::didFinishLoading (this=0x17058480) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/mac/SubresourceLoaderMac.mm:195
#26 0x0137859c in WebCore::ResourceLoader::didFinishLoading (this=0x17058480) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/loader/mac/ResourceLoaderMac.mm:446
#27 0x013878e3 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] (self=0x17058df0, _cmd=0x90a9d160, con=0x17058e00) at /Users/mrowe/Documents/Source/SVN/WebKit-Nightlies/WebCore/platform/network/mac/ResourceHandleMac.mm:295
#28 0x9265be00 in -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback] ()
#29 0x92659ea5 in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] ()
#30 0x92659b41 in _sendCallbacks ()
#31 0x90829379 in CFRunLoopRunSpecific ()
#32 0x90828eb5 in CFRunLoopRunInMode ()
#33 0x92dcdb90 in RunCurrentEventLoopInMode ()
#34 0x92dcd297 in ReceiveNextEventCommon ()
#35 0x92dcd0ee in BlockUntilNextEventMatchingListInMode ()
#36 0x9326f465 in _DPSNextEvent ()
#37 0x9326f056 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#38 0x00006f96 in ?? ()
#39 0x93268ddb in -[NSApplication run] ()
#40 0x9325cd2f in NSApplicationMain ()
#41 0x0005f7de in ?? ()
#42 0x0005f6f9 in ?? ()
(gdb)
Comment 1 Mark Rowe (bdash) 2007-01-01 16:03:22 PST
This looks very similar to bug 12044 and bug 12041.  One difference is that I could not cause this to crash with a canvas element when it was the last element inside the body.
Comment 2 Shinichiro Hamaji 2009-08-13 04:42:42 PDT
Created attachment 34730 [details]
Patch v1


---
 5 files changed, 64 insertions(+), 2 deletions(-)
Comment 3 Shinichiro Hamaji 2009-08-13 04:49:30 PDT
> Created an attachment (id=34730) [details]

Now, I don't see the original ASSERTion Mark reported but this HTML seems to hit another ASSERTion. Also, if a run-in video tag has control, safari crashes even if it is release mode build.

Though I'm not sure if this is the best way (I don't know how non-block run-in should be handled), this patch should fix the crash.
Comment 4 Darin Adler 2009-08-13 09:58:42 PDT
Comment on attachment 34730 [details]
Patch v1

review- because of the double change log in the LayoutTests directory.

> +    // block.  We also don't handle the run-in if the element isn't a
> +    // block (e.g., <canvas>, <video>, and etc.).
> +    if (!child->isRunIn() || !child->childrenInline() && !child->isReplaced() || !child->isRenderBlock())
>          return false;

This definitely gets rid of the assertion/crash, but I am not 100% certain it's correct. I'd like to hear from Hyatt or Dan Bernstein.
Comment 5 Dave Hyatt 2009-08-13 10:17:33 PDT
Comment on attachment 34730 [details]
Patch v1

It seems like replaced elements could be display:run-in and actually be made to work... I am curious what Mozilla does.
Comment 6 Shinichiro Hamaji 2009-08-13 14:03:17 PDT
Thanks Darin and David for your comments.

It seems that Firefox (3.5.2) doesn't handle replaced run-in properly. If I understand correctly, browsers should layout

 <canvas style="display:run-in;"></canvas>
 <p>foo</p>

just like

 <p><canvas style="display:inline"></canvas>foo</p>

With the following test case, firefox is treating the run-in canvas just like a block element:

http://tinyurl.com/qbodd6

Maybe it's OK to commit a patch which fixes the crash bug with incorrect replaced run-in handling and a FIXME comment for now?
Comment 7 Shinichiro Hamaji 2009-08-13 14:03:49 PDT
Created attachment 34785 [details]
Patch v2


---
 5 files changed, 53 insertions(+), 1 deletions(-)
Comment 8 Darin Adler 2009-08-13 14:07:30 PDT
Comment on attachment 34785 [details]
Patch v2

Are replaced renderers that can show up here that are not a RenderBlock?
Comment 9 Darin Adler 2009-08-13 14:07:55 PDT
(In reply to comment #8)
> Are replaced renderers that can show up here that are not a RenderBlock?

I meant to say:

Are replaced renderers the only kind that can show up here that are not a RenderBlock?
Comment 10 Shinichiro Hamaji 2009-08-13 14:32:17 PDT
> Are replaced renderers the only kind that can show up here that are not a
> RenderBlock?

I think so... But yeah, I'm not 100% sure and people may add renderer which passes this check. I'll upload another patch. Thanks for this catch!
Comment 11 Shinichiro Hamaji 2009-08-13 14:32:49 PDT
Created attachment 34788 [details]
Patch v3


---
 5 files changed, 53 insertions(+), 1 deletions(-)
Comment 12 Eric Seidel (no email) 2009-08-13 14:53:14 PDT
Comment on attachment 34788 [details]
Patch v3

Style:
if (window.layoutTestController) {
 17     layoutTestController.dumpAsText();
 18 }
Comment 13 Shinichiro Hamaji 2009-08-13 18:08:51 PDT
Created attachment 34801 [details]
Patch v4


---
 5 files changed, 52 insertions(+), 1 deletions(-)
Comment 14 Shinichiro Hamaji 2009-08-13 18:09:45 PDT
> Style:
> if (window.layoutTestController) {
>  17     layoutTestController.dumpAsText();
>  18 }

Ah, I shouldn't have brackets for single-line if, right? Fixed.
Comment 15 Eric Seidel (no email) 2009-08-17 16:29:54 PDT
Comment on attachment 34801 [details]
Patch v4

Shinichiro doesn't have hit commit bit yet, so adding cq+.
Comment 16 Eric Seidel (no email) 2009-08-17 17:24:53 PDT
Comment on attachment 34801 [details]
Patch v4

Clearing flags on attachment: 34801

Committed r47400: <http://trac.webkit.org/changeset/47400>
Comment 17 Eric Seidel (no email) 2009-08-17 17:24:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Eric Seidel (no email) 2009-08-17 17:50:08 PDT
I think this caused the Tiger and Qt bots to show failures.  Re-opening.
Comment 19 Shinichiro Hamaji 2009-08-17 23:11:14 PDT
Created attachment 35018 [details]
Qt test fix


---
 2 files changed, 13 insertions(+), 1 deletions(-)
Comment 20 Shinichiro Hamaji 2009-08-17 23:14:01 PDT
(In reply to comment #18)
> I think this caused the Tiger and Qt bots to show failures.  Re-opening.

Sorry for this. I believe the patch fixes the test failure. As Qt doesn't support <video>, the string "foo" in <video> is rendered and it makes the test fail. I think we can just remove the string. The main point of this test is just checking if the webcore crashes or not and we don't need to test availability of video tag.
Comment 21 Eric Seidel (no email) 2009-08-17 23:19:54 PDT
Comment on attachment 35018 [details]
Qt test fix

LGTM.
Comment 22 Eric Seidel (no email) 2009-08-17 23:50:47 PDT
Comment on attachment 35018 [details]
Qt test fix

Rejecting patch 35018 from commit-queue.  This patch will require manual commit.

['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Comment 23 Eric Seidel (no email) 2009-08-18 00:19:42 PDT
Comment on attachment 35018 [details]
Qt test fix

Testing 11087 test cases.
fast/profiler/call.html -> crashed

Exiting early after 1 failures.  7375 tests run.


Was the failure.  Unlikely that was related.  Adding this back to the commit-queue.
Comment 24 Eric Seidel (no email) 2009-08-18 00:33:17 PDT
Comment on attachment 35018 [details]
Qt test fix

Clearing flags on attachment: 35018

Committed r47421: <http://trac.webkit.org/changeset/47421>
Comment 25 Eric Seidel (no email) 2009-08-18 00:33:22 PDT
All reviewed patches have been landed.  Closing bug.