<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[209115] trunk/Source/WebInspectorUI</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/209115">209115</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-11-29 20:22:21 -0800 (Tue, 29 Nov 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Web Inspector: Improve name sorting in HeapSnapshot data grids
https://bugs.webkit.org/show_bug.cgi?id=165170
&lt;rdar://problem/28784421&gt;

Patch by Joseph Pecoraro &lt;pecoraro@apple.com&gt; 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 &quot;Class Name&quot;
rename it to &quot;Name&quot;.

* 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUILocalizationsenlprojlocalizedStringsjs">trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotContentView.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotDataGridTreejs">trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotInstanceDataGridNodejs">trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (209114 => 209115)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog        2016-11-30 03:50:41 UTC (rev 209114)
+++ trunk/Source/WebInspectorUI/ChangeLog        2016-11-30 04:22:21 UTC (rev 209115)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2016-11-29  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
+
+        Web Inspector: Improve name sorting in HeapSnapshot data grids
+        https://bugs.webkit.org/show_bug.cgi?id=165170
+        &lt;rdar://problem/28784421&gt;
+
+        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 &quot;Class Name&quot;
+        rename it to &quot;Name&quot;.
+
+        * 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.
+
</ins><span class="cx"> 2016-11-29  Matt Baker  &lt;mattbaker@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Breakpoints button enabled state is too subtle
</span></span></pre></div>
<a id="trunkSourceWebInspectorUILocalizationsenlprojlocalizedStringsjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js (209114 => 209115)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js        2016-11-30 03:50:41 UTC (rev 209114)
+++ trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js        2016-11-30 04:22:21 UTC (rev 209115)
</span><span class="lines">@@ -140,7 +140,6 @@
</span><span class="cx"> localizedStrings[&quot;Checked&quot;] = &quot;Checked&quot;;
</span><span class="cx"> localizedStrings[&quot;Child Layers&quot;] = &quot;Child Layers&quot;;
</span><span class="cx"> localizedStrings[&quot;Children&quot;] = &quot;Children&quot;;
</span><del>-localizedStrings[&quot;Class Name&quot;] = &quot;Class Name&quot;;
</del><span class="cx"> localizedStrings[&quot;Classes&quot;] = &quot;Classes&quot;;
</span><span class="cx"> localizedStrings[&quot;Clear&quot;] = &quot;Clear&quot;;
</span><span class="cx"> localizedStrings[&quot;Clear Log&quot;] = &quot;Clear Log&quot;;
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotContentView.js (209114 => 209115)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotContentView.js        2016-11-30 03:50:41 UTC (rev 209114)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotContentView.js        2016-11-30 04:22:21 UTC (rev 209115)
</span><span class="lines">@@ -118,7 +118,7 @@
</span><span class="cx">                 sortable: true,
</span><span class="cx">             },
</span><span class="cx">             className: {
</span><del>-                title: WebInspector.UIString(&quot;Class Name&quot;),
</del><ins>+                title: WebInspector.UIString(&quot;Name&quot;),
</ins><span class="cx">                 sortable: true,
</span><span class="cx">                 disclosure: true,
</span><span class="cx">             }
</span><span class="lines">@@ -147,7 +147,7 @@
</span><span class="cx">                 sortable: true,
</span><span class="cx">             },
</span><span class="cx">             className: {
</span><del>-                title: WebInspector.UIString(&quot;Class Name&quot;),
</del><ins>+                title: WebInspector.UIString(&quot;Name&quot;),
</ins><span class="cx">                 sortable: true,
</span><span class="cx">                 disclosure: true,
</span><span class="cx">             }
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotDataGridTreejs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js (209114 => 209115)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js        2016-11-30 03:50:41 UTC (rev 209114)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js        2016-11-30 04:22:21 UTC (rev 209115)
</span><span class="lines">@@ -51,8 +51,27 @@
</span><span class="cx">     {
</span><span class="cx">         let multiplier = sortOrder === WebInspector.DataGrid.SortOrder.Ascending ? 1 : -1;
</span><span class="cx">         let numberCompare = (columnIdentifier, a, b) =&gt; multiplier * (a.data[columnIdentifier] - b.data[columnIdentifier]);
</span><del>-        let localeCompare = (columnIdentifier, a, b) =&gt; multiplier * (a.data[columnIdentifier].localeCompare(b.data[columnIdentifier]));
</del><ins>+        let nameCompare = (a, b) =&gt; {
+            // Sort by property name if available. Property names before no property name.
+            if (a.propertyName || b.propertyName) {
+                if (a.propertyName &amp;&amp; !b.propertyName)
+                    return multiplier * -1;
+                if (!a.propertyName &amp;&amp; b.propertyName)
+                    return multiplier * 1;
+                let propertyNameCompare = a.propertyName.localeCompare(b.propertyName);
+                console.assert(propertyNameCompare !== 0, &quot;Property names should be unique, we shouldn't have equal property names.&quot;);
+                return multiplier * propertyNameCompare;
+            }
</ins><span class="cx"> 
</span><ins>+            // 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;
+        };
+
</ins><span class="cx">         switch (columnIdentifier) {
</span><span class="cx">         case &quot;retainedSize&quot;:
</span><span class="cx">             return numberCompare.bind(this, &quot;retainedSize&quot;);
</span><span class="lines">@@ -61,7 +80,7 @@
</span><span class="cx">         case &quot;count&quot;:
</span><span class="cx">             return numberCompare.bind(this, &quot;count&quot;);
</span><span class="cx">         case &quot;className&quot;:
</span><del>-            return localeCompare.bind(this, &quot;className&quot;);
</del><ins>+            return nameCompare;
</ins><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsHeapSnapshotInstanceDataGridNodejs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js (209114 => 209115)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js        2016-11-30 03:50:41 UTC (rev 209114)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js        2016-11-30 04:22:21 UTC (rev 209115)
</span><span class="lines">@@ -97,6 +97,16 @@
</span><span class="cx">     get node() { return this._node; }
</span><span class="cx">     get selectable() { return false; }
</span><span class="cx"> 
</span><ins>+    get propertyName()
+    {
+        if (!this._edge)
+            return &quot;&quot;;
+
+        if (!this._propertyName)
+            this._propertyName = WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(this._edge);
+        return this._propertyName;
+    }
+
</ins><span class="cx">     createCells()
</span><span class="cx">     {
</span><span class="cx">         super.createCells();
</span><span class="lines">@@ -146,9 +156,9 @@
</span><span class="cx"> 
</span><span class="cx">             if (this._edge) {
</span><span class="cx">                 let nameElement = containerElement.appendChild(document.createElement(&quot;span&quot;));
</span><del>-                let edgeText = WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(this._edge);
-                if (edgeText)
-                    nameElement.textContent = edgeText + &quot;: &quot; + this._node.className + &quot; &quot;;
</del><ins>+                let propertyName = this.propertyName;
+                if (propertyName)
+                    nameElement.textContent = propertyName + &quot;: &quot; + this._node.className + &quot; &quot;;
</ins><span class="cx">                 else
</span><span class="cx">                     nameElement.textContent = this._node.className + &quot; &quot;;
</span><span class="cx">             }
</span><span class="lines">@@ -208,6 +218,10 @@
</span><span class="cx">     {
</span><span class="cx">         this.removeEventListener(&quot;populate&quot;, this._populate, this);
</span><span class="cx"> 
</span><ins>+        function propertyName(edge) {
+            return edge ? WebInspector.HeapSnapshotRootPath.pathComponentForIndividualEdge(edge) : &quot;&quot;;
+        }
+
</ins><span class="cx">         this._node.retainedNodes((instances, edges) =&gt; {
</span><span class="cx">             // Reference edge from instance so we can get it after sorting.
</span><span class="cx">             for (let i = 0; i &lt; instances.length; ++i)
</span><span class="lines">@@ -214,8 +228,8 @@
</span><span class="cx">                 instances[i].__edge = edges[i];
</span><span class="cx"> 
</span><span class="cx">             instances.sort((a, b) =&gt; {
</span><del>-                let fakeDataGridNodeA = {data: a};
-                let fakeDataGridNodeB = {data: b};
</del><ins>+                let fakeDataGridNodeA = {data: a, propertyName: propertyName(a.__edge)};
+                let fakeDataGridNodeB = {data: b, propertyName: propertyName(b.__edge)};
</ins><span class="cx">                 return this._tree._sortComparator(fakeDataGridNodeA, fakeDataGridNodeB);
</span><span class="cx">             });
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>