Bug 208481

Summary: Align garbage collection for XMLHttpRequest objects with the specification
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: BindingsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, darin, esprehn+autocc, ews-watchlist, ggaren, kangil.han, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://xhr.spec.whatwg.org/#garbage-collection
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2020-03-02 15:45:02 PST 
Simplify XHR JS wrapper lifetime management and align with the specification:
- https://xhr.spec.whatwg.org/#garbage-collection
Comment 1 Chris Dumez 2020-03-02 15:48:56 PST 
Created attachment 392202 [details]
Patch
Comment 2 Chris Dumez 2020-03-03 13:08:59 PST 
Created attachment 392318 [details]
Patch
Comment 3 Chris Dumez 2020-03-06 14:05:45 PST 
Created attachment 392770 [details]
Patch
Comment 4 Chris Dumez 2020-03-06 16:32:11 PST 
Created attachment 392802 [details]
Patch
Comment 5 Chris Dumez 2020-03-06 16:47:41 PST 
Created attachment 392809 [details]
Patch
Comment 6 Ryosuke Niwa 2020-03-09 11:12:30 PDT 
Comment on attachment 392809 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        e now override ActiveDOMObject::hasPendingActivity() to match exactly the text

Nit: e -> We?

> Source/WebCore/xml/XMLHttpRequest.cpp:1167
> +        || hasEventListeners(eventNames().loadEvent)
> +        || hasEventListeners(eventNames().timeoutEvent)

Can we order these events in alphabetical/lexicological order?
Comment 7 Ryosuke Niwa 2020-03-09 11:24:06 PDT 
Comment on attachment 392809 [details]
Patch

Oops, I didn't meant to set cq+
Comment 8 Chris Dumez 2020-03-09 12:26:15 PDT 
Created attachment 393061 [details]
Patch
Comment 9 WebKit Commit Bot 2020-03-09 13:17:27 PDT 
Comment on attachment 393061 [details]
Patch

Clearing flags on attachment: 393061

Committed r258159: <https://trac.webkit.org/changeset/258159>
Comment 10 WebKit Commit Bot 2020-03-09 13:17:29 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-03-09 13:23:17 PDT 
<rdar://problem/60239100>
Comment 12 Darin Adler 2020-03-09 13:58:37 PDT 
Comment on attachment 393061 [details]
Patch

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

> Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h:36
>  class EventTarget;

Should have removed this.
Comment 13 Chris Dumez 2020-03-09 14:01:10 PDT 
(In reply to Darin Adler from comment #12)
> Comment on attachment 393061 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393061&action=review
> 
> > Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h:36
> >  class EventTarget;
> 
> Should have removed this.

Indeed, will follow up, thanks.
Comment 14 Chris Dumez 2020-03-09 14:56:01 PDT 
(In reply to Chris Dumez from comment #13)
> (In reply to Darin Adler from comment #12)
> > Comment on attachment 393061 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=393061&action=review
> > 
> > > Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h:36
> > >  class EventTarget;
> > 
> > Should have removed this.
> 
> Indeed, will follow up, thanks.

<https://trac.webkit.org/changeset/258165>