Bug 107819 - Web Inspector: each node in a detached DOM tree is shown in its own "detached DOM tree" entry in heap profiler
Summary: Web Inspector: each node in a detached DOM tree is shown in its own "detached...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on: 100707
Blocks: 108202 108322
  Show dependency tree / graph
 
Reported: 2013-01-24 06:16 PST by Yury Semikhatsky
Modified: 2013-01-30 03:36 PST (History)
12 users (show)

See Also:

Attachments
Test page (287 bytes, text/html)
2013-01-24 06:16 PST, Yury Semikhatsky 		no flags Details
Heap profiler screenshot (43.49 KB, image/png)
2013-01-24 06:17 PST, Yury Semikhatsky 		no flags Details
Patch (2.12 KB, patch)
2013-01-24 07:35 PST, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch without static_cast<Node*> (5.73 KB, patch)
2013-01-25 03:23 PST, Yury Semikhatsky 		abarth: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2013-01-24 06:16:32 PST 
Created attachment 184478 [details]
Test page

After http://trac.webkit.org/changeset/133044/trunk/Source/WebCore/bindings/v8/V8GCController.cpp each js node wrapper became a root of a separate detached DOM tree.

On the attached page we have a detached DOM tree with 3 divs. Each of them becomes a seperate DOM tree(see screenshot).
Comment 1 Yury Semikhatsky 2013-01-24 06:17:03 PST 
Created attachment 184480 [details]
Heap profiler screenshot
Comment 2 Yury Semikhatsky 2013-01-24 07:35:10 PST 
Created attachment 184491 [details]
Patch
Comment 3 Adam Barth 2013-01-24 19:15:07 PST 
Comment on attachment 184491 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184491&action=review

> Source/WebCore/bindings/v8/V8GCController.cpp:106
> +                OwnPtr<RetainedObjectInfo> retainedObjectInfo;
> +                if (group[0].WrapperClassId() == v8DOMNodeClassId)
> +                    retainedObjectInfo = adoptPtr(new RetainedDOMInfo(static_cast<Node*>(root)));

Why is this static cast valid?
Comment 4 Yury Semikhatsky 2013-01-25 02:57:25 PST 
Comment on attachment 184491 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184491&action=review

>> Source/WebCore/bindings/v8/V8GCController.cpp:106
>> +                    retainedObjectInfo = adoptPtr(new RetainedDOMInfo(static_cast<Node*>(root)));
> 
> Why is this static cast valid?

Because V8DOMWrapper::associateObjectWithWrapper will set wrapper class id to v8DOMNodeClassId only if the wrapped object is a DOM node and the opaque root for DOM nodes is always a node. I started with a patch that introduced NodeImplicitConnection and ObjectImplicitConnection derived from ImplicitConnection and made the compiler control that the root is a node. But it looked unnecessarily complicated.
Comment 5 Yury Semikhatsky 2013-01-25 03:23:10 PST 
Created attachment 184715 [details]
Patch without static_cast<Node*>
Comment 6 Adam Barth 2013-01-25 09:28:43 PST 
Comment on attachment 184715 [details]
Patch without static_cast<Node*>

Yay for no static_cast.  Thanks!
Comment 7 Adam Barth 2013-01-25 09:29:09 PST 
Comment on attachment 184491 [details]
Patch

IMHO the later patch is better.
Comment 8 Yury Semikhatsky 2013-01-28 00:07:14 PST 
Committed r140949: <http://trac.webkit.org/changeset/140949>