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 treegraph
 
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

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>