Bug 108202

Summary: Web Inspector: test that nodes from the same detached DOM tree will get into one group in heap snapshot
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, dglazkov, gyuyoung.kim, haraken, keishi, loislo, pfeldman, pmuellr, rakuco, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 107819    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch vsevik: review+, webkit.review.bot: commit-queue-

Description Yury Semikhatsky 2013-01-29 09:17:19 PST 
JS wrappers for all DOM nodes from the same detached DOM tree should get into single "Detached DOM Tree" entry in the JS heap snapshot. We need a layout test for this to avoid problems like https://bugs.webkit.org/show_bug.cgi?id=107819
Comment 1 Yury Semikhatsky 2013-01-29 09:22:48 PST 
Created attachment 185254 [details]
Patch
Comment 2 Yury Semikhatsky 2013-01-29 09:24:42 PST 
(In reply to comment #1)
> Created an attachment (id=185254) [details]
> Patch

This patch depends on V8 change https://code.google.com/p/v8/source/detail?r=13486 which is not rolled yet.
Comment 3 WebKit Review Bot 2013-01-29 10:06:09 PST 
Comment on attachment 185254 [details]
Patch

Attachment 185254 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16198210

New failing tests:
inspector-protocol/heap-snapshot-with-detached-dom-tree.html
Comment 4 Yury Semikhatsky 2013-01-29 10:15:15 PST 
Created attachment 185258 [details]
Patch
Comment 5 Adam Barth 2013-01-29 12:54:32 PST 
Comment on attachment 185258 [details]
Patch

I'm excited to have this test, but I'm not familiar enough with the testing harness to review it.
Comment 6 Ilya Tikhonovsky 2013-01-30 01:48:02 PST 
Comment on attachment 185258 [details]
Patch

looks good. I'd also check that the detached dom tree has expected number of wrappers inside and there is no second detached dom tree.
Comment 7 Yury Semikhatsky 2013-01-30 05:32:35 PST 
Created attachment 185474 [details]
Patch
Comment 8 Yury Semikhatsky 2013-01-30 05:33:16 PST 
(In reply to comment #6)
> (From update of attachment 185258 [details])
> looks good. I'd also check that the detached dom tree has expected number of wrappers inside and there is no second detached dom tree.

Done.
Comment 9 Yury Semikhatsky 2013-01-30 06:18:37 PST 
Created attachment 185485 [details]
Patch
Comment 10 Ilya Tikhonovsky 2013-01-30 06:19:55 PST 
Comment on attachment 185485 [details]
Patch

lgtm
Comment 11 WebKit Review Bot 2013-01-30 08:19:56 PST 
Comment on attachment 185485 [details]
Patch

Attachment 185485 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16218563

New failing tests:
inspector-protocol/heap-profiler/heap-snapshot-with-detached-dom-tree.html
Comment 12 Yury Semikhatsky 2013-01-31 02:54:20 PST 
Committed r141393: <http://trac.webkit.org/changeset/141393>