Bug 214054

Summary: Heap Snapshot Object Graph view doesn't get populated in some cases when inspecting a JSContext
Product: WebKit Reporter: Christopher Reid <chris.reid>
Component: Web InspectorAssignee: Christopher Reid <chris.reid>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, joepeck, ross.kirsling, toyoshim, webkit-bug-importer, yoshiaki.jitsukawa, yutak
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=214092
Attachments:
Description Flags
patch
none
Patch for landing none

Christopher Reid
Reported 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.
Attachments
patch (1.91 KB, patch)
2020-07-07 14:12 PDT, Christopher Reid
no flags
Patch for landing (2.47 KB, patch)
2020-07-08 11:29 PDT, Christopher Reid
no flags
Christopher Reid
Comment 1 2020-07-07 14:12:06 PDT
Devin Rousso
Comment 2 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.
Devin Rousso
Comment 3 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 :)
Christopher Reid
Comment 4 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.
Christopher Reid
Comment 5 2020-07-08 11:29:45 PDT
Created attachment 403798 [details] Patch for landing
EWS
Comment 6 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].
Radar WebKit Bug Importer
Comment 7 2020-07-08 11:52:17 PDT
Note You need to log in before you can comment on or make changes to this bug.