| Summary: | FetchResponse::clone should use the relevant realm for the cloned response | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||
| Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, ashvayka, cdumez, esprehn+autocc, ews-watchlist, kondapallykalyan, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=230941 | ||||||
| Attachments: |
|
||||||
|
Description
youenn fablet
2021-12-13 05:12:46 PST
Created attachment 446991 [details]
Patch
Comment on attachment 446991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446991&action=review This is marvelous! I was looking into Fetch stuff during https://bugs.webkit.org/show_bug.cgi?id=230941 yet didn't come up with a test case. With that said, are there any fetch methods we can fix in the same way? Namely the SEC FetchResponse's error() / redirect() are using to create another response? > LayoutTests/http/wpt/fetch/clone-realm.html:1 > +<!doctype html> There is no way we can WPT export this test, right? > With that said, are there any fetch methods we can fix in the same way? > Namely the SEC FetchResponse's error() / redirect() are using to create > another response? These are static methods and I am not sure there are some observable behavior so maybe it is just fine > > LayoutTests/http/wpt/fetch/clone-realm.html:1 > > +<!doctype html> > > There is no way we can WPT export this test, right? We could but, as written it only passes in Chrome and Safari, not Firefox. (In reply to youenn fablet from comment #4) > > With that said, are there any fetch methods we can fix in the same way? > > Namely the SEC FetchResponse's error() / redirect() are using to create > > another response? > > These are static methods and I am not sure there are some observable > behavior so maybe it is just fine Oh, I've totally missed that. It's a bit weird that spec uses "this's relevant Realm" prose for non-platform objects. In https://bugs.webkit.org/show_bug.cgi?id=230941 I've ignored both static methods and constructors regarding _current_ vs _relevant_. I guess you are right and there might be some weird test case involving Object.setPrototypeOf() for constructors. Not expecting anyone to investigate it though. > > > > LayoutTests/http/wpt/fetch/clone-realm.html:1 > > > +<!doctype html> > > > > There is no way we can WPT export this test, right? > > We could but, as written it only passes in Chrome and Safari, not Firefox. The test looks solid, and the spec normatively requires _relevant_ realm. I would say Firefox's failure is +1 reason to export. > > > > LayoutTests/http/wpt/fetch/clone-realm.html:1
> > > > +<!doctype html>
> > >
> > > There is no way we can WPT export this test, right?
> >
> > We could but, as written it only passes in Chrome and Safari, not Firefox.
>
> The test looks solid, and the spec normatively requires _relevant_ realm. I
> would say Firefox's failure is +1 reason to export.
I'll check again but I think Firefox is generating a valid Response, whose body is in a somewhat special state, calling text() will return a promise that never resolves for instance.
Let's land this one for now and I'll follow up with a WPT proper upstream with WPT review.
Committed r286970 (245192@main): <https://commits.webkit.org/245192@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446991 [details]. Digging further, Chrome is not always throwing when cloning in detached contexts. This might create a backward compatibility risk so we might want to reduce our throwing as well. |