[webkit-reviews] review denied: [Bug 49974] Web Inspector: Request JSON-serialized heap profile. : [Attachment 74670] fix WK compilation
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 24 06:06:12 PST 2010
Pavel Feldman <pfeldman at chromium.org> has denied Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 49974: Web Inspector: Request JSON-serialized heap profile.
https://bugs.webkit.org/show_bug.cgi?id=49974
Attachment 74670: fix WK compilation
https://bugs.webkit.org/attachment.cgi?id=74670&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
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
More information about the webkit-reviews
mailing list