Summary: | Web Inspector: [Chromium] Landing detailed heap snapshots, part 3 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Naganov <mnaganov> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Mikhail Naganov <mnaganov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, apavlov, bweinstein, eric, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 50510 | ||||||||
Attachments: |
|
Description
Mikhail Naganov
2011-02-14 03:25:32 PST
Created attachment 82301 [details]
patch
Comment on attachment 82301 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=82301&action=review > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:34 > + this.defaultPopulateCount = populateCount; _defaultPopulateCount > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:35 > + this.provider = null; this._provider > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:53 > + this.provider.sort(this.comparator()); { on the next line. > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:325 > + this.isDeletedNode = !!baseSnapshot; Should be private > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:399 > + this.name = constructor; Make thing private. > Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:476 > + this.it1 = it1; Make things private please. Comment on attachment 82301 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=82301&action=review >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:34 >> + this.defaultPopulateCount = populateCount; > > _defaultPopulateCount Done. >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:35 >> + this.provider = null; > > this._provider Done. >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:53 >> + this.provider.sort(this.comparator()); > > { on the next line. Done. >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:325 >> + this.isDeletedNode = !!baseSnapshot; > > Should be private Done. >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:399 >> + this.name = constructor; > > Make thing private. Done. >> Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js:476 >> + this.it1 = it1; > > Make things private please. Done. Created attachment 82332 [details]
patch
Comment on attachment 82332 [details]
patch
I am concerned with the amount of the code you are adding. We already show up for too long. Could you please make your code for profiler panel reside in an iframe and load on demand?
@pfeldman: OK. I will move it to an iframe. Manually committed http://trac.webkit.org/changeset/78700 2011-02-14 Mikhail Naganov <mnaganov@chromium.org> Reviewed by Pavel Feldman. Web Inspector: [Chromium] Landing detailed heap snapshots, part 3. https://bugs.webkit.org/show_bug.cgi?id=54385 Provide node classes for data grid views: - summary -- list of objects by constuctor; - diff -- summaries btw two snapshots; - explorer -- explore heap graph; - dominators tree; * WebCore.gypi: * WebCore.vcproj/WebCore.vcproj: * inspector/front-end/DetailedHeapshotGridNodes.js: Added. * inspector/front-end/HeapSnapshot.js: (WebInspector.HeapSnapshotFilteredOrderedIterator): (WebInspector.HeapSnapshotFilteredOrderedIterator.prototype.incInstancesCount): (WebInspector.HeapSnapshotFilteredOrderedIterator.prototype.get instancesCount): (WebInspector.HeapSnapshotFilteredOrderedIterator.prototype.next): (WebInspector.HeapSnapshotFilteredOrderedIterator.prototype.resetInstancesCount): (WebInspector.HeapSnapshotEdgesProvider): (WebInspector.HeapSnapshotNodesProvider): * inspector/front-end/PleaseWaitMessage.js: (WebInspector.PleaseWaitMessage.prototype.get instance): * inspector/front-end/WebKit.qrc: * inspector/front-end/inspector.html: http://trac.webkit.org/changeset/78700 might have broken GTK Linux 32-bit Release |