[Webkit-unassigned] [Bug 49974] Web Inspector: Request JSON-serialized heap profile.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49974


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #74670|review?                     |review-
               Flag|                            |




--- Comment #8 from Pavel Feldman <pfeldman at chromium.org>  2010-11-24 06:06:12 PST ---
(From update of attachment 74670)
View in context: https://bugs.webkit.org/attachment.cgi?id=74670&action=review

> WebCore/ChangeLog:5
> +        [Chromium] Request JSON-serialized heap profile.

Please use Web Inspector prefix. It would be nice to see more details on the change in the ChangeLog.

> WebCore/bindings/js/ScriptHeapSnapshot.h:44
> +        virtual void WriteChunk(const String& chunk) = 0;

OutputStream's API is fairly conservative: Write, Flush and Close.

> WebCore/bindings/v8/ScriptHeapSnapshot.cpp:56
> +class OutputStreamAdaptor : public v8::OutputStream {

Adapter?

> WebCore/inspector/front-end/HeapSnapshotView.js:38
> +    get done() {

{ on the next line.

> WebCore/inspector/front-end/HeapSnapshotView.js:42
> +    get isElement() {

ditto x 7

> WebCore/inspector/front-end/HeapSnapshotView.js:145
> +        this._nodeTypeOffset = meta.fields.indexOf('type');

This does not sound optimal:

for (var i = 0; i < fields.length; ++i)
  map [fields[i]] = i;

this._nodeNameOffset = map['name'];
this._nodeIdOffset = map['id'];

> WebCore/inspector/front-end/HeapSnapshotView.js:147
> +        this._nodeIdOffset = meta.fields.indexOf('id');

Please use double quotes.

> WebCore/inspector/front-end/HeapSnapshotView.js:435
> +        if (!this._loadedCallbacks[uid]) return;

return on the next line please.

> WebCore/inspector/front-end/HeapSnapshotView.js:443
> +        this._loadedCallbacks[uid].callback(profile);

this._loadedCallbacks[uid](profile); (bind other arguments should you need them).

> WebCore/inspector/front-end/HeapSnapshotView.js:538
> +            if (node.isHidden) {

No { } for single lines.

> WebCore/inspector/front-end/HeapSnapshotView.js:539
> +                result.lowlevels[node.name] = { count: node.instancesCount, size: node.selfSize, type: node.name };

{count

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.