Bug 240202

Summary: Explain how node reference counting works in Introduction.md
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Tools / TestsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ashvayka, cdumez, darin, ggaren, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Added
none
Updated
none
Updated
none
Patch for landing none

Ryosuke Niwa
Reported 2022-05-06 23:24:45 PDT
Explain how node reference counting works.
Attachments
Added (7.74 KB, patch)
2022-05-06 23:27 PDT, Ryosuke Niwa
no flags
Updated (8.41 KB, patch)
2022-05-07 00:08 PDT, Ryosuke Niwa
no flags
Updated (8.41 KB, patch)
2022-05-07 00:10 PDT, Ryosuke Niwa
no flags
Patch for landing (18.99 KB, patch)
2022-05-07 21:38 PDT, Ryosuke Niwa
no flags
Ryosuke Niwa
Comment 1 2022-05-06 23:27:15 PDT
Ryosuke Niwa
Comment 2 2022-05-06 23:34:44 PDT
Comment on attachment 458990 [details] Added View in context: https://bugs.webkit.org/attachment.cgi?id=458990&action=review > Introduction.md:1054 > +It's equally crucial to observe that keeping C++ Node object alive by storing `Ref` or `RefPtr` This section may need to be revised once I wrote more about active DOM objects but it's good enough for now IMO.
Chris Dumez
Comment 3 2022-05-06 23:54:03 PDT
Comment on attachment 458990 [details] Added View in context: https://bugs.webkit.org/attachment.cgi?id=458990&action=review Nice. >> Introduction.md:1054 >> +It's equally crucial to observe that keeping C++ Node object alive by storing `Ref` or `RefPtr` > > This section may need to be revised once I wrote more about active DOM objects but it's good enough for now IMO. Seems like we should mention GCReachableRef as I believe this is a pretty common way to keep a JS Node wrapper alive? Note that AFAIK, there is not much usage of ActiveDOMObject for Node types: Source/WebCore/html/HTMLCanvasElement.idl: Source/WebCore/html/HTMLImageElement.idl: Source/WebCore/html/HTMLMarqueeElement.idl: Source/WebCore/html/HTMLMediaElement.idl: Source/WebCore/html/HTMLSourceElement.idl: Source/WebCore/html/HTMLTrackElement.idl: Are these really the only ones that can fire events asynchronously? Here is one example: ``` void HTMLTextFormControlElement::scheduleSelectEvent() { document().eventLoop().queueTask(TaskSource::UserInteraction, [protectedThis = GCReachableRef { *this }] { protectedThis->dispatchEvent(Event::create(eventNames().selectEvent, Event::CanBubble::Yes, Event::IsCancelable::No)); }); } ``` HTMLTextFormControlElement isn't an ActiveDOMObject but seems to rely on GCReachableRef instead. > Introduction.md:1071 > +The referencing node count of a document is the total number of Node's whose `ownerDocument` is the document. Node's -> Nodes ?
Ryosuke Niwa
Comment 4 2022-05-07 00:05:24 PDT
(In reply to Chris Dumez from comment #3) > Comment on attachment 458990 [details] > Added > > View in context: > https://bugs.webkit.org/attachment.cgi?id=458990&action=review > > Nice. > > >> Introduction.md:1054 > >> +It's equally crucial to observe that keeping C++ Node object alive by storing `Ref` or `RefPtr` > > > > This section may need to be revised once I wrote more about active DOM objects but it's good enough for now IMO. > > Seems like we should mention GCReachableRef as I believe this is a pretty > common way to keep a JS Node wrapper alive? Oh, that's a good point. Totally forgot about that.
Ryosuke Niwa
Comment 5 2022-05-07 00:08:35 PDT
Ryosuke Niwa
Comment 6 2022-05-07 00:10:24 PDT
Chris Dumez
Comment 7 2022-05-07 10:38:21 PDT
Comment on attachment 458992 [details] Updated r=me
Ryosuke Niwa
Comment 8 2022-05-07 10:58:55 PDT
Comment on attachment 458992 [details] Updated Clearing flags on attachment: 458992 Committed r293949 (250395@trunk): <https://commits.webkit.org/250395@trunk>
Ryosuke Niwa
Comment 9 2022-05-07 10:58:58 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2022-05-07 10:59:15 PDT
Ryosuke Niwa
Comment 11 2022-05-07 21:38:49 PDT
Reopening to attach new patch.
Ryosuke Niwa
Comment 12 2022-05-07 21:38:52 PDT
Created attachment 459005 [details] Patch for landing
Ryosuke Niwa
Comment 13 2022-05-07 21:39:18 PDT
Comment on attachment 459005 [details] Patch for landing Wrong bug.
Note You need to log in before you can comment on or make changes to this bug.