Web Inspector: Improve name sorting in HeapSnapshot data grids
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Nov 2016 04:22:21 +0000 (04:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Nov 2016 04:22:21 +0000 (04:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165170
<rdar://problem/28784421>

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2016-11-29
Reviewed by Matt Baker.

When sorting the Name column, group named properties and unnamed
properties and sort them each individually:

  - Sort named properties by their property name (property names will be unique if they exist)
  - Sort unnamed properties by their class name (guaranteed)
  - Sort any tied class names by their object id

This makes using the Object Graph with Name sort easier to follow.
In the ascending sort you see all the named properties first,
followed by the unnamed (internal) properties.

* UserInterface/Views/HeapSnapshotContentView.js:
(WebInspector.HeapSnapshotObjectGraphContentView):
Since this data grid column now sorts on more than just the "Class Name"
rename it to "Name".

* UserInterface/Views/HeapSnapshotDataGridTree.js:
(WebInspector.HeapSnapshotDataGridTree.buildSortComparator):
Make the sort of the `className` column more general to handle sorting
by property names, class names, and object identifiers.

* UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:
(WebInspector.HeapSnapshotInstanceDataGridNode.prototype.get propertyName):
(WebInspector.HeapSnapshotInstanceDataGridNode.prototype.createCellContent):
Provide a lazy `propertyName` accessor where we compute it once and stash
it on the DataGridNode to avoid extra work when resorting.

(WebInspector.HeapSnapshotInstanceDataGridNode.prototype._populate.propertyName):
(WebInspector.HeapSnapshotInstanceDataGridNode.prototype._populate):
In the initial populated sort, provide the necessary property name property
the sort comparator expects.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209115 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotContentView.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js

index 090c95c..f454a14 100644 (file)
@@ -1,3 +1,43 @@
+2016-11-29  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Improve name sorting in HeapSnapshot data grids
+        https://bugs.webkit.org/show_bug.cgi?id=165170
+        <rdar://problem/28784421>
+
+        Reviewed by Matt Baker.
+
+        When sorting the Name column, group named properties and unnamed
+        properties and sort them each individually:
+
+          - Sort named properties by their property name (property names will be unique if they exist)
+          - Sort unnamed properties by their class name (guaranteed)
+          - Sort any tied class names by their object id
+
+        This makes using the Object Graph with Name sort easier to follow.
+        In the ascending sort you see all the named properties first,
+        followed by the unnamed (internal) properties.
+
+        * UserInterface/Views/HeapSnapshotContentView.js:
+        (WebInspector.HeapSnapshotObjectGraphContentView):
+        Since this data grid column now sorts on more than just the "Class Name"
+        rename it to "Name".
+
+        * UserInterface/Views/HeapSnapshotDataGridTree.js:
+        (WebInspector.HeapSnapshotDataGridTree.buildSortComparator):
+        Make the sort of the `className` column more general to handle sorting
+        by property names, class names, and object identifiers.
+
+        * UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:
+        (WebInspector.HeapSnapshotInstanceDataGridNode.prototype.get propertyName):
+        (WebInspector.HeapSnapshotInstanceDataGridNode.prototype.createCellContent):
+        Provide a lazy `propertyName` accessor where we compute it once and stash
+        it on the DataGridNode to avoid extra work when resorting.
+
+        (WebInspector.HeapSnapshotInstanceDataGridNode.prototype._populate.propertyName):
+        (WebInspector.HeapSnapshotInstanceDataGridNode.prototype._populate):
+        In the initial populated sort, provide the necessary property name property
+        the sort comparator expects.
+
 2016-11-29  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: Breakpoints button enabled state is too subtle
index b78a9f6..bb46246 100644 (file)
@@ -140,7 +140,6 @@ localizedStrings["Charge ā€˜%sā€™ to Callers"] = "Charge ā€˜%sā€™ to Callers";
 localizedStrings["Checked"] = "Checked";
 localizedStrings["Child Layers"] = "Child Layers";
 localizedStrings["Children"] = "Children";
-localizedStrings["Class Name"] = "Class Name";
 localizedStrings["Classes"] = "Classes";
 localizedStrings["Clear"] = "Clear";
 localizedStrings["Clear Log"] = "Clear Log";
index f4a7756..4758465 100644 (file)
@@ -118,7 +118,7 @@ WebInspector.HeapSnapshotInstancesContentView = class HeapSnapshotInstancesConte
                 sortable: true,
             },
             className: {
-                title: WebInspector.UIString("Class Name"),
+                title: WebInspector.UIString("Name"),
                 sortable: true,
                 disclosure: true,
             }
@@ -147,7 +147,7 @@ WebInspector.HeapSnapshotObjectGraphContentView = class HeapSnapshotObjectGraphC
                 sortable: true,
             },
             className: {
-                title: WebInspector.UIString("Class Name"),
+                title: WebInspector.UIString("Name"),
                 sortable: true,
                 disclosure: true,
             }
