Bug 240202

Created attachment 458990 [details]
Added

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.

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 ?

(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.

Created attachment 458991 [details]
Updated

Created attachment 458992 [details]
Updated

Comment on attachment 458992 [details]
Updated

r=me

Comment on attachment 458992 [details]
Updated

Clearing flags on attachment: 458992

Committed r293949 (250395@trunk): <https://commits.webkit.org/250395@trunk>

All reviewed patches have been landed.  Closing bug.

<rdar://problem/92910315>

Reopening to attach new patch.

Created attachment 459005 [details]
Patch for landing

Comment on attachment 459005 [details]
Patch for landing

Wrong bug.