Review naming and conventions for integration into the HTML standard

[annevk]

Before I attempt writing the corresponding HTML, let's get it reviewed first by @domenic.

[annevk]

One thing I worry about is the names of the abstract operations. They need to be globally unique as I understand it. Should we prefix them with WindowProxyAndLocation or some such? (If we do that some of the CrossOrigin can go away of course.)

[domenic]

(Disclaimer: still planning to do a more full review Monday, so this is just about your above question.)

Looking through all the names, I think they are almost all unlikely to have collisions. CrossOrigin is a sufficient prefix.

The two exceptions:

• StandardDefined should maybe become IsLocationStandardDefinedPropertyKey. You could trim some words to make it less verbose but I guess throwing in "Location" at least is good. And it's only used once so verbose might be fine.
• DefaultInternalMethod we should upstream to ES with a pull request.

Things I also noticed:

• Editorially there seems to be convergence on AbstractOp ( foo, bar ) (note the spaces)
• [[Enumerate]] might be going away per last TC39. We'll see. Keep it for now.
• I think shared operations are currently nested under WindowProxy. Since they are shared they should be in some section that is a sibling to both WindowProxy and Location. Maybe "Exotic cross-origin object infrastructure" or something, with a small intro paragraph explaining that this is the shared infrastructure.

[annevk]

IsSameOrigin would also be an exception I think? IsWindowProxyOrLocationSameOrigin? I wonder if this is the same same-origin check we want to use elsewhere, I suspect it might be in which case IsPlatformObjectSameOrigin might be okay.

I thought that in ECMAScript the abstract operation was nested under the first occurrence, but maybe that is not always the case. Happy to introduce a new section for this using that heading.

[domenic]

Yeah I think the nesting is only when they're used solely by their parent. Agreed on IsSameOrigin probably.

[domenic]

Review finished! Definitely will be important to clean up the organization, as discussed above. But naming and conventions mostly seem good.

[annevk]

Here is a write up of the setup that might help with review as discussion on IRC. I inlined the issues I think the current text has.

The rough idea is as follows. Let IDL define Location and Window, including their prototype chain. We define WindowProxy in its entirety.

Issue: What internal methods does IDL need to allocate a Location platform object that we cannot override until after allocation? E.g., does IDL need [[SetPrototypeOf]] or does it simply set [[Prototype]] directly?

All the properties IDL defines for Location need to appear non-configurable (see IsLocationDefaultProperty). We cannot actually make them non-configurable because we change what is exposed in the cross-origin case.

Issues: We only make own properties appear non-configurable and there are no own properties left once we remove [Unforgeable]. However, since the prototype object is not exposed cross-origin, perhaps we only need to make the own properties exposed cross-origin appear non-configurable? (If that is true, we can remove [[defaultProperties]] and change IsLocationDefaultProperty to only work with those.)

Although that is also true for Window, https://bugzilla.mozilla.org/show_bug.cgi?id=1197958#c4 prevents us from solving that at the moment. So for Window we violate some internal method invariants, on purpose.

For both Location and WindowProxy, in the cross-origin case, properties returned from CrossOriginProperties need to appear as if they are own properties, but their actual behavior might need to be found on the prototype chain. So we need to bypass [[GetPrototypeOf]] overrides here somehow. Probably through a simple flag. Whenever this happens we store the result in [[crossOriginPropertyDescriptorMap]].

Issue: We only use [[GetOwnProperty]] at the moment and don't walk the prototype chain at all.

The other concern I have is that we call an internal method (or its default equivalent) and that in turn ends up invoking an internal method we defined without that give the desired effect. I guess that requires checking them all carefully. I expect we need some kind of flag (also for the [[GetPrototypeOf]] mentioned above) to bypass our methods and invoke the default ones until that flag is unset again. @bzbarsky mentioned this during a meeting I think.

[bzbarsky]

E.g., does IDL need [[SetPrototypeOf]] or does it simply set [[Prototype]] directly?

I think a good working assumption is that IDL basically does the equivalent of ES6 Object.create (or indeed simply calls it; since the IDL stuff "runs" before any page scripts, we don't even have to worry about calling the initial value). This means that [[Prototype]] is set directly (via ObjectCreate) and then ObjectDefineProperties ends up using the [[DefineOwnProperty]] internal method of the Location or Window object. Note that [[DefineOwnProperty]] internally calls[[GetOwnProperty]]and we'd probably want both to behave "normally" until theObject.create` call returns.

and there are no own properties left once we remove [Unforgeable].

Er, why not? The Location properties all need to be own. Otherwise there's no point in making them appear, or be, non-configurable: non-configurable stuff on the proto can just be shadowed by an own property.

This is why IDL properties annotated with [Unforgeable] become own properties -- it makes absolutely no sense to leave them on the prototype.

but their actual behavior might need to be found on the prototype chain.

I don't see why, for either Location or Window. At least modulo whatever happens in the cross-origin case for named frames...

Issue: We only use [[GetOwnProperty]] at the moment and don't walk the prototype chain at all.

In the same-origin case, we need to do it for things that are not defined on the actual Window interface (e.g. addEventListener).

[annevk]

@bzbarsky okay, so if all Location properties all need to be own, IDL will need some special annotation for that, right? That would certainly simplify things.

[bzbarsky]

That annotation is [[Unforgeable]]. Why would we not use it here, since it does exactly what we want: make them non-configurable (in the actual storage; we can lie about it in [[GetOwnProperty]]) and own props?

[annevk]

Yeah, my bad. I never grasped we just wanted to move [[Unforgeable]] from the interface to all the members. This is now clear in the draft document. I should probably be able to sort out the remainder myself. Will let you know when I get stuck again.

[domenic]

Issue: We only use [[GetOwnProperty]] at the moment and don't walk the prototype chain at all.

This is specifically an issue for the cross-origin [[HasProperty]], [[Get]], and [[Set]]. (Plus [[Enumerate]], but that is going away.) The same-origin case is taken care of by the DefaultInternalMethod.

The concrete scenarios we need to handle here are stuff like:

crossOriginWindow.toString;
crossOriginWindow.toString = "foo";
'toString' in crossOriginWindow;

// same for crossOriginLocation
// same for EventTarget methods and properties

I guess the question is: should these work? I assume for web-compat they need to. Since [[GetPrototypeOf]] returns null, the idea is to make cross-origin windows/locations appear as if their prototype chains are all flattened, so they have own-properties corresponding to everything on Object.prototype (and EventTarget.prototype, for WindowProxy)?

If so, it might be simplest to just add extra initialization steps that copy over the methods, similar to what you've done for @@toStringTag.

The other concern I have is that we call an internal method (or its default equivalent) and that in turn ends up invoking an internal method we defined without that give the desired effect. I guess that requires checking them all carefully. I expect we need some kind of flag (also for the [[GetPrototypeOf]] mentioned above) to bypass our methods and invoke the default ones until that flag is unset again. @bzbarsky mentioned this during a meeting I think.

My intuition is that no such flag will be needed, as the internal methods are reasonably well-factored already. You don't want one of them to accidentally punch a hole through the security layer by calling the original, non-censored [[GetOwnProperty]] or something.

[annevk]

toString doesn't have to work apparently. It throws today. So actually I think we might be all good, apart from some refactoring here and there.

[annevk]

Closing this since I don't think there's anything left. The only thing that needs cleaning up is [[GetOwnProperty]] for Window and that is #32.