Bug 108202 - Web Inspector: test that nodes from the same detached DOM tree will get into one group in heap snapshot
Summary: Web Inspector: test that nodes from the same detached DOM tree will get into ...
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: 107819
Blocks:
  Show dependency tree / graph
 
Reported: 2013-01-29 09:17 PST by Yury Semikhatsky
Modified: 2013-01-31 02:54 PST (History)
14 users (show)

See Also:

Attachments
Patch (12.16 KB, patch)
2013-01-29 09:22 PST, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (13.49 KB, patch)
2013-01-29 10:15 PST, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (9.88 KB, patch)
2013-01-30 05:32 PST, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (9.89 KB, patch)
2013-01-30 06:18 PST, Yury Semikhatsky 		vsevik: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (3) 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-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>