Bug 165170 - Web Inspector: Improve name sorting in HeapSnapshot data grids
Summary: Web Inspector: Improve name sorting in HeapSnapshot data grids
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-11-29 16:00 PST by Joseph Pecoraro
Modified: 2016-11-30 10:59 PST (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (8.01 KB, patch)
2016-11-29 16:08 PST, Joseph Pecoraro
mattbaker: review+
Details | Formatted Diff | Diff
[IMAGE] Object Graph with improved name sort (301.21 KB, image/png)
2016-11-29 16:09 PST, Joseph Pecoraro
no flags Details
[IMAGE] Instances with improved name sort (object id) (229.09 KB, image/png)
2016-11-29 16:09 PST, Joseph Pecoraro
no flags Details
[PATCH] For Landing (8.96 KB, patch)
2016-11-29 17:19 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-11-29 16:00:52 PST
Summary:
Improve name sorting in HeapSnapshot data grids

Steps to Reproduce:
1. Inspect this page
2. Take a Heap Snapshot
3. Drill into Heap Snapshot > Object Graph
4. Expand Window object
5. Sort by "Class Name" column
  => Sorting is difficult to follow. Named properties are mingled with unnamed (internal) properties.

A better sort would be grouping named properties and unnamed properties:

  - 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

Now "Class Name" becomes an ambiguous DataGrid column title in Object Graph, so rename it to just "Name".

This sort will improve the "Class Name" sort in both the Instances view and the Object Graph, because we fallback to sorting by object id in ties.
Comment 1 Joseph Pecoraro 2016-11-29 16:01:16 PST
<rdar://problem/28784421>
Comment 2 Joseph Pecoraro 2016-11-29 16:08:41 PST
Created attachment 295663 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2016-11-29 16:09:02 PST
Created attachment 295664 [details]
[IMAGE] Object Graph with improved name sort
Comment 4 Joseph Pecoraro 2016-11-29 16:09:28 PST
Created attachment 295666 [details]
[IMAGE] Instances with improved name sort (object id)
Comment 5 Matt Baker 2016-11-29 16:54:40 PST
(In reply to comment #4)
> Created attachment 295666 [details]
> [IMAGE] Instances with improved name sort (object id)

Was this screenshot taken before the column rename to "Name"?
Comment 6 Joseph Pecoraro 2016-11-29 16:59:39 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Created attachment 295666 [details]
> > [IMAGE] Instances with improved name sort (object id)
> 
> Was this screenshot taken before the column rename to "Name"?

Nope, the Instances view keeps the name "Class Name". Only the "Object Graph" one changed to "Name". Do you think it would make sense to change both to be "Name"?
Comment 7 Matt Baker 2016-11-29 17:04:19 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Created attachment 295666 [details]
> > > [IMAGE] Instances with improved name sort (object id)
> > 
> > Was this screenshot taken before the column rename to "Name"?
> 
> Nope, the Instances view keeps the name "Class Name". Only the "Object
> Graph" one changed to "Name". Do you think it would make sense to change
> both to be "Name"?

I think they should both be "Name", since the column has a mix of class names, property names, etc.
Comment 8 Joseph Pecoraro 2016-11-29 17:19:19 PST
Created attachment 295678 [details]
[PATCH] For Landing
Comment 9 WebKit Commit Bot 2016-11-29 20:22:41 PST
Comment on attachment 295678 [details]
[PATCH] For Landing

Clearing flags on attachment: 295678

Committed r209115: <http://trac.webkit.org/changeset/209115>