fetch() is unclear on which realm's Request it uses

[domenic]

https://fetch.spec.whatwg.org/commit-snapshots/e6cbef27724dd6111d1646898ef4f3f9ad56740b/#fetch-method

Let requestObject be the result of invoking the initial value of Request as constructor with input and init as arguments. If this throws an exception, reject p with it and return p

Which realm does the Request constructor come from?

I think we want it to be the relevant realm of this WindowOrWorkerGlobalScope object. That seems most in line with our plan in https://github.com/heycam/webidl/issues/135, although that is about slightly different phrasing ("a new Response" vs. this line's "invoking the initial value of Request as constructor").

BTW, editorial issues with this sentence:

• "as constructor" -> "as a constructor"
• Missing period
• "Request" links to the class, but should probably link to the constructor.

Found by @benjamingr and @TimothyGu in IRC: https://freenode.logbot.info/whatwg/20180712#c1621394

(BTW, @TimothyGu found another good thing to test: what if you create a Request from another window, and pass it to fetch()? What client does it use? Some inspection of Chrome's source code reveals it may not be using the other window, but that's what the spec says should happen.)

[TimothyGu]

(For those who don't want to read IRC logs, all of this is observable through the Referer header and using the fetch() function from an iframe for example.)

We probably don't want to use the word "invoking" here at all, since it can mean multiple things: the ES Call abstract operation; invoke a callback function algorithm (this is probably the closest, since it contains logic to convert IDL arguments to back ES, even though Request is not a callback function in the IDL sense); or just "execute the steps listed for that constructor"?

I think we want it to be the relevant realm of this WindowOrWorkerGlobalScope object.

This would cause a slight discrepancy between fetch() and Request(). The Request() constructor explicitly uses the current settings object as the client of the new request it's creating; i.e., the realm for which the Request function was created.

Edit: Hmm, I guess this discrepancy is fine, since if we pass a Request object to fetch() it'll recreate the request anyway, so a circumstance where fetch.call(otherWindow, "url") and fetch.call(otherWindow, new Request("url")) are different should never happen.

I think we would want more testing before deciding if we want toshould use the relevant realm or the current realm, but philosophically the question really is "do we want to make fetch() in a realm bound to that realm?"

[domenic]

I mean, it has to be bound to some realm. And we generally want to pick the relevant realm of this, for methods. (For constructors the "relevant realm of this" is not super well-defined, since the this object is still being created, and is derived from the realm of new.target, which is usually-but-not-with-Reflect.construct the current realm.)

[TimothyGu]

Right, I see.

This kind of spawns another issue though. Currently in Chrome and Firefox fetch.call(otherWindow, 'url') actually returns an instance of otherWindow.Promise that resolves to an instance of otherWindow.Response, even though the fetch belongs to the current window, but not in Safari. I feel like Chrome and Firefox’s behavior is quite unexpected.

[domenic]

Chrome and Firefox's behavior is the desired one. This is a clear-cut instance of https://github.com/heycam/webidl/issues/135 (triggered by "let p be a new promise").

More information on why at https://html.spec.whatwg.org/#realms-settings-objects-global-objects, starting with "In general, web platform specifications should use..."

[annevk]

@jakearchibald @wanderview and I discovered that implementations might not follow this for the Request constructor steps either, which typically say to use the "current settings object". Instead, if you do something like frame.contentWindow.fetch(...), the service worker of frame ends up being used, rather than of the parent which did the invocation. So maybe most of these need to be changed to "relevant".

[annevk]

@rwlbuis this might be another interesting WebKit bug. I put some initial tests at https://github.com/web-platform-tests/wpt/pull/24601. I need #1054 merged before working on specification changes though.

[rwlbuis]

@rwlbuis this might be another interesting WebKit bug. I put some initial tests at web-platform-tests/wpt#24601. I need #1054 merged before working on specification changes though.

Yes it may well be. Are you going to file bugs for the implementations?

[annevk]

Yeah, before committing the change to Fetch bugs will be filed.

[annevk]

@domenic @TimothyGu does this mean https://infra.spec.whatwg.org/#parse-json-from-bytes needs to accept a realm?

[annevk]

See also #400, #730, #731, #825, and #826.

Some other tests (already landed): https://github.com/web-platform-tests/wpt/pull/13850.

[domenic]

@domenic @TimothyGu does this mean https://infra.spec.whatwg.org/#parse-json-from-bytes needs to accept a realm?

I suppose so. It currently always uses the current Realm, because that's what ECMAScript always does. But if we want the web platform to continue its inconsistency with ECMAScript (which was reaffirmed semi-recently in https://github.com/heycam/webidl/issues/135#issuecomment-411109348), then we'll need to make that overridable.