[webkit-reviews] review denied: [Bug 52624] Web Inspector: [Chromium] Prepare for landing of detailed heap snapshots : [Attachment 80037] rebaselined

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 52624: Web Inspector: [Chromium] Prepare for landing of detailed heap
snapshots
https://bugs.webkit.org/show_bug.cgi?id=52624

Attachment 80037: rebaselined
https://bugs.webkit.org/attachment.cgi?id=80037&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80037&action=review

A bunch of nits, otherwise looks good.

> Source/WebCore/bindings/js/ScriptProfiler.h:41
> +    class HeapSnapshotControl {

HeapSnapshotProgress:
start(totalWork)
worked(workDone)
done()
bool isCanceled()

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:42
> +    get statusBarItems()

why do you need this?

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:59
> +	   WebInspector.View.prototype.show.call(this, parentElement);

why do you need this?

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:64
> +	   WebInspector.View.prototype.hide.call(this);

why do you need this?

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:65
> +    },

no trailing coma

> Source/WebCore/inspector/front-end/HeapSnapshot.js:46
> +	   return this._getType() === this._snapshot._edgeElementType;

No "get" prefixes in WebCore.