[webkit-reviews] review denied: [Bug 92348] Web Inspector: Profiles: extract save to file / load from code : [Attachment 154619] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied eustas.bug at gmail.com's request
for review:
Bug 92348: Web Inspector: Profiles: extract save to file / load from code
https://bugs.webkit.org/show_bug.cgi?id=92348

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

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


> Source/WebCore/ChangeLog:19
> +	   transfer process: onTransferStarted, onChunktransfered,
onTransferFinished.

typo: onChunktransfered -> onChunkTransferred

> Source/WebCore/inspector/front-end/FileUtils.js:34
> +WebInspector.ChunkedTransferrerDelegate = function()

Maybe ChunkedTransferDelegate ?

> Source/WebCore/inspector/front-end/FileUtils.js:67
> +    this.onerror = null;

Can you make it private and pass onerror handler into the constructor, or
better add onError method to ChunkedTransferrerDelegate?

> Source/WebCore/inspector/front-end/FileUtils.js:74
> +    start: function(output)

Why do we have separate ChunkedTransferrer and not merge it into
ChunkedTransferrerDelegate ?

> Source/WebCore/inspector/front-end/FileUtils.js:87
> +    getLoadedSize: function()

We don't use get prefix for getters in WebKit.

> Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:-45
> -    startLoading: function(callback)

Why did it change? It should only be invoked by HeapSnapshotLoaderProxy and I
don't see why it is affected by this patch.

> Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:69
> +	   for (var i = startIndex; i < lastIndex; ++i) {

Why did it change?

> Source/WebCore/inspector/front-end/HeapSnapshotProxy.js:404
> +	      
snapshotProxy.updateStaticData(notifyPendingConsumers.bind(this));

Could you make heap profiler loading changes in a separate patch?