Constructor & prototype reform

[pmdartus]

Context

Currently, the constructor and prototype chains are shared between all the realms. This approach has multiple downstream impacts:

• on the maintainer side, it requires hacks to implements interfaces that need a reference to the current global object, eg: [HTMLConstructor]. WebIDL2JS also had to implement the [WebIDL2JSFactory] IDL extended attribute to generate wrapper class per window object.
• on the consumer side, it is unexpected for developers that all constructors and prototypes are shared, especially when they use a test framework abstracting jsdom. I personally spent a lot of time figuring out the best way to apply polyfill in the context of jsdom.

While recreating a brand new wrapper class for each new realm won't scale, I think there might be other alternatives worth exploring.

Approach 1: New constructor / Same prototype

This approach consists of recreating a brand new constructor per realm and reuse the same prototype. WebIDL2JS would generate for all the interfaces annotated with the [Exposed] IDL extended attribute would an install method that would be in charge of creating the interface in the context of the realm.

class TestGlobal extends BaseGlobal { /* ... */ }

const iface = {
    /* ... */

    install(globalObject, privateData) {
        const Test = function() {
            return iface.setup(Object.create(TestGlobal.prototype), [], privateData);
        };
        // Apply static properties if necessary

        Object.setPrototypeOf(Test, TestGlobal.prototype);

        Object.defineProperty(globalObject, "Test", {
            configurable: true,
            overridable: true,
            value: Test
        });

        // Apply [NamedConstructor] if necessary
    }
}
module.export = iface;

You can find here an experimental branch with the code change: pmdartus/constructor

Pros:

• Really light-weight approach.
• No need for [WebIDL2JSFactory] anymore.
• The new install method can be used to implement [NamedConstructor].
• Minimal change needed on jsdom

Cons:

• It doesn't solve the polyfill issue.
• With this approach the only the (new Test()).constructor points to TestGlobal instead of Test, since the prototype is shared. When running the experimental WebIDL2JS against jsdom test, this issue made 5 tests failed.

Approach 2: New constructor / New prototype

In this approach, a brand new constructor and prototype chain is created per realm. Instead of creating a brand new wrapper class per realm, like [WebIDL2JSFactory], we would instead only create a new prototype chain but reuse the existing property descriptors. It also reuses the concept of install showed in proposal 1.

The assumption here is that the performance cost of the creation of a prototype chain while reusing the existing descriptors is way lower than re-creating a brand new class per realm.

class TestGlobal extends BaseGlobal { /* ... */ }

const iface = {
    /* ... */

    install(globalObject, privateData) {
        const Test = function() {
            return iface.setup(Object.create(TestGlobal.prototype), [], privateData);
        };
        // Apply static properties if necessary

        const proto = Object.create(globalObject.Base.prototype);
        Object.defineProperties(proto, Object.getOwnProperties(TestGlobal.prototype));

        Object.setPrototypeOf(Test, proto);

        Object.defineProperty(globalObject, "Test", {
            configurable: true,
            overridable: true,
            value: Test
        });

        // Apply [NamedConstructor] if necessary
    }
}
module.export = iface;

Pros:

• No need for [WebIDL2JSFactory] anymore.
• The new install method can be used to implement [NamedConstructor].
• Solves the polyfill issue.
• Fully spec compliant.

Cons:

• Heavier solution. Would need to do a POC to evaluate the actual performance penalty of this approach.
• Requires changes to jsdom. Now that we have a brand new prototype per realm, we need to pass the global object for every create() and createImpl() invocations, for it to retrieve the right prototype associated with the realm.

---

Before diving deeper into this, I would like getting some initial feedback on both approaches and see if it worth pursuing.

/cc @domenic @TimothyGu @Sebmaster @Zirro

[TimothyGu]

I've personally done some testing with the heaviest-weight solution, which is reconstructing the entire set of wrapper classes for each realm. As you have predicted, it didn't quite scale too well even after enabling compile cache support through VM module, both in terms of memory and in processing time. (It did work though.)

I'd like to raise an additional potential goal for this. We are currently failing many WPTs since the TypeErrors we throw are in fact from the root realm / global context, so instanceof TypeError fails. Not saying we have to get this fixed but it would seem worthwhile.

If possible, I'd like to approach this iteratively, like finishing the first approach first and building the second on top of that work. The removal of WebIDL2JSFactory ext attr is enough of a win to me.

I'd refrain from saying it is fully spec-compliant however, since based on what I'm reading Document.prototype.getElementById instanceof Function would still be false.

[pmdartus]

Thanks for the feedback @TimothyGu. I wrote a proof-concept of the 2 approaches in an iterable fashion to flush out implementation uncertainties about those proposals.

Approach 1: New constructor / Same prototype

Code changes

• webidl2js
• jsdom

Observations

As expected more than 100 tests start failing. There are 2 types of test failures:

• the expected constructor issue: document.createElement('div').constructor === HTMLDivElement
• the removal of [WebIDL2JSFactory] break almost all the websocket test, since one of the tests is mutating the prototype and making all the subsequent tests fails.

In light of those test failures, I don't think this approach is viable.

Approach 2: New constructor / New prototype

Code changes

• webidl2js
• jsdom

Observations

The approach 2 changes are layered on top of the approach 1 changes. Making this approach works in JSDOM requires a lot of changes (needed to touch more than 50 files). However, 100% of the WPT is now passing.

Here is a couple of interesting finding regarding this proof-of-concept:

• wrapper.create and wrapper.createImpl need take the global object as parameter. Those methods require access to the global object to known which constructor needs to be associated with the created wrapper object (code). The downstream effect is that now all the impl classes in jsdom need to be aware of the relevant global object.
• the installation order to the interfaces is important. For instance: Node needs to be installed on the global object before Element. This is because of the creation of the Element.prototype requires Node.prototype. I added an assertion in the generated code to ease the debugging.

One of the areas of uncertainty was the performance impact of such change. This approach increases the interface creation time by 15% and increases the window object creation time by 75% (raw data). IMO, this is a fair price to pay to get isolation between realms.

[TimothyGu]

Glad to hear about the progress! I think the performance decrease is fine – certainly much better than my full isolation approach. The 100% passing WPT with phase 2 is especially encouraging.

[domenic]

Thank you so much for looking in to this @pmdartus. This is truly heroic work on the foundational issues of jsdom that have been with us since the beginning, and it's so exciting and encouraging that you've been tackling them.

I tend to agree new constructor / same prototype isn't really viable, and we should proceed with new constructor / new prototype.

The remaining question is whether we want to preserve any mode of same constructor / same prototype, as in jsdom today. I guess such a mode would not have custom elements support, in exchange for faster operation.

My initial gut feeling is that we should not preserve such a mode. We should just make the next major jsdom release have new constructors / new prototypes everywhere. Here are the contributing factors:

• The complexity of forever supporting this branching structure in jsdom will probably slow down development. As a volunteer project, that kind of friction to contribution is especially dangerous.
• We currently are inconsistent anyway, with the [WebIDL2JSFactory] classes behaving one way and the rest behaving another way. Increasing consistency will be better for users.
• It's unclear how these modes would continue in the future. Would we draw the line at custom elements? Would we move all the current [WebIDL2JSFactory] things into the "isolated mode"? Named constructors? Or would we just kind of make a judgment call like we do today---this multi-constructor-requiring feature seems small, so let's keep it even in "shared mode". None of these seem great.
• It is probably the case that the slowdown of (require jsdom + create a Window) will be small, and less than the 75% you quote for just the create a Window steps, because the changes probably shift work from require-time to Window-creation time. Increasing the total is more worrying to me than increasing the Window-creation time, especially because the total represents the workflow of "I just want to use jsdom in my little script".

I know in jsdom/jsdom#2548 we were discussing a flag in order to land custom elements sooner. Given that I'd prefer not to do a release of jsdom with such a flag, what do you think the sequencing should be? Should we review + land custom elements first, then do constructor reform? Or should we review and land constructor reform first, and do custom elements on top of it?

[pmdartus]

Based on the encouraging result of the constructor reform investigation, I think the most reasonable approach is to make it land before the custom elements. I will try to create a draft PR soon for us to discuss the new webidl2js API shape.