whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.1k stars 2.66k forks source link

Chrome/Safari handles bareword calls to Window methods differently than Firefox #3997

Open TimothyGu opened 6 years ago

TimothyGu commented 6 years ago
<!-- a.html -->
<iframe id=f src=b1.html></iframe>
<!-- <iframe id=f src=b2.html></iframe> -->
<script>
const frame = document.getElementById("f");
frame.onload = () => {
  const { addEventListenerToFirstWindow } = frame.contentWindow;
  frame.onload = () => {
    addEventListenerToFirstWindow("x", () => { console.log("sadness"); });
    frame.contentWindow.dispatchEvent(new Event("x"));
  };
  frame.src = "c.html";
};
</script>

<!-- b1.html -->
<script>
function addEventListenerToFirstWindow(...args) {
  return addEventListener(...args);
}
</script>
<!-- b2.html -->
<script>
"use strict";
function addEventListenerToFirstWindow(...args) {
  return addEventListener.apply(undefined, args);
}
</script>
<!-- b3.html -->
<script>
function addEventListenerToFirstWindow(...args) {
  return addEventListener.apply(this, args);
}
</script>

<!-- c.html -->

Here's the matrix of things browsers do:

Which b.html Chrome/Safari Firefox Edge
b1.html sadness Can't execute code from a freed script
b2.html sadness Can't execute code from a freed script
b3.html sadness sadness Can't execute code from a freed script

For both b1.html and b2.html, in Chrome and Safari, "sadness" is printed, which suggests that the bareword call is made to the WindowProxy (which is then delegated to c.html's Window object). However, in Firefox, "sadness" is not printed, which suggests that Gecko uses the Window object of that frame rather than the WindowProxy.

For b3.html, I explicitly set the this value to this (i.e., the global this value, the WindowProxy), and we see that the event listener is added to the WindowProxy's current Window.

The spec'd behavior, however, is trickier.

In the case of b1.html, CallExpression's Evaluate runtime semantic is called, which through EvaluateCall eventually ends up calling addEventListener.[[Call]] with thisArgument being undefined. Built-in function object's [[Call]] then forwards the thisArgument unchanged to the host-defined steps, which in our case is Web IDL. And per Web IDL rules, specifically step 2.1.2.1 of create an operation function, the undefined is converted to the global object of the realm – the original Window object, which implements the EventTarget interface that defines the addEventListener operation. As such, "sadness" should not be printed for b1.html.

In the case of b2.html, Function.prototype.apply() calls addEventListener.[[Call]] directly with the provided undefined as thisArgument. Per the reasoning of b1.html, "sadness" should not be printed for b2.html either.

For b3.html, addEventListener.[[Call]] is called with the WindowProxy object as the thisArgument. It seems to be underspecified what it should then do as the WindowProxy object doesn't implement the EventTarget interface, but assuming the WindowProxy object is converted using the [[Window]] internal slot to the Window object, it would result in the current (c.html's) Window object. And indeed that is what all three browsers do.

It seems like Firefox conforms to the spec right now, but not Chrome or Safari. In particular, in Chrome / V8, there is an explicit step that converts the call receiver from the global object to the global proxy, citing the fact that we don't ever want to expose the global object as opposed to the global proxy: https://cs.chromium.org/chromium/src/v8/src/execution.cc?l=184-190&rcl=ac0bb41b7ceed029d067c91205599d96ea90f94d However, this seems to be one of the cases where we actually do want the global object. /cc @bmeurer

domenic commented 6 years ago

/cc @bzbarsky as the one who originally taught me about this distinction.

In spec land it seems this means that the global object we supply when creating a JS realm might actually be the WindowProxy?

We may also want to cross-post this to tc39/ecma262 once we figure out our story here.

TimothyGu commented 6 years ago

In spec land it seems this means that the global object we supply when creating a JS realm might actually be the WindowProxy?

I think so, based on the way Chrome behaves. Nope. The property resolution still works in Chrome per spec.

TimothyGu commented 6 years ago

Updated the OP with a more comprehensive test as well as an analysis on what the spec does.

bzbarsky commented 6 years ago

What Firefox does is that when doing a [[Call]] it will replace the global with the WindowProxy for the this value unless the call is to a WebIDL-defined getter/setter/method. It sounds like Chrome is doing the same but across the board?

Basically, we wanted these two scripts to behave identically:

function f() {
  return document;
}

and

var doc = document;
function f() {
  return doc;
}

when f is called after navigation. One of the drivers for that decision was that it's really nice to avoid the security checks WindowProxy always has to do on bareword gets of things like "window", "document", "performance", etc... The other was that this way WebIDL properties on Window act more like data properties (since they are mostly on the object itself, not a prototype, etc).

Implementing the Chrome behavior is doable (we'd basically remove the WebIDL-function special-case).

TimothyGu commented 6 years ago

What Firefox does is that when doing a [[Call]] it will replace the global with the WindowProxy for the this value unless the call is to a WebIDL-defined getter/setter/method. It sounds like Chrome is doing the same but across the board?

Yeah, that seems like the case without getting too deep into the code. I also conjecture that there is a piece of code somewhere in Chromium for Web IDL calls that eventually unwraps WindowProxy down to the original Window but I'm not sure how that works.

bzbarsky commented 6 years ago

Right, you need that no matter what, because code can do window.document and then the this for the getter will be the WindowProxy.