Created attachment 144469 [details] Patch

Comment on attachment 144469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144469&action=review > Source/WebCore/ChangeLog:7 > + It declares the API for HSLoader, HSLoaderProxy and HSSaveToFileReceiver. Please don't abbreviate, use HeapSnapshotLoader etc. > Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:52 > + this._reset(); It looks as if you were going to reuse the object after dispose. > Source/WebCore/inspector/front-end/HeapSnapshotView.js:839 > + this._loader = null; _loader -> _receiver > Source/WebCore/inspector/front-end/HeapSnapshotView.js:936 > + this.sidebarElement.subtitle = Number.bytesToString(this._snapshotProxy.totalSize); This will fail if _snapshotProxy is null. > Source/WebCore/inspector/front-end/HeapSnapshotView.js:1016 > + this._file = file; All file loading stuff can be extracted into a separate entity. > LayoutTests/inspector/profiler/heap-snapshot-loader.html:42 > + maxJSObjectId: 6 Can you extract this value from the snapshot mock?

Committed r118742: <http://trac.webkit.org/changeset/118742>

(In reply to comment #2) > (From update of attachment 144469 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144469&action=review > > > Source/WebCore/ChangeLog:7 > > + It declares the API for HSLoader, HSLoaderProxy and HSSaveToFileReceiver. > > Please don't abbreviate, use HeapSnapshotLoader etc. > done > > Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:52 > > + this._reset(); > > It looks as if you were going to reuse the object after dispose. > technically it is possible but practically it lives in a worker and is using only once. > > Source/WebCore/inspector/front-end/HeapSnapshotView.js:839 > > + this._loader = null; > > _loader -> _receiver done > > > Source/WebCore/inspector/front-end/HeapSnapshotView.js:936 > > + this.sidebarElement.subtitle = Number.bytesToString(this._snapshotProxy.totalSize); > > This will fail if _snapshotProxy is null. We can save a snapshot only if it has been loaded. In such case _snapshotProxy property is set. > > > Source/WebCore/inspector/front-end/HeapSnapshotView.js:1016 > > + this._file = file; > > All file loading stuff can be extracted into a separate entity. I'll do that in a separate patch. > > > LayoutTests/inspector/profiler/heap-snapshot-loader.html:42 > > + maxJSObjectId: 6 > > Can you extract this value from the snapshot mock?

This test fails on Lion and Qt. * inspector/profiler/heap-snapshot-loader.html I created a new bug report. * https://bugs.webkit.org/show_bug.cgi?id=87741