Introduce a new type MaybeUnreflectedDom<T> (fixes #25701)

[nox]

No description provided.

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/dom/bindings/codegen/CodegenRust.py, components/script/lib.rs, components/script/dom/windowproxy.rs, components/script/dom/bindings/reflector.rs, components/script/dom/bindings/root.rs and 2 more
• @KiChjang: components/script/dom/bindings/codegen/CodegenRust.py, components/script/lib.rs, components/script/dom/windowproxy.rs, components/script/dom/bindings/reflector.rs, components/script/dom/bindings/root.rs and 2 more

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!
• These commits modify script code, but no tests are modified. Please consider adding a test!

[nox]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit 14846d0 with merge 4bf1aa0...

[bors-servo]

💔 Test failed - status-taskcluster

[nox]

All the failures are either the solid white intermittent issue or a silent crash that happens sometimes.

[nox]

That silent crash doesn't even reproduce locally.

[jdm]

Could you give me an overview of what this PR is doing? It's not clear to me how all the pieces fit together to fix #25701.

[nox]

Could you give me an overview of what this PR is doing? It's not clear to me how all the pieces fit together to fix #25701.

Sure. What it does is that it roots the boxed not-yet-reflected DOM instance with a different smart pointer type than Dom<T>. MaybeUnreflectedDom<T> traces the rooted T by first checking whether the reflector is set, in which case it proceeds just like Dom<T>, otherwise it directly traces the T, which will invoke the JSTraceable implementation of Reflector, which will do nothing but that's not a problem given it's not set yet.

[jdm]

This is a nice solution to the problem! Thanks for explaining it to me.

[jdm]

@bors-servo r+

[bors-servo]

📌 Commit 14846d0 has been approved by jdm

[bors-servo]

⌛ Testing commit 14846d0 with merge b6ecb05...

[bors-servo]

💔 Test failed - status-taskcluster

[nox]

Not sure if this crash is my fault or not, we'll see how the other build jobs fare.

[asajeffrey]

This is getting us pretty close to being able to support lazy creation of DOM reflectors.

[nox]

This only works because the T here is always the actual concrete type of the DOM instance, if you create an Element and you store it as MaybeUnreflectedDom<Node>, you won't trace everything that is specific to Element.

[asajeffrey]

Hmm, so we'd need support for dynamic dispatch of trace methods if we want to support subtyping of maybe-reflected DOM objects.

[nox]

Tried to reproduce the issue locally and failed.

@bors-servo retry

[bors-servo]

⌛ Testing commit 14846d0 with merge 554c90a...

[bors-servo]

💔 Test failed - status-taskcluster

[jdm]

@bors-servo retry
Network

[bors-servo]

⌛ Testing commit 14846d0 with merge 5a6b2d9...

[bors-servo]

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 5a6b2d9 to master...