[webkit-reviews] review granted: [Bug 85595] Web Inspector: save HeapSnapshot draft implementation : [Attachment 140386] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 85595: Web Inspector: save HeapSnapshot draft implementation
https://bugs.webkit.org/show_bug.cgi?id=85595

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

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=140386&action=review


> Source/WebCore/inspector/front-end/HeapSnapshotView.js:794
> +	       return;

We should invoke the callback here.

> Source/WebCore/inspector/front-end/HeapSnapshotView.js:795
> +	   if (!this.proxy) {

Why is the proxy field public?

> Source/WebCore/inspector/front-end/HeapSnapshotView.js:830
> +	   else {

It should go on the previous line.

> Source/WebCore/inspector/front-end/ProfilesPanel.js:1108
> +	   if (!profile.canSave || !profile.canSave())

Double check to be sure?

> Source/WebCore/inspector/front-end/inspector.js:1044
> +	   WebInspector._saveCallbacks[url] = callback;

Please add and assert here if we cannot override existing callback.