Bug 105768 - Web Inspector: extract JS heap profiler specifics into JSHeapSnapshot.js
Summary: Web Inspector: extract JS heap profiler specifics into JSHeapSnapshot.js
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:
Blocks:
 
Reported: 2012-12-26 07:26 PST by Yury Semikhatsky
Modified: 2012-12-26 07:56 PST (History)
8 users (show)

See Also:

Attachments
Patch (48.67 KB, patch)
2012-12-26 07:30 PST, Yury Semikhatsky 		pfeldman: review+
Details | Formatted Diff | Diff
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 2012-12-26 07:26:58 PST 
HeapSnapshot.js can be split into a generic heap graph representation which can be reused for native heap and a JavaScript-specific part.
Comment 1 Yury Semikhatsky 2012-12-26 07:30:15 PST 
Created attachment 180747 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-12-26 07:38:38 PST 
Comment on attachment 180747 [details]
Patch

lgtm with nits
Comment 3 Ilya Tikhonovsky 2012-12-26 07:41:06 PST 
Comment on attachment 180747 [details]
Patch

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

> Source/WebCore/inspector/front-end/HeapSnapshot.js:746
> +        var node = this.createNode(0);

please remove 0

> Source/WebCore/inspector/front-end/HeapSnapshot.js:892
> +        var node = this.createNode(rootNodeIndex);

rootNode()

> LayoutTests/inspector/profiler/heap-snapshot.html:14
> +            var nodeRoot = snapshot.createNode(snapshot._rootNodeIndex);

rootNode

> LayoutTests/inspector/profiler/heap-snapshot.html:28
> +            var nodeRoot = snapshot.createNode(snapshot._rootNodeIndex);

ditto
Comment 4 Yury Semikhatsky 2012-12-26 07:54:38 PST 
(In reply to comment #3)
> (From update of attachment 180747 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180747&action=review
> 
> > Source/WebCore/inspector/front-end/HeapSnapshot.js:746
> > +        var node = this.createNode(0);
> 
> please remove 0
> 
Done.

> > Source/WebCore/inspector/front-end/HeapSnapshot.js:892
> > +        var node = this.createNode(rootNodeIndex);
> 
> rootNode()
> 
I'd leave in the current state as we put rootNodeIndex into the array in the next line.

> > LayoutTests/inspector/profiler/heap-snapshot.html:14
> > +            var nodeRoot = snapshot.createNode(snapshot._rootNodeIndex);
> 
> rootNode
> 
There is no such method on the mock.

> > LayoutTests/inspector/profiler/heap-snapshot.html:28
> > +            var nodeRoot = snapshot.createNode(snapshot._rootNodeIndex);
> 
> ditto
There is no such method on the mock.
Comment 5 Yury Semikhatsky 2012-12-26 07:56:00 PST 
Committed r138477: <http://trac.webkit.org/changeset/138477>