Bug 12052

Summary:

Assertion failure in WebCore::RenderHTMLCanvas::layout

Product:

WebKit

Reporter:

Mark Rowe (bdash) <mrowe>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Major

CC:

darin, eric, hamaji

Priority:

Keywords:

HasReduction

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Patch v1	none
Patch v2	none
Patch v3	none
Patch v4	none
Qt test fix	none

Mark Rowe (bdash)

Reported 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)

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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Mark Rowe (bdash)

Comment 1 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.

Shinichiro Hamaji

Comment 2 2009-08-13 04:42:42 PDT

Created attachment 34730 [details] Patch v1 --- 5 files changed, 64 insertions(+), 2 deletions(-)

Shinichiro Hamaji

Comment 3 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.

Darin Adler

Comment 4 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.

Dave Hyatt

Comment 5 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.

Shinichiro Hamaji

Comment 6 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?

Shinichiro Hamaji

Comment 7 2009-08-13 14:03:49 PDT

Created attachment 34785 [details] Patch v2 --- 5 files changed, 53 insertions(+), 1 deletions(-)

Darin Adler

Comment 8 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?

Darin Adler

Comment 9 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?

Shinichiro Hamaji

Comment 10 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!

Shinichiro Hamaji

Comment 11 2009-08-13 14:32:49 PDT

Created attachment 34788 [details] Patch v3 --- 5 files changed, 53 insertions(+), 1 deletions(-)

Eric Seidel (no email)

Comment 12 2009-08-13 14:53:14 PDT

Comment on attachment 34788 [details] Patch v3 Style: if (window.layoutTestController) { 17 layoutTestController.dumpAsText(); 18 }

Shinichiro Hamaji

Comment 13 2009-08-13 18:08:51 PDT

Created attachment 34801 [details] Patch v4 --- 5 files changed, 52 insertions(+), 1 deletions(-)

Shinichiro Hamaji

Comment 14 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.

Eric Seidel (no email)

Comment 15 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+.

Eric Seidel (no email)

Comment 16 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>

Eric Seidel (no email)

Comment 17 2009-08-17 17:24:58 PDT

All reviewed patches have been landed. Closing bug.

Eric Seidel (no email)

Comment 18 2009-08-17 17:50:08 PDT

I think this caused the Tiger and Qt bots to show failures. Re-opening.

Shinichiro Hamaji

Comment 19 2009-08-17 23:11:14 PDT

Created attachment 35018 [details] Qt test fix --- 2 files changed, 13 insertions(+), 1 deletions(-)

Shinichiro Hamaji

Comment 20 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.

Eric Seidel (no email)

Comment 21 2009-08-17 23:19:54 PDT

Comment on attachment 35018 [details] Qt test fix LGTM.

Eric Seidel (no email)

Comment 22 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

Eric Seidel (no email)

Comment 23 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.

Eric Seidel (no email)

Comment 24 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>

Eric Seidel (no email)

Comment 25 2009-08-18 00:33:22 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.