Bug 55563

Summary: Web Inspector: [Chromium] Landing detailed heap snapshots, part 4
Product: WebKit Reporter: Mikhail Naganov <mnaganov>
Component: Web Inspector (Deprecated)Assignee: Mikhail Naganov <mnaganov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit-ews, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 50510    
Attachments:
Description Flags
Screenshot -- Summary data grid
none
Screenshot -- Comparison data grid
none
Screenshot -- Containment data grid
none
Screenshot -- Dominators data grid
none
Screenshot -- Retaining paths list
none
Screenshot -- Legend popup
none
patch
pfeldman: review-, mnaganov: commit-queue-
addressed comments and fixed JSC binding
mnaganov: commit-queue-
now --binary diff pfeldman: review+, mnaganov: commit-queue-

Description Mikhail Naganov 2011-03-02 02:23:51 PST
This part adds implementations for data grids used to display different heap snapshots projections. We are almost done.
Comment 1 Mikhail Naganov 2011-03-02 02:27:55 PST
Created attachment 84385 [details]
Screenshot -- Summary data grid
Comment 2 Mikhail Naganov 2011-03-02 02:29:06 PST
Created attachment 84386 [details]
Screenshot -- Comparison data grid
Comment 3 Mikhail Naganov 2011-03-02 02:30:30 PST
Created attachment 84387 [details]
Screenshot -- Containment data grid
Comment 4 Mikhail Naganov 2011-03-02 02:31:46 PST
Created attachment 84388 [details]
Screenshot -- Dominators data grid
Comment 5 Mikhail Naganov 2011-03-02 02:32:49 PST
Created attachment 84389 [details]
Screenshot -- Retaining paths list
Comment 6 Mikhail Naganov 2011-03-02 02:34:02 PST
Created attachment 84390 [details]
Screenshot -- Legend popup
Comment 7 Alexander Pavlov (apavlov) 2011-03-02 02:42:13 PST
(In reply to comment #6)
> Created an attachment (id=84390) [details]
> Screenshot -- Legend popup

The column titles should read, "Property types" and "Object types" (without plural for the first noun)
Comment 8 Mikhail Naganov 2011-03-02 02:43:05 PST
Created attachment 84392 [details]
patch
Comment 9 Pavel Feldman 2011-03-02 02:52:57 PST
Comment on attachment 84392 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84392&action=review

> Source/WebCore/bindings/v8/ScriptHeapSnapshot.h:62
> +    int getExactRetainedSize(uint64_t nodeId);

exactRetainedSize

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:118
> +    _getSortFields: function(sortColumn, sortAscending)

_sortFields

> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:746
> +        if (this.views[this.views.current] === "Containment") {

Are you comparing to localized strings?
Comment 10 Early Warning System Bot 2011-03-02 03:23:49 PST
Attachment 84392 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8071874
Comment 11 Mikhail Naganov 2011-03-02 05:13:39 PST
Created attachment 84404 [details]
addressed comments and fixed JSC binding
Comment 12 Mikhail Naganov 2011-03-02 05:13:54 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=84390) [details] [details]
> > Screenshot -- Legend popup
> 
> The column titles should read, "Property types" and "Object types" (without plural for the first noun)

Fixed
Comment 13 Mikhail Naganov 2011-03-02 05:15:09 PST
(In reply to comment #9)
> (From update of attachment 84392 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84392&action=review
> 
> > Source/WebCore/bindings/v8/ScriptHeapSnapshot.h:62
> > +    int getExactRetainedSize(uint64_t nodeId);
> 
> exactRetainedSize
> 

Fixed

> > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:118
> > +    _getSortFields: function(sortColumn, sortAscending)
> 
> _sortFields
> 

Fixed

> > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:746
> > +        if (this.views[this.views.current] === "Containment") {
> 
> Are you comparing to localized strings?

No. 'views' contains non-localized strings. See DetailedHeapShotview.js:404
Comment 14 Mikhail Naganov 2011-03-02 05:19:38 PST
Created attachment 84409 [details]
now --binary diff
Comment 15 Mikhail Naganov 2011-03-02 08:47:11 PST
Manually committed as http://trac.webkit.org/changeset/80135

2011-03-02  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Pavel Feldman.

        Web Inspector: [Chromium] Landing detailed heap snapshots, part 4.
        https://bugs.webkit.org/show_bug.cgi?id=55563

        This part adds implementations for data grids used to display
        different heap snapshots projections. We are almost done.

        * English.lproj/localizedStrings.js:
        * WebCore.gypi:
        * bindings/v8/ScriptHeapSnapshot.cpp:
        (WebCore::ScriptHeapSnapshot::getExactRetainedSize):
        * bindings/v8/ScriptHeapSnapshot.h:
        * inspector/Inspector.idl:
        * inspector/InspectorProfilerAgent.cpp:
        (WebCore::InspectorProfilerAgent::getExactHeapSnapshotNodeRetainedSize):
        * inspector/InspectorProfilerAgent.h:
        * inspector/front-end/DetailedHeapshotGridNodes.js:
        (WebInspector.HeapSnapshotObjectNode):
        (WebInspector.HeapSnapshotObjectNode.prototype._createProvider):
        (WebInspector.HeapSnapshotInstanceNode):
        (WebInspector.HeapSnapshotInstanceNode.prototype._createProvider):
        (WebInspector.HeapSnapshotDominatorObjectNode):
        (WebInspector.HeapSnapshotDominatorObjectNode.prototype._createProvider):
        (MixInSnapshotNodeFunctions):
        * inspector/front-end/DetailedHeapshotView.js:
        (WebInspector.HeapSnapshotContainmentDataGrid):
        (WebInspector.HeapSnapshotSortableDataGrid):
        (WebInspector.HeapSnapshotConstructorsDataGrid):
        (WebInspector.HeapSnapshotDiffDataGrid):
        (WebInspector.HeapSnapshotDominatorsDataGrid):
        (WebInspector.HeapSnapshotRetainingPathsList):
        (WebInspector.DetailedHeapshotView.profileCallback):
        (WebInspector.DetailedHeapshotView):
        * inspector/front-end/HeapSnapshot.js:
        (WebInspector.HeapSnapshotEdge.prototype.get isInvisible):
        (WebInspector.HeapSnapshotEdge.prototype.toString):
        (WebInspector.HeapSnapshot.prototype._init):
        (WebInspector.HeapSnapshot.prototype._buildAggregatesIndexes):
        (WebInspector.HeapSnapshot.prototype._markInvisibleEdges):
        (WebInspector.HeapSnapshotPathFinder.prototype._skipEdge):
        * inspector/front-end/Images/helpButtonGlyph.png: Added.
        * inspector/front-end/Panel.js:
        (WebInspector.Panel.prototype.reset):
        * inspector/front-end/Popover.js:
        (WebInspector.Popover):
        (WebInspector.Popover.prototype.show):
        (WebInspector.Popover.prototype.hide):
        (WebInspector.Popover.prototype.get visible):
        * inspector/front-end/ProfilesPanel.js:
        (WebInspector.ProfilesPanel.prototype._reset):
        (WebInspector.ProfilesPanel.prototype.getProfile):
        * inspector/front-end/heapProfiler.css:
        * inspector/front-end/inspector.js:
        (WebInspector.resetFocusElement):