index 578a98e..7057f4c 100644 (file)
@@ -51,7 +51,26 @@ WebInspector.HeapSnapshotDataGridTree = class HeapSnapshotDataGridTree extends W
     {
         let multiplier = sortOrder === WebInspector.DataGrid.SortOrder.Ascending ? 1 : -1;
         let numberCompare = (columnIdentifier, a, b) => multiplier * (a.data[columnIdentifier] - b.data[columnIdentifier]);
-        let localeCompare = (columnIdentifier, a, b) => multiplier * (a.data[columnIdentifier].localeCompare(b.data[columnIdentifier]));
+        let nameCompare = (a, b) => {
+            // Sort by property name if available. Property names before no property name.
+            if (a.propertyName || b.propertyName) {
+                if (a.propertyName && !b.propertyName)
+                    return multiplier * -1;
+                if (!a.propertyName && b.propertyName)
+                    return multiplier * 1;
+                let propertyNameCompare = a.propertyName.localeCompare(b.propertyName);
+                console.assert(propertyNameCompare !== 0, "Property names should be unique, we shouldn't have equal property names.");
+                return multiplier * propertyNameCompare;
+            }
+
+            // Sort by class name and object id if no property name.
+            let classNameCompare = a.data.className.localeCompare(b.data.className);
+            if (classNameCompare)
+                return multiplier * classNameCompare;
+            if (a.data.id || b.data.id)
+                return multiplier * (a.data.id - b.data.id);
+            return 0;
+        };
 
         switch (columnIdentifier) {
         case "retainedSize":
@@ -61,7 +80,7 @@ WebInspector.HeapSnapshotDataGridTree = class HeapSnapshotDataGridTree extends W
         case "count":
             return numberCompare.bind(this, "count");
         case "className":
-            return localeCompare.bind(this, "className");
+            return nameCompare;
         }
     }
 
index fa75d25..72c816d 100644 (file)
@@ -97,6 +97,16 @@ WebInspector.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGr
     get node() { return this._node; }
     get selectable() { return false; }
 
+    get propertyName()
+    {
+        if (!this._edge)
+            return "";
+
+        if (!this._propertyName)
+            this._propertyName = WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(this._edge);
+        return this._propertyName;
+    }
+
     createCells()
     {
         super.createCells();
@@ -146,9 +156,9 @@ WebInspector.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGr
 
             if (this._edge) {
                 let nameElement = containerElement.appendChild(document.createElement("span"));
-                let edgeText = WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(this._edge);
-                if (edgeText)
-                    nameElement.textContent = edgeText + ": " + this._node.className + " ";
+                let propertyName = this.propertyName;
+                if (propertyName)
+                    nameElement.textContent = propertyName + ": " + this._node.className + " ";
                 else
                     nameElement.textContent = this._node.className + " ";
             }
@@ -208,14 +218,18 @@ WebInspector.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGr
     {
         this.removeEventListener("populate", this._populate, this);
 
+        function propertyName(edge) {
+            return edge ? WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(edge) : "";
+        }
+
         this._node.retainedNodes((instances, edges) => {
             // Reference edge from instance so we can get it after sorting.
             for (let i = 0; i < instances.length; ++i)
                 instances[i].__edge = edges[i];
 
             instances.sort((a, b) => {
-                let fakeDataGridNodeA = {data: a};
-                let fakeDataGridNodeB = {data: b};
+                let fakeDataGridNodeA = {data: a, propertyName: propertyName(a.__edge)};
+                let fakeDataGridNodeB = {data: b, propertyName: propertyName(b.__edge)};
                 return this._tree._sortComparator(fakeDataGridNodeA, fakeDataGridNodeB);
             });