[webkit-reviews] review denied: [Bug 28429] Adding Heap profiler page to Web Inspector : [Attachment 38566] Proposed patch to start adding Heap profiler UI behind a flag

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 28429: Adding Heap profiler page to Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=28429

Attachment 38566: Proposed patch to start adding Heap profiler UI behind a flag
https://bugs.webkit.org/attachment.cgi?id=38566&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> +    if (Preferences.heapProfilerPresents) {

This pref should be named heapProfilerPresent.

> +	   this.snapshotsListTreeElement = new
WebInspector.SidebarSectionTreeElement(WebInspector.UIString("HEAP SNAPSHOTS"),
{}, true);

I think you can just use null in place of the {} if you don't have a
represented object.

> +	   if (this.snapshotsListTreeElement)
this.snapshotsListTreeElement.removeChildren();

This should be on two lines to match our style guidelines.

> +	       if (this.snapshotButton) this.snapshotButton.visible = true;

Ditto.

> +	       if (this.snapshotButton) this.snapshotButton.visible = false;

Ditto.

> +.heap-snapshot-status-bar-item .glyph {
> +    -webkit-mask-image: url(Images/focusButtonGlyph.png);
> +}
> +

Can you add a FIXME comment about need a new image here.