We would like to save HeapSnapshots. The simplest solution for now is to force the back-end to push the snapshot to the front-end second time and persist it to the disk with help of save and append operations of the embedder.
Created attachment 140185 [details] Patch
Comment on attachment 140185 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140185&action=review > Source/WebCore/inspector/Inspector.json:2541 > + "name": "typeId", Please put all properties in of an entry on one line as in the rest code. > Source/WebCore/inspector/Inspector.json:2547 > + "name": "title", This part of change may well be done in a separate CL. > Source/WebCore/inspector/Inspector.json:2572 > + "optional": true Add a description? > Source/WebCore/inspector/Inspector.json:2577 > + "optional": true Add a description? > Source/WebCore/inspector/front-end/DebuggerModel.js:47 > + this._scriptsWithSourceURL = {}; Look like a part of a different change. Please revert. > Source/WebCore/inspector/front-end/ProfilesPanel.js:149 > + if (this._saveOperationInProgress) { What if "save profile" is invoked before the profile load has completed? > Source/WebCore/inspector/front-end/ProfilesPanel.js:153 > + if (!this.proxy) Where does this proxy property come from? > Source/WebCore/inspector/front-end/ProfilesPanel.js:172 > + this._saveToFile = fileName; _saveToFile -> _fileName? > Source/WebCore/inspector/front-end/ProfilesPanel.js:525 > + temporaryProfile: function(typeId) temporaryProfile -> findTemporaryProfile > Source/WebCore/inspector/front-end/ProfilesPanel.js:927 > + if (!!temporaryProfile !== isProfiling) { Prefer early return. > Source/WebCore/inspector/front-end/ProfilesPanel.js:1072 > + contextMenu.appendItem(WebInspector.UIString("Save profile"), profile.save.bind(profile)); localizedStrings.js change is missing > Source/WebCore/inspector/front-end/inspector.js:1043 > + WebInspector.saveCallbacks[url] = callback; Can callback be null? Why saveCallbacks is not private? What should happen to the callback if save is invoked several times with same URL? > Source/WebCore/inspector/front-end/inspector.js:1065 > +WebInspector.appendCallbacks = []; Should be private
Comment on attachment 140185 [details] Patch Attachment 140185 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12634124 New failing tests: inspector/profiler/heap-snapshot-comparison-expansion-preserved-when-sorting.html inspector/profiler/heap-snapshot-summary-show-all.html inspector/profiler/heap-snapshot-dominators-shown-node-count-preserved-when-sorting.html inspector/profiler/heap-snapshot-dominators-expansion-preserved-when-sorting.html http/tests/security/sandboxed-iframe-modify-self.html fast/frames/cached-frame-counter.html accessibility/aria-disabled.html fast/loader/text-document-wrapping.html inspector/profiler/heap-snapshot-summary-sorting.html inspector/profiler/heap-snapshot-comparison-sorting.html inspector/profiler/heap-snapshot-dominators-show-all.html inspector/profiler/heap-snapshot-summary-expansion-preserved-when-sorting.html inspector/profiler/heap-snapshot-dominators-show-next.html inspector/profiler/heap-snapshot-comparison-shown-node-count-preserved-when-sorting.html fast/canvas/webgl/shader-precision-format.html inspector/profiler/heap-snapshot-comparison-show-next.html inspector/profiler/heap-snapshot-comparison-show-all.html inspector/profiler/heap-snapshot-containment-expansion-preserved-when-sorting.html inspector/profiler/heap-snapshot-dominators-sorting.html inspector/profiler/heap-snapshot-comparison-dom-groups-change.html inspector/profiler/heap-snapshot-containment-sorting.html inspector/profiler/heap-snapshot-containment-show-all.html http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html inspector/profiler/heap-snapshot-containment-show-next.html inspector/profiler/heap-snapshot-summary-show-next.html fast/loader/javascript-url-in-embed.html inspector/profiler/heap-snapshot-summary-shown-node-count-preserved-when-sorting.html inspector/profiler/heap-snapshot-containment-shown-node-count-preserved-when-sorting.html
Created attachment 140217 [details] Archive of layout-test-results from ec2-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 140384 [details] Patch
Created attachment 140386 [details] Patch
Comment on attachment 140386 [details] Patch 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.
(In reply to comment #2) > (From update of attachment 140185 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140185&action=review > > > Source/WebCore/inspector/Inspector.json:2541 > > + "name": "typeId", > > Please put all properties in of an entry on one line as in the rest code. done in separate patch > > > Source/WebCore/inspector/Inspector.json:2547 > > + "name": "title", > > This part of change may well be done in a separate CL. done in separate patch > > > Source/WebCore/inspector/Inspector.json:2572 > > + "optional": true > > Add a description? I'll do it in separate patch > > > Source/WebCore/inspector/Inspector.json:2577 > > + "optional": true > > Add a description? I'll do it in separate patch > > > Source/WebCore/inspector/front-end/DebuggerModel.js:47 > > + this._scriptsWithSourceURL = {}; > > Look like a part of a different change. Please revert. it was obsolete change. > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:149 > > + if (this._saveOperationInProgress) { > > What if "save profile" is invoked before the profile load has completed? done > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:153 > > + if (!this.proxy) > > Where does this proxy property come from? It was done in a method of ProfilesPanel. Now it is lazily initialized in HeapProfileHeader.load. > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:172 > > + this._saveToFile = fileName; > > _saveToFile -> _fileName? done > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:525 > > + temporaryProfile: function(typeId) > > temporaryProfile -> findTemporaryProfile done in separate patch > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:927 > > + if (!!temporaryProfile !== isProfiling) { > > Prefer early return. done in separate patch > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:1072 > > + contextMenu.appendItem(WebInspector.UIString("Save profile"), profile.save.bind(profile)); > > localizedStrings.js change is missing > > > Source/WebCore/inspector/front-end/inspector.js:1043 > > + WebInspector.saveCallbacks[url] = callback; > > Can callback be null? Why saveCallbacks is not private? What should happen to the callback if save is invoked several times with same URL? done > > > Source/WebCore/inspector/front-end/inspector.js:1065 > > +WebInspector.appendCallbacks = []; > > Should be private done
Committed r116218: <http://trac.webkit.org/changeset/116218>
(In reply to comment #7) > (From update of attachment 140386 [details]) > 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. done > > > Source/WebCore/inspector/front-end/HeapSnapshotView.js:795 > > + if (!this.proxy) { > > Why is the proxy field public? done > > > Source/WebCore/inspector/front-end/HeapSnapshotView.js:830 > > + else { > > It should go on the previous line. done > > > Source/WebCore/inspector/front-end/ProfilesPanel.js:1108 > > + if (!profile.canSave || !profile.canSave()) > > Double check to be sure? This function exists only in HeapProfileHeader. I'd change this in a separate patch. > > > Source/WebCore/inspector/front-end/inspector.js:1044 > > + WebInspector._saveCallbacks[url] = callback; > > Please add and assert here if we cannot override existing callback. Such assert will prevent the second save operation if the first one failed because we have no notifications for unsuccessful saves. I'll fix this in a separate patch.