Bug 214054 - Heap Snapshot Object Graph view doesn't get populated in some cases when inspecting a JSContext
Summary: Heap Snapshot Object Graph view doesn't get populated in some cases when insp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Christopher Reid
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-07 13:19 PDT by Christopher Reid
Modified: 2020-07-08 11:52 PDT (History)
9 users (show)

See Also:


Attachments
patch (1.91 KB, patch)
2020-07-07 14:12 PDT, Christopher Reid
no flags Details | Formatted Diff | Diff
Patch for landing (2.47 KB, patch)
2020-07-08 11:29 PDT, Christopher Reid
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Reid 2020-07-07 13:19:05 PDT
When a JSContext is created with a non-null globalObjectClass passed to JSGlobalContextCreateInGroup, that root object is created as an instance of CallbackGlobalObject and not GlobalObject.
The Object graph view only searches for objects that are instances of GlobalObject in the JSContext case so nothing gets populated in this case.
Comment 1 Christopher Reid 2020-07-07 14:12:06 PDT
Created attachment 403723 [details]
patch
Comment 2 Devin Rousso 2020-07-07 19:39:54 PDT
Comment on attachment 403723 [details]
patch

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

> Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:271
> +        this.heapSnapshot.instancesWithClassName("CallbackGlobalObject", (instances) => {

While this is fine, it would be nice to show all roots rather than just a specific few, as these roots may change in the future :)

I believe we could expose a way to iterate `heapSnapshot._nodeOrdinalIsGCRoot`, calling `HeapSnapshot.prototype.serializeNode` on each.
Comment 3 Devin Rousso 2020-07-07 19:40:31 PDT
Comment on attachment 403723 [details]
patch

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

>> Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:271
>> +        this.heapSnapshot.instancesWithClassName("CallbackGlobalObject", (instances) => {
> 
> While this is fine, it would be nice to show all roots rather than just a specific few, as these roots may change in the future :)
> 
> I believe we could expose a way to iterate `heapSnapshot._nodeOrdinalIsGCRoot`, calling `HeapSnapshot.prototype.serializeNode` on each.

this can be done as a FIXME followup bug, or behind a `WI.Setting` btw :)
Comment 4 Christopher Reid 2020-07-08 11:25:13 PDT
(In reply to Devin Rousso from comment #3)
> Comment on attachment 403723 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403723&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:271
> >> +        this.heapSnapshot.instancesWithClassName("CallbackGlobalObject", (instances) => {
> > 
> > While this is fine, it would be nice to show all roots rather than just a specific few, as these roots may change in the future :)
> > 
> > I believe we could expose a way to iterate `heapSnapshot._nodeOrdinalIsGCRoot`, calling `HeapSnapshot.prototype.serializeNode` on each.
> 
> this can be done as a FIXME followup bug, or behind a `WI.Setting` btw :)

Yeah that would be good to have.
I created a followup bug https://webkit.org/b/214092 and will point a FIXME to it when landing.
Comment 5 Christopher Reid 2020-07-08 11:29:45 PDT
Created attachment 403798 [details]
Patch for landing
Comment 6 EWS 2020-07-08 11:51:06 PDT
Committed r264124: <https://trac.webkit.org/changeset/264124>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403798 [details].
Comment 7 Radar WebKit Bug Importer 2020-07-08 11:52:17 PDT
<rdar://problem/65234451>