Bug 54385 – Web Inspector: [Chromium] Landing detailed heap snapshots, part 3

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Mikhail Naganov]

Provide node classes for data grid views:
 - summary -- list of objects by constuctor;
 - diff -- summaries btw two snapshots;
 - explorer -- explore heap graph;
 - dominator tree;

[Mikhail Naganov]

Created attachment 82301 [details]
patch

[Pavel Feldman]

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.

[Mikhail Naganov]

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.

[Mikhail Naganov]

Created attachment 82332 [details]
patch

[Pavel Feldman]

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?

[Mikhail Naganov]

@pfeldman: OK. I will move it to an iframe.

Manually committed http://trac.webkit.org/changeset/78700


2011-02-14  Mikhail Naganov  <[e-mail]>

        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:

[WebKit Review Bot]

http://trac.webkit.org/changeset/78700 might have broken GTK Linux 32-bit Release