Bug 157371 - Web Inspector: HeapSnapshot should exploratory Object Graph view
Summary: Web Inspector: HeapSnapshot should exploratory Object Graph view
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-05-04 22:29 PDT by Joseph Pecoraro
Modified: 2016-05-05 19:35 PDT (History)
11 users (show)

See Also:


Attachments
[IMAGE] JSContext GlobalObject Object Graph (369.66 KB, image/png)
2016-05-04 22:30 PDT, Joseph Pecoraro
no flags Details
[IMAGE] Web Page Windows List (377.90 KB, image/png)
2016-05-04 22:31 PDT, Joseph Pecoraro
no flags Details
[PATCH] Work In Progress - Needs More Testing (48.12 KB, patch)
2016-05-04 22:34 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (57.00 KB, patch)
2016-05-05 12:17 PDT, 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-05-04 22:29:34 PDT
* SUMMARY
HeapSnapshot should exploratory Object Graph view

Should be able to explore the Heap. A good starting point are the global objects themselves. This avoids complexity around things the VM references directly (Conservative Roots, JSGlobalObject structures, small strings, DOMWrapperWorld references, etc) but its still understandable to users:

• JSContext Inspector - show the Global Object
• Web Inspector - show the Window objects
Comment 1 Joseph Pecoraro 2016-05-04 22:30:11 PDT
Created attachment 278157 [details]
[IMAGE] JSContext GlobalObject Object Graph
Comment 2 Joseph Pecoraro 2016-05-04 22:31:27 PDT
Created attachment 278158 [details]
[IMAGE] Web Page Windows List

Since the Object Preview was mostly useless on windows, I changed the preview to compute the `window.location.href` for the window object. That tends to be much more useful for pages with lots and lots of frames. But we still might need to make this stronger.
Comment 3 Radar WebKit Bug Importer 2016-05-04 22:33:03 PDT
<rdar://problem/26107304>
Comment 4 Joseph Pecoraro 2016-05-04 22:34:57 PDT
Created attachment 278159 [details]
[PATCH] Work In Progress - Needs More Testing

This basically shares all the code. The Object Graph view just shows the 
"Window" || "GlobalObject" instances at the top level, and when expanding we always show the retained size.

I want to test it more tomorrow:
- Window objects seem to be kept alive across navigations making the graph output poor
- Is window.location.href enough? That screenshot is kinda heavy, but that page is heavy.
Comment 5 Joseph Pecoraro 2016-05-04 22:37:10 PDT
Comment on attachment 278159 [details]
[PATCH] Work In Progress - Needs More Testing

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

> Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstancesContentView.css:-54
> -.heap-snapshot > .data-grid tr:not(.selected) td .preview-error {
> -    color: red;
> -}

The "Internal Object" and "Preview Not Available" messages were standing out way too much. This makes them black text, much easier on the eyes.
Comment 6 Joseph Pecoraro 2016-05-05 12:17:05 PDT
Created attachment 278179 [details]
[PATCH] Proposed Fix
Comment 7 WebKit Commit Bot 2016-05-05 13:33:38 PDT
Comment on attachment 278179 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 278179

Committed r200474: <http://trac.webkit.org/changeset/200474>
Comment 8 WebKit Commit Bot 2016-05-05 13:33:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2016-05-05 13:48:34 PDT
should exploratory?
Comment 10 Joseph Pecoraro 2016-05-05 13:51:49 PDT
(In reply to comment #9)
> should exploratory?

=(. *should have*.
Comment 11 Joseph Pecoraro 2016-05-05 19:35:24 PDT
Comment on attachment 278179 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:189
>                  WebInspector.runtimeManager.evaluateInInspectedWindow("(" + getCompletions + ")(\"" + result.type + "\")", "completion", false, true, true, false, false, receivedPropertyNamesFromEvaluate.bind(this));

Missed renaming this `getCompletions` =(