[webkit-reviews] review granted: [Bug 44531] Web Inspector: Store heap snapshots in InspectorProfilerAgent : [Attachment 65440] Style fixes

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has granted Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 44531: Web Inspector: Store heap snapshots in InspectorProfilerAgent
https://bugs.webkit.org/show_bug.cgi?id=44531

Attachment 65440: Style fixes
https://bugs.webkit.org/attachment.cgi?id=65440&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
More nits before you land. New file is too big to review the logic, but given
that it is a WebKit -> WebCore move, I think it is fine.

I think you'll need to merge with incoming change from Ilya (#44617).
WebCore/inspector/front-end/HeapSnapshotView.js:31
 +  /**
nit: no JSDoc in front-end yet.

WebCore/inspector/front-end/HeapSnapshotView.js:124
 +  };
nit: no ;

WebCore/inspector/front-end/HeapSnapshotView.js:278
 +	jumpToFirstSearchResult:
WebInspector.CPUProfileView.prototype.jumpToFirstSearchResult,
I personally hate this. Please inherit both from single abstract view instead.
Can be done in subsequent patch. (Add FIXME here)

WebCore/inspector/front-end/inspector.css:3915
 +  .heap-snapshot-sidebar-tree-item .icon {
Please introduce a separate file "heap-profiler.css" instead. Can be done in
immediate follow-up.

WebKit/chromium/src/js/DevTools.js:41
 +  // Here and below are overrides to existing WebInspector methods only.
This comment no longer applies. Nuke it!

WebKit/chromium/src/js/DevTools.js:65
 +	var oldShow = WebInspector.ProfilesPanel.prototype.show;
Put a FIXME telling it should be upstreamed.