Layering: add HostObjectDefinePropertyReturnFalse

[domenic]

This allows host environments which need to override the Object.defineProperty behavior, for legacy compatibility, to preserve invariants while avoiding breaking web applications that depend on not-throwing when defining non-configurable, non-writable properties on WindowProxy. This does not alter the behavior of Reflect.defineProperty or [[DefineOwnProperty]].

Closes #672 (from the ES spec side). See also https://bugzilla.mozilla.org/show_bug.cgi?id=1197958.

[allenwb]

Things like this exist to deal with a single specific situation and don't warrant generalization into abstract operations that might be applied in other situations. I think we should avoid adding attractive nuisance hooks to the ES spec. that could be taken as a general invitation for hosts to do unanticipated bizarre things. In such cases some carefully chosen prose is better than adding additional abstract operations.

I think a cleaner way to allow for the specific problem this change is intended to address (see https://github.com/tc39/ecma262/issues/672) to add a new paragraph to https://tc39.github.io/ecma262/#sec-error-handling-and-language-extensions that says something like:

"An implementation my define failure behavior other than throwing TypeError for Object.defineProperty when it's first argument is an implementation provided exotic object and the throwing behavior would cause legacy code to fail."

The HTML spec. does not need to algorithmicallhy "call-back" into the ES spec. All it needs to do is defined the circumstances where it returns false rather than throwings.

[domenic]

Right. As I stated, I think this is a clearer way of doing this, and the normative requirements on hosts (derived from your wording) serve to discourage the general invitation you're worried about.

[bterlson]

I think this is needs-consensus in the vein of #673.

Also it's not purely "layering", so should be marked Normative (HostEnsureCanCompileStrings should have been as well, whoops).

"only for legacy code" prose doesn't seem like a realistic normative requirement, so in practice any ecmascript implementation may adopt this behavior. Language similar to what I propose in https://github.com/tc39/ecma262/issues/668#issuecomment-240795846 is probably better for legacy hacks.

[domenic]

Hmm OK, I guess that's fair; I hadn't quite realized that this is technically a normative change. I'll update the PR and prepare a TC39 presentation for this, #673, and maybe #683 (for which I will also create a PR).

Personally I think "only for legacy code" is a pretty reasonable normative requirement; it's meant to prevent hosts from taking this as a license to do weird things. It's similar to how in Web IDL we've started renaming things "legacyX" to indicate that you shouldn't use them. I think this is a bit nicer than explicitly referencing HTML (even non-normatively). But if you prefer such an explicit reference then that works for me. Nit: I'd prefer not referencing a section number since that just creates something you need to keep updated, so I'd probably say just "HTML" or "HTML's obsolete features section".

[michaelficarra]

This was discussed at the Sept meeting. Conclusion appears to be that this goes in Annex B, mentioning HTML explicitly.

[annevk]

It seems the "needs consensus" label can be removed?

[littledan]

Would anyone like to do this editorial change, to put the new spec text in Annex B as suggested in TC39? It'd be great to get this patch landed and enable the fix to https://github.com/tc39/ecma262/issues/672 .

[domenic]

@annevk @bzbarsky, do you remember whether this is sufficient to fix the violations? I believe we ended up needing to do a different solution (and I believe Firefox implemented a different solution) as this one was not sufficiently web-compatible.

If I recall, the violations in question are:

• https://html.spec.whatwg.org/#windowproxy-getownproperty:willful-violation
• https://html.spec.whatwg.org/#windowproxy-defineownproperty:willful-violation

[domenic]

Digging around Firefox bugs my read is that nobody has yet tried to ship the proposed change (i.e., this ecma262 change + the corresponding HTML spec change which doesn't exist yet). See https://bugzilla.mozilla.org/show_bug.cgi?id=1329324 which hasn't seen any movement.

[annevk]

Yeah, it seems this would require an implementer to test the waters.

[bzbarsky]

Yeah, on the Firefox side I haven't had time to prod this along. I will try to do that now.

[bzbarsky]

It doesn't help that the lack of an actual spec on the ES side means https://bugzilla.mozilla.org/show_bug.cgi?id=1329321 totally didn't implement the infrastructure I actually need for WindowProxy. So I can't even experiment with this yet to see whether it's web-compatible... I'll see what I can do to prod that along.

[bzbarsky]

And in particular, the actual behavior for Window still needs to be sorted out, of course. See https://bugzilla.mozilla.org/show_bug.cgi?id=1178638#c13 and following.

[evilpie]

I wonder if returning null instead of false would be a bit more consistent with APIs like Object.getPrototypeOf that return either an object or null. I guess false was chosen because it matches Reflect.defineProperty?

[bzbarsky]

OK, so I am actually not quite sure what exact behavior we are aiming for here. Specifically, consider this case:

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6 });

Should that second call succeed? What about:

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6, configurable: true });

And what about:

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6 });

and

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6, configurable: true });

Finally, what about:

var foo; // Defines a non-configurable property on the Window
window.foo = 5;

This last one is interesting, because https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-set calls https://tc39.github.io/ecma262/#sec-ordinaryset which calls https://tc39.github.io/ecma262/#sec-ordinarysetwithowndescriptor. And this does a [[DefineOwnProperty]] call which is basically identical to Object.defineProperty(window, "foo", { value: 5 });. And if we force that descriptor to be configurable, then the assignment will fail, which is clearly not web-compatible...

@domenic @annevk

[annevk]

@erights ^^

[allenwb]

OK, so I am actually not quite sure what exact behavior we are aiming for here. Specifically, consider this case:

Object.defineProperty(window, "foo", { value: 5, configurable: false });

note that writeable:false is implicit in the above descriptor (assume "foo" doesn't already exist)

Object.defineProperty(window, "foo", { value: 6 });

Should that second call succeed? What about:

No because, "foo" property is non-writable

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6, configurable: true });

fails because configurable attribute of a property can not be changed from false to true

And what about:

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6 });

Depends upon whether or not window is the global objects. If it is, succeeds because assignment to the global object creates a writable non-configurable property. If it isn't (eg, b/c windowproxy) it depends upon how assignment to the actual global object is propagated to the window object. So, depends upon HTML spec.

and

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6, configurable: true });

Same as above, if window is the global object then would fail because can't change configurable from false to true.

Finally, what about:

var foo; // Defines a non-configurable property on the Window
window.foo = 5;

as above, but basically just demonstrates that windowproxy creates writable properties on window

This last one is interesting, because https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-set calls https://tc39.github.io/ecma262/#sec-ordinaryset which calls https://tc39.github.io/ecma262/#sec-ordinarysetwithowndescriptor. And this does a [[DefineOwnProperty]] call which is basically identical to Object.defineProperty(window, "foo", { value: 5 });. And if we force that descriptor to be configurable, then the assignment will fail, which is clearly not web-compatible...

You need to be careful in talking about [[DefineOwnProperty]] equivalents to remember that incomplete descriptors default missing attributes to false when used to define a new property. For existing properties missing attributes default to " no change".

[bzbarsky]

No because, "foo" property is non-writable

OK, assume both descriptors set writable: true.

fails because configurable attribute of a property can not be changed from false to true

Well, does Object.defineProperty(window, "foo", { value: 5, configurable: false }); actually create a non-configurable property? That is one big open question here! The property it creates is supposed to be reported as configurable via getOwnPropertyDescriptor. The question is how it should behave for defineProperty purposes.

Depends upon whether or not window is the global objects

window is the WindowProxy, of course. There is no direct access to the Window (which is the global object) in the web platform. I'd really appreciate us focusing on the actual context here instead of spending a lot of space on irrelevant hypotheticals; the issue is complicated enough as it is, without confusing it by considering cases that don't exist.

it depends upon how assignment to the actual global object is propagated to the window object

It's not; the propagation is in the opposite directly. The property var defines lives on the global object. The window object forwards some operations to that global.

So, depends upon HTML spec.

To some extent. Part of the question here is what behavior is desired here, or in other words what exactly we want the HTML spec to say.

You need to be careful in talking about [[DefineOwnProperty]] equivalents to remember that incomplete descriptors default missing attributes to false

Yes, I am well aware of this.

Again, to make this clear: We do not want to report properties as being non-configurable on WindowProxy, because they can always get reconfigured during navigation. When someone tries to define a non-configurable property on the WindowProxy, we want to report that we failed to do that, but still define the property on the underlying Window. The open questions are:

1. Should the attempt to define on the underlying Window define the property as configurable or not? Does this depend on whether an explicit configurable attribute was supplied in the descriptor?
2. Does this depend on whether the Window already has a property of that name and on its configurability state?

My understanding of earlier discussion was that we wanted to coerce properties to be defined as configurable on the Window, but that causes problems with the "var followed by assignment" case, because the assignment would throw. So I am trying to figure out what behavior we actually want here that would be least-surprise.

If the answer is just "use the given descriptor as-is, return false (in which cases?), and have getOwnPropertyDescriptor always report configurable even if this does not match the observed behavior of defineProperty in some cases", that's fine. But I want to make sure we're all on the same page for the behavior we're aiming to implement.

[allenwb]

OK, I getting swapped back in and now understand the basis of your questions. window is an exotic object, but still must conform to the essential property invariants. So, cutting to the chase...

If the answer is just "use the given descriptor as-is, return false (in which cases?), and have

You don't have to return false when defining a new property requests configurable:false even if you are going to ignore that request. You can ignore the configurable: false and still return true. The invariant for [[DefineOwnProperty]] only requires returning false return for changing a non-configurable property to configurable.

getOwnPropertyDescriptor always report configurable even if this does not match the observed behavior of defineProperty in some cases", that's fine. But I want to make sure we're all on the same page for the behavior we're aiming to implement.

Yes, that sounds fine to me. The window exotic object internal methods should maintain a shadow copy of the configurable value passed to [[DefineOwnProperty]] and use the shadow to control what is allowed. Except that [[GetOwnPropertyDescriptor]] always reports configurable: false.

Also remember that undefined, NaN, and Infinity are specified as {configurable: false, writable: false} and [[GetOwnPropertyDescriptor]] should report that.

[bzbarsky]

You don't have to return false when defining a new property requests configurable:false even if you are going to ignore that request.

Actually, signaling that the request to create a new non-configurable property was ignored is literally the entire basis for this whole discussion. The people who are relying on the essential property invariants say they need to know when their attempt to define a non-configurable property didn't actually do that.

Also remember that undefined, NaN, and Infinity are specified as {configurable: false, writable: false} and [[GetOwnPropertyDescriptor]] should report that.

Hmm. I guess that would be OK in the sense that their property descriptors do not observably change on navigation (though they can go from returning a property descriptor to throwing on access). But it would somewhat slow down property access if we need to check for those three special cases, and we'd need to figure out whether there are other such special cases as well...

[erights]

There certainly is a lot of context to reload, and it is important to get all the simultaneous constraints right. I admit that my memory of all this is rusty and needs a refresh. Can we arrange a conf call (hangout or something) to discuss this?

[bzbarsky]

I can do that, in general... Whom do we need in the room?

[allenwb]

Actually, signaling that the request to create a new non-configurable property was ignored is literally the entire basis for this whole discussion. The people who are relying on the essential property invariants say they need to know when their attempt to define a non-configurable property didn't actually do that.

But the specification of the essential invariants has never promised that. The promise is that if a property has been observed to be non-configurable (and non-writable) its attributes (including value if non-writable + special exemption for writable to non-writable transactions) will never observably change.

If a caller needs to verify that that setting configurable to true actually worked they can check it via [[GetOwnPropertyDescript]] after the define call. The essential invariants guarantee that if [[GetOwnPropertyDescriptor]] says configurable is false then it can never observably change to true.

Hmm. I guess that would be OK in the sense that their property descriptors do not observably change on navigation (though they can go from returning a property descriptor to throwing on access). But it would somewhat slow down property access if we need to check for those three special cases, and we'd need to figure out whether there are other such special cases as well...

This is currently a requirement of the spec. At least I consider the fact the the spec. says the the attributes of those properties are {writable: false, configurable: false} is an a priori observation of the configurable property so it may never change to configurable: true.

I would also hope that implementations already take advantage of the fact that if they aren't shadowed their constant values can be assumed.

[bzbarsky]

But the specification of the essential invariants has never promised that.

Now we're re-having a discussion from a year or so ago.... Please talk this over with @erights? My understanding was that the whole reason we have a problem here is that what the essential invariants nominally promise is not sufficient for actual uses of defineProperty that are depending on this stuff. But really, I don't care that much about any of this, including the essential invariants. I just want TC39 to decide what it actually wants here, in a way that's unambiguously implementable. We're not there right now and the amount of time I have to spend on this is severely limited...

At least I consider the fact the the spec. says the the attributes of those properties are {writable: false, configurable: false} is an a priori observation of the configurable property

On the global, right? Not on the WindowProxy....

I would also hope that implementations already take advantage of the fact that if they aren't shadowed their constant values can be assumed.

Sure, for barewords. Not so much for window.NaN: that can throw on get, for example, so just converting it to a runtime constant doesn't work.

[bzbarsky]

Sure, for barewords.

In Firefox. I think in some other browsers even barewords go through the WindowProxy in some cases, in inconsistent ways.

[erights]

How about either

• Tuesday Oct 16 from 1pm-3pm PDT, at the weekly Frozen Realms shim collaboration mtg, or
• Thursday Oct 18 from 1pm-3pm PDT, at the weekly SES Strategy mtg?

Both are by hangout. Doodle poll at https://doodle.com/poll/eigqy7aekwnqi3wc

[erights]

Hi @bzbarsky we had a great conversation at the SES meeting on this topic with @warner and @jfparadis . None of us have read this whole thread. But our conversation, and our examination of your https://github.com/tc39/ecma262/pull/688#issuecomment-427417248 message, quoted below, came to a coherent conclusion; or at least it seemed so to us in ignorance of the rest of the thread. Rather than check first, it seems more valuable to capture our conclusions from the conversation while it is still fresh. Apologies if we are only raising points that are redundant or already refuted.

But first some terminology:

• windowProxy. In our answer below, we use windowProxy to indicate the reified global object that JS code in the browser can get its hands on. windowProxy is not an EcmaScript proxy. When a frame is navigated, the same windowProxy is also the global for the fresh JS environment of the new frame.
• hiddenWindow. Note the italics and lack of code font. This is an explanatory internal spec object that JS code can never get a direct reference to. We avoid the conventional term "window" because the top level JS code window in the browser environment evaluates to the windowProxy. I assume that the "Window" in the text of your question consistently refers to this object.
• refuses configuration. If a property reports that it is configurable but acts in all other ways as if it is non-configurable, we say it refuses configuration. We do not say unqualifiedly that it "acts as if it is non-configurable" because that implies that it would also report that is non-configurable. Thus, when you said "non-configurable on Window", I interpret that to mean "configurable, but refuses configuration".

The constraints that our answers hope to meet:

• Preserve the stateful invariants no matter what.
• Compromise only the observable behavior of Object.defineProperty(windowProxy, ...) in the sense that it acts differently than it would on any other object; and no other operation, even defineProperty-like operations, act differently even on windowProxy.
• Make the exception as narrow as possible while still not breaking the legacy code we found we could not break. In particular, the exception is for Object.defineProperty alone, as applied to windowProxy alone.
• Code written naively not taking into account the consequences of frame navigation or of this special case motivated by supporting frame navigation should be minimally disrupted by this special case.
• Don't require any extra bookkeeping beyond what browsers already do. In particular, we cannot require any per-property memory of properties defined in a context that has already been navigated away from.
• Keep simple: the formal semantics, the informal semantics to sophisticated programmers, the informal semantics to less sophisticated programmers or those not thinking about the special case, and the implementation burden.

Even with all these constraints, there are still multiple possible answers to your examples below. Of the possible ways of being "minimally disruptive" of naive code, we decided that a request to make a non-configurable property, though it must fail and must report failure, does also create, if possible, a configurable property that refuses configuration. We explain this as if it creates a non-configurable property on the hiddenWindow, though no one can observe this.

OK, so I am actually not quite sure what exact behavior we are aiming for here. Specifically, consider this case:

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6 });

Should that second call succeed?

• The first call defines a configurable, non-writable property named "foo" with value 5 that refuses configuration.
• The first call must then fail by returning false, since a property with the attributes it asked for was not created. It fails by returning false, rather than by throwing, because in the absence of this special case it would have succeeded and this is the least disruptive difference that does not break legacy code. Since the property refuses configuration, it does little harm to most naive code for it to assume (by not checking the return value) that it got what it asked for. Under all behaviors other than reflective examination, it did get what it asked for.
• The second call must fail with a thrown exception. It clearly must fail in order to preserve the object invariants. But naive code should not be allowed to continue on normal control flow paths that assume success, because even without reflective examination, the naive code did not get what it asked for.

What about:

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6, configurable: true });

• The first call defines a configurable, non-writable property named "foo" with value 5 that refuses configuration.
• The first call must then fail by returning false, for the reasons stated above.
• The second call must fail with a thrown exception, but only because of the value change.

To explain the last bullet above consider the example

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 5, configurable: true });

Here, the second call must succeed, because it did not ask for an attribute setting that conflicts with the current attribute setting. The fact that the first call resulted in a property that refuses configuration is irrelevant, even though the second call, had it come first, would have resulted in a property that does not refuse configuration.

And what about:

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6 });

• The first line succeeds at defining a writable configurable property named "foo" with value 5 that refuses to be configured.
• The second call succeeds at changing this to a writable configurable property named "foo" with value 6 that refuses to be configured. This is allowed because
  • the property is writable, so the value can change
  • the new descriptor didn't specify configurability, so it preserves whatever the previous configurability setting was.

and

 var foo = 5; // Defines a non-configurable property on the Window
 Object.defineProperty(window, "foo", { value: 6, configurable: true });

• The first line succeeds at defining a writable configurable property named "foo" with value 5 that refuses to be configured.
• The second call succeeds at changing this to a writable configurable property named "foo" with value 6 that refuses to be configured. This is allowed because
  • the property is writable, so the value can change
  • the new descriptor's desired configurability setting is the current setting, so it is not asking for it to change.

Finally, what about:

var foo; // Defines a non-configurable property on the Window
window.foo = 5;

This last one is interesting, because https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-set calls https://tc39.github.io/ecma262/#sec-ordinaryset which calls https://tc39.github.io/ecma262/#sec-ordinarysetwithowndescriptor.

• The first line succeeds at defining a writable configurable property named "foo" with value undefined that refuses to be configured.
• The second line succeeds at changing it to a writable configurable property named "foo" with value 5 that refuses to be configured. This succeeds because the property is writable.

And this does a [[DefineOwnProperty]] call which is basically identical to Object.defineProperty(window, "foo", { value: 5 });. And if we force that descriptor to be configurable, then the assignment will fail, which is clearly not web-compatible...

During our conversation, I argued against this, and was expecting to do so again in this summary of our discussion. However, now that I reread this carefully, I realize I don't actually understand what you're trying to say. What do you mean by "basically identical" and "force"?

@domenic @annevk

[bzbarsky]

@erights, I appreciate you having the discussion and the concrete answers to my questions. I think we're getting closer to getting this pinned down...

When a frame is navigated, the same windowProxy is also the global for the fresh JS environment of the new frame.

The global for the JS environment is what you call the hiddenWindow. The windowProxy itself stores no state other than what the current hiddenWindow is; all other state (e.g. the names and values of global variables) is stored on the hiddenWindow. The windowProxy has internal methods that call the internal methods of hiddenWindow in some cases.

Thus, when you said "non-configurable on Window", I interpret that to mean "configurable, but refuses configuration".

That's an interesting question. There is no way to directly query property descriptors from hiddenWindow, so there's no obvious way to tell these apart from script apart from the effect they have on defineProperty-like calls, right?

In practice in browsers, var defines an actual non-configurable property on hiddenWindow.

We explain this as if it creates a non-configurable property on the hiddenWindow, though no one can observe this

Makes sense. That's what browsers already do, good.

The second call must fail with a thrown exception, but only because of the value change.

Hmm. But we were modeling this situation as a non-configurable property on the hiddenWindow. Seems to me that even without a value change this should throw an exception, because it looks like it's trying to redefine from a non-configurable to a configurable property...

Here, the second call must succeed, because it did not ask for an attribute setting that conflicts with the current attribute setting.

[[DefineOwnProperty]] on windowProxy can just decide to throw for any reason it wants, in theory.

But ok, if we posit that we want the behavior you describe, what is the actual behavior of [[DefineOwnProperty]] on windowProxy to accomplish this, given that there is in fact a non-configurable property on the hiddenWindow named "foo"?

Same concern applies to this example:

var foo = 5; // Defines a non-configurable property on the Window
Object.defineProperty(window, "foo", { value: 6, configurable: true });

Let me ask another question, which may tease out what you mean by "refuses to be configured". If I do:

var foo = 5; // Defines a non-configurable property on the Window
Object.defineProperty(window, "foo", { get: function() {}, configurable: true });

what should happen?

What do you mean by "basically identical" and "force"?

"basically identical" means that the way https://tc39.github.io/ecma262/#sec-ordinarysetwithowndescriptor ends up acting in the IsDataDescriptor(ownDesc) case when the ownDesc is that it calls Receiver.[[DefineOwnProperty]] with a descriptor created as { [[Value]]: V }. And in strict mode that ends up with the same behavior as the defineProperty call with that descriptor, afaict.

and "force"

One of the implementation strategies for the [[DefineOwnProperty]] internal method on windowProxy that I considered modifies the value of configurable in the descriptor to true before forwarding to the [[DefineOwnProperty]] internal method of the hiddenWindow. But that runs into problems when the latter has a non-configurable property on it already. This comes back to what exactly you mean by "refuses to be configured" and how you propose windowProxy's [[DefineOwnProperty]] be implemented. Again, all the property storage is on the hiddenWindow.

One thing I want to make sure we're all ok on here. If we only compromise Object.defineProperty then Object.defineProperties, when called on a windowProxy, will keep defining properties until it gets to the first descriptor which has explicit configurable: false (or is missing configurable: true? This depends on how [[DefineOwnProperty]] is defined on windowProxy). Once it gets to such a descriptor, it will define the property as a property that "refuses configuration", then throw, right? At least that's what falls out of putting all the logic in [[DefineOwnProperty]] on windowProxy and Object.defineProperty.

[allenwb]

Object.defineProperty(window, "foo", { value: 5, configurable: false });
Object.defineProperty(window, "foo", { value: 6 });

...

• The first call must then fail by returning false, since a property with the attributes it asked for was not created. It fails by returning false, rather than by throwing, because in the absence of this special case it would have succeeded and this is the least disruptive difference that does not break legacy code. Since the property refuses configuration, it does little harm to most naive code for it to assume (by not checking the return value) that it got what it asked for. Under all behaviors other than reflective examination, it did get what it asked for.

This can't be correct because Object.defineProperty is specified to return its first parameter if it does not fail. eg:

Object.defineProperty(Object.defineProperty(obj, "foo", {value: 1}), "bar", {value: 2});

Returning false would break that use case. It's [[DefineOwnProperty]] that returns true/false to indicate success or failure.

[bzbarsky]

This can't be correct because Object.defineProperty is specified to return its first parameter if it does not fail.

That is one of the things we are proposing changing: that in this specific case it will return false.

Returning false would break that use case.

Yes, it would, for the specific case of obj being a WindowProxy and a non-configurable property being defined. We haven't seen instances of this pattern being used with WindowProxy and non-configurable properties so far.