Implement WebIDL attribute PutForwards

[nox]

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/4847

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[nox]

This does not properly put in scope the forwarded attribute Methods trait, looking into it…

[nox]

Problem solved.

[jdm]

Reviewed files:

• components/script/dom/bindings/codegen/CodegenRust.py @ r3
• components/script/dom/bindings/codegen/parser/WebIDL.py @ r1

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2810 [r3] (raw file):}
Let's either make use of this or remove it.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4285 [r3] (raw file):}
Why can't we use attribute_arguments here?

---

^{components/script/dom/bindings/codegen/parser/WebIDL.py, line 1002 [r3] (raw file):}
Are these changes imported from Gecko? We have a pretty good track record of keeping WebIDL.py up to date with upstream, and we should make use of their work here.

---

---

Comments from the review on Reviewable.io

[jdm]

Reviewed files:

• components/script/dom/testbinding.rs @ r1
• components/script/dom/webidls/TestBinding.webidl @ r1

---

Comments from the review on Reviewable.io

[nox]

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2810 [r3] (raw file):}
I forgot to throw it away.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4285 [r3] (raw file):}
Didn't try, just went directly to the simplest code.

---

^{components/script/dom/bindings/codegen/parser/WebIDL.py, line 1002 [r3] (raw file):}
I didn't know there was an upstream. Where is it? That should be written in the file.

---

---

Comments from the review on Reviewable.io

[nox]

@jdm

The code you linked me to is not ideal IMO, it just uses the JSAPI directly and won't let me get fancy once I implement Reflect (#5881). I want to be able to generate a reflected forwarded setter that will just set the attribute directly through Element::AttributeHandlers and whatnot, without even needing to use the getter.

Note that my changes to WebIDL.py could very well be upstream'd. They are nothing more than new checks and a small refactoring.

[jdm]

@Ms2ger Does the previous comment sound web-compatible?

[nox]

I reached this conclusion from:

• https://heycam.github.io/webidl/#dfn-attribute-setter
• https://dom.spec.whatwg.org/#concept-reflect
• https://html.spec.whatwg.org/#reflecting-content-attributes-in-idl-attributes

More generally, I don't see how it would make sense for a reflecting IDL attribute to behave differently from the underlying reflected content attribute.

[Ms2ger]

I still believe the spec is crazy, but we should still follow it

[nox]

@Ms2ger What do you mean by follow it? In which way do I not follow it?

[nox]

@Ms2ger The only moment my assumptions aren't true is if the attribute is also [LenientThis], but in this case nothing at all is called anyway because the setter should stop at steps 5 or 7:

• If validThis is false and the attribute was not specified with the [LenientThis] extended attribute, then throw a TypeError.
• If validThis is false, then return.

Please show me one example where my assumptions break things.

[Ms2ger]

Think outside the box :)

<pre>
<script>
function w(s) {
  document.body.firstChild.appendChild(new Text(s + "\n"))
}

var l = document.createElement("link");
w(l.getAttribute("sizes")) // null

l.sizes = "bar";
w(l.getAttribute("sizes")) // bar

Object.defineProperty(l.sizes, "value", {
  get: function() { return "foo" },
  set: function() { w("hi!") }
})
l.sizes = "baz"; // hi!
w(l.getAttribute("sizes")) // bar
</script>
</pre>

[nox]

I stand corrected, and you stand insane. :P Will dumb down my code.

[nox]

@Ms2ger What if I use JS_GetPropertyDescriptor() and shortcut everything if they have been left untouched?

[Ms2ger]

Ew.

[jdm]

-S-awaiting-review +S-needs-code-changes

---

Reviewed files:

• components/script/dom/bindings/codegen/CodegenRust.py @ r4
• components/script/dom/bindings/codegen/parser/WebIDL.py @ r4

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2855 [r4] (raw file):}
nit: no need for the surrounding ()

---

Comments from the review on Reviewable.io

[jdm]

-S-awaiting-review +S-needs-squash

---

Reviewed files:

• components/script/dom/bindings/codegen/CodegenRust.py @ r5

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+

[bors-servo]

📌 Commit cc5eee4 has been approved by jdm

[bors-servo]

⌛ Testing commit cc5eee4 with merge 29a43a0...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2