[webkit-reviews] review denied: [Bug 97226] Web Inspector: [refactoring] simplify interface to FileOutputStream : [Attachment 164948] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 97226: Web Inspector: [refactoring] simplify interface to FileOutputStream
https://bugs.webkit.org/show_bug.cgi?id=97226

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

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


> Source/WebCore/inspector/front-end/FileUtils.js:316
> +	  
WebInspector.fileManager.addEventListener(WebInspector.FileManager.EventTypes.S
avedURL, callbackWrapper, this);

callbackWrapper -> callbackWrapper.bind(this)

> Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:-141
> -	       this.transferChunk("");

I'd say that after this change it is easier to break the code by changing one
of the cases.

> Source/WebCore/inspector/front-end/HeapSnapshotView.js:864
> +	       this._numberOfChunks = 0;

Now it is only reset if there is now worker yet, is it intentional?

> Source/WebCore/inspector/front-end/HeapSnapshotView.js:936
> +	   if (!transferFinished && this._receiver)

You should make sure the receiver is closed when it is reset to null.