Bug 31288

Summary: Add a function to show render tree for debugging
Product: WebKit Reporter: Shinichiro Hamaji <hamaji>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1
none
Patch v2 darin: review+

Shinichiro Hamaji
Reported 2009-11-10 00:57:40 PST
RenderObject::showTree only dumps DOM tree and we cannot see anonymous render nodes. WebCore::externalRepresentation dumps a render tree. But it has two issues: 1. it calls RenderObject::layout() as a side effect. This may cause crash in some contexts and 2. it returns the tree as a String instead of outputting to stderr, which isn't so convenient. So, it would be OK to add another function to dump the render tree.
Attachments
Patch v1 (4.01 KB, patch)
2009-11-10 00:59 PST, Shinichiro Hamaji
no flags
Patch v2 (4.31 KB, patch)
2009-11-30 00:35 PST, Shinichiro Hamaji
darin: review+
Shinichiro Hamaji
Comment 1 2009-11-10 00:59:49 PST
Created attachment 42851 [details] Patch v1
Shinichiro Hamaji
Comment 2 2009-11-10 01:01:53 PST
Here is the sample output of this function. Any comments will be appreciated. (gdb) p showRenderTree(this, this->parent()) RenderView 0xe4cf1c #document 0x8872600 RenderBlock 0xe4d46c HTML 0xe4b660 RenderBody 0xeb797c BODY 0xeb7460 RenderBlock 0xeb9e6c P 0xeb7a00 RenderText 0xeb9efc #text 0xebc410 "This is the WebKit version of " RenderInline 0xebc69c A 0xeba060 RenderText 0xebc77c #text 0xeba120 "CSS 2.1 Test Suite: dynamic changes to 'counter-increment'" RenderText 0xebc7fc #text 0xebc7c0 "." RenderBlock 0xebc9ac DIV 0xebc900 RenderBlock 0xebcbdc P 0xebca60 RenderText 0xebccec #text 0xebcb00 "The following two lines should be the same:" RenderBlock 0xebb7bc DIV 0xebcde0 RenderInline 0xebbabc SPAN 0xebb8e0 CLASS=increment - RenderInline (generated) 0xebbcec * RenderCounter 0xebbc8c RenderText 0xebbd3c RenderInline 0xebbffc SPAN 0xebbe20 CLASS=increment RenderInline (generated) 0xebc0bc RenderCounter 0xebcf7c RenderText 0xebcfdc RenderInline 0xebd29c SPAN 0xebd0c0 CLASS=increment RenderInline (generated) 0xebd45c RenderCounter 0xebd3fc RenderText 0xebd4ac RenderBlock 0xebd64c DIV 0xebd5a0 RenderText 0xebd72c #text 0xebd570 "1-2-new-3-4-" RenderBlock 0xebd93c DIV 0xebd890 RenderInline 0xe1e1fc SPAN 0xec1850 RenderText 0xe672dc #text 0xec4230 "Before the dynamic change:" RenderBR 0xe98d7c BR 0xec4260
Eric Seidel (no email)
Comment 3 2009-11-29 13:20:17 PST
Comment on attachment 42851 [details] Patch v1 I'm not sure what "outputted" is supposed to mean. I guess it's printedCharacters? Can it just default to 0, with = 0?
Shinichiro Hamaji
Comment 4 2009-11-30 00:35:01 PST
Created attachment 44009 [details] Patch v2
Shinichiro Hamaji
Comment 5 2009-11-30 00:42:02 PST
Thanks for the review! I really want to use this function for my debugging. > I'm not sure what "outputted" is supposed to mean. I guess it's > printedCharacters? Yes, and thanks for the comment, printedCharacters is definitely better naming. I changed the name. > Can it just default to 0, with = 0? Unfortunately, gdb cannot handle default parameters. If we make it a default parameter, we always need to pass 0 for RenderObject::showRenderObject when debugging with GDB. That's why I created two overloaded functions. Also, I've noticed we may call showRenderObject() for a NULL RenderObject while we are debugging. I added NULL checks for this pointers. This looks a bit weird to check if this pointer is NULL. If you don't like this, I may make showRenderObject a global function.
Adam Barth
Comment 6 2009-11-30 12:49:56 PST
style-queue ran check-webkit-style on attachment 44009 [details] without any errors.
Darin Adler
Comment 7 2009-12-07 10:30:16 PST
Comment on attachment 44009 [details] Patch v2 > + for (const WebCore::RenderObject* child = firstChild(); child; child = child->nextSibling()) I don't think the WebCore prefix is needed here. r=me
Shinichiro Hamaji
Comment 8 2009-12-07 21:30:23 PST
> I don't think the WebCore prefix is needed here. Fixed and landed, thanks!
Note You need to log in before you can comment on or make changes to this bug.