[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





--- Comment #9 from Mikhail Naganov <mnaganov at chromium.org>  2010-11-24 09:24:27 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.

Details provided.

>> WebCore/bindings/js/ScriptHeapSnapshot.h:44
>> +        virtual void WriteChunk(const String& chunk) = 0;
> 
> OutputStream's API is fairly conservative: Write, Flush and Close.

I don't need Flush. Changed to Write and Close.

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

Both "Adaptor" and "Adapter" are valid. But as I see, "Adapter" is used more frequently in WK sources. Changing to "Adapter".

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

Fixed

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

Fixed

>> 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'];

This array has < 10 elements, and this code runs only once, so using linear search is fine.

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

Done.

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

Done.

>> WebCore/inspector/front-end/HeapSnapshotView.js:443
>> +        this._loadedCallbacks[uid].callback(profile);
> 
> this._loadedCallbacks[uid](profile); (bind other arguments should you need them).

Fixed.

>> WebCore/inspector/front-end/HeapSnapshotView.js:538
>> +            if (node.isHidden) {
> 
> No { } for single lines.

Fixed.

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

Done.

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