Support pair and value iterable WebIDL bindings

[jdm]

The actual iterator implementation and JSAPI calls related to setting up the interface are ported directly from Gecko's Codegen.py, IterableIterator.h, and IterableIterator.webidl. The changes to support multiple interfaces in one file are required because the internal iterator interface the parser generates gets associated with the original interface's WebIDL file. It seemed like a good time to address #571 in that case.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #12628 and fix #571.
• There are tests for these changes

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/script/dom/bindings/codegen/CodegenRust.py, components/script/dom/bindings/mod.rs, components/script/dom/testbindingiterable.rs, components/script/dom/bindings/codegen/parser/update.sh, components/script/dom/webidls/IterableIterator.webidl, components/script/dom/webidls/TestBindingIterable.webidl, components/script/dom/mod.rs, components/script/dom/webidls/TestBindingPairIterable.webidl, components/script/dom/bindings/codegen/parser/WebIDL.py, components/script/dom/bindings/iterable.rs, components/script/dom/bindings/codegen/parser/callback-location.patch, components/script/dom/testbindingpairiterable.rs, components/script/dom/bindings/codegen/Configuration.py

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[jdm]

r? @Manishearth

[jdm]

I don't understand why appveyor is failing, since that error was intended to be addressed by the modification to WebIDL.py (which worked locally and on travis).

[Manishearth]

I'm not familiar enough with codegen to give this a proper review -- I understand the changes, but not enough to find any mistakes/issues/etc.

r? @Ms2ger

[KiChjang]

@nox is also a good person to review codegen.

[nox]

Reviewed 5 of 5 files at r1, 7 of 7 files at r2.
Review status: 7 of 18 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

a discussion (no related file):
Can you paste a diff of the generated code showing the changes of supporting multiple WebIDL interfaces in a single file?

---

components/script/dom/bindings/codegen/CodegenRust.py, line 1749 [r1] (raw file):

            return t.identifier

        def removeWrapperAndNullableTypes(types):

I don't understand what that function is for, it looks like a misnomer. My intuition is that it peels wrappers and nullable types, but then it returns an array so I don't know what it does.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 2110 [r1] (raw file):

    unionStructs = (i[1] for i in sorted(unionStructs.items(), key=operator.itemgetter(0)))

    return CGImports(CGList(unionStructs, "\n\n"), [], [], [], [], imports, config, ignored_warnings=[])

Nit: could you use named arguments?

---

components/script/dom/bindings/codegen/CodegenRust.py, line 2708 [r1] (raw file):

    def __init__(self, descriptor):
        CGGetPerInterfaceObject.__init__(self, descriptor, "GetProtoObject",
                                         "PrototypeList::ID", pub=True)

Why do we need to always expose GetProtoObject and GetConstructorObject now?

---

components/script/dom/bindings/codegen/CodegenRust.py, line 1521 [r2] (raw file):

                "condition": PropertyDefiner.getControllingCondition(m,
                                                                     descriptor)
            })

So where are the magic definitions of all these methods?

---

components/script/dom/bindings/codegen/parser/callback-location.patch, line 13 [r1] (raw file):

                 else:
                     type = IDLWrapperType(self.getLocation(p, 1), p[1])
                 p[0] = self.handleModifiers(type, p[2])

Did you upstream this?

---

Comments from Reviewable

[nox]

I've reviewed the first two commits and will wait for an answer before reviewing the rest. On a side-note, @jdm could you shorten your commit titles? They are hard to read in git-log and on GH.

[jdm]

Review status: 7 of 18 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 1749 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

I don't understand what that function is for, it looks like a misnomer. My intuition is that it peels wrappers and nullable types, but then it returns an array so I don't know what it does.

It accepts an array of types, and returns an array of types that have had their wrapper and nullable-ness removed.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 2708 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

Why do we need to always expose GetProtoObject and GetConstructorObject now?

The functions need to be public to be reexported, and the code that adds the names to the reexport list shouldn't need to replicate the logic to decide if the functions need to be public or not. This was the easiest solution.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 1521 [r2] (raw file):

Previously, nox (Anthony Ramine) wrote…

So where are the magic definitions of all these methods?

They are self-hosted and already exist.

---

components/script/dom/bindings/codegen/parser/callback-location.patch, line 13 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

Did you upstream this?

Not yet. I'd like to do that as a followup.

---

Comments from Reviewable

[nox]

-S-awaiting-review +S-awaiting-answer

Feel free to r=me if my last question is nonsense.

---

Reviewed 11 of 11 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

a discussion (no related file):

Previously, nox (Anthony Ramine) wrote…

Can you paste a diff of the generated code showing the changes of supporting multiple WebIDL interfaces in a single file?

You forgot to do that.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 2675 [r4] (raw file):

""" % properties))

        aliasedMembers = [m for m in self.descriptor.interface.members if m.isMethod() and m.aliases]

What are aliases here? I thought we didn't support Alias yet. Is that something different?

---

Comments from Reviewable

[jdm]

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

a discussion (no related file):

Previously, nox (Anthony Ramine) wrote…

You forgot to do that.

Didn't forget; just not complete yet.

---

components/script/dom/bindings/codegen/CodegenRust.py, line 2675 [r4] (raw file):

Previously, nox (Anthony Ramine) wrote…

What are aliases here? I thought we didn't support Alias yet. Is that something different?

forEach is an automatic alias of @@iterator that is created by the webidl parser.

---

Comments from Reviewable

[jdm]

Before/after diff for TestBindingPairIterableBinding.rs when adding the iterable declaration that causes an extra interface to be generated in the binding file: https://gist.github.com/jdm/0c2fa6068c6f630a5377550f635cc2bb

[jdm]

Whee, now appveyor passes and travis fails with the same error getting Function's original location wrong. I wonder if this is caused by running multiple parsers in parallel...

[bors-servo]

☔ The latest upstream changes (presumably #13009) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

@bors-servo: r=nox
One appveyor build had a weird problem with angle; not the fault of this PR.

[bors-servo]

📌 Commit 928c4df has been approved by nox

[bors-servo]

⌛ Testing commit 928c4df with merge 25841eb...

[bors-servo]

💔 Test failed - arm64

[jdm]

@bors-servo: r=nox

[bors-servo]

📌 Commit 86a0c45 has been approved by nox

[bors-servo]

⌛ Testing commit 86a0c45 with merge 1370fa5...

[bors-servo]

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev