Bug 31288 - Add a function to show render tree for debugging
Summary: Add a function to show render tree for debugging
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-10 00:57 PST by Shinichiro Hamaji
Modified: 2009-12-07 21:30 PST (History)
2 users (show)

See Also:


Attachments
Patch v1 (4.01 KB, patch)
2009-11-10 00:59 PST, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v2 (4.31 KB, patch)
2009-11-30 00:35 PST, Shinichiro Hamaji
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 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.
Comment 1 Shinichiro Hamaji 2009-11-10 00:59:49 PST
Created attachment 42851 [details]
Patch v1
Comment 2 Shinichiro Hamaji 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
Comment 3 Eric Seidel (no email) 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?
Comment 4 Shinichiro Hamaji 2009-11-30 00:35:01 PST
Created attachment 44009 [details]
Patch v2
Comment 5 Shinichiro Hamaji 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.
Comment 6 Adam Barth 2009-11-30 12:49:56 PST
style-queue ran check-webkit-style on attachment 44009 [details] without any errors.
Comment 7 Darin Adler 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
Comment 8 Shinichiro Hamaji 2009-12-07 21:30:23 PST
> I don't think the WebCore prefix is needed here.

Fixed and landed, thanks!