Implemented type attribute for OscillatorNode interface

[PurpleHairEngineer]

Please see servo/media#242 for implementation of the new oscillator node message.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #___ (GitHub issue number if applicable)

• There are tests for these changes OR
• These changes do not require tests because ___

---

This change is 

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @ferjm (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/dom/webidls/OscillatorNode.webidl
• @KiChjang: components/script/dom/webidls/OscillatorNode.webidl

[highfive]

Warning

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

[jdm]

The implementation changes appear to be missing.

[PurpleHairEngineer]

@jdm The implementation changes are in servo/media#242. What do you mean when you say they appear to be missing? I didn't know how to submit a pull request for both servo/servo and servo/media at the same time, which is why I split it up into two different pull requests.

[jdm]

This change does not compile right now, since there are missing SetType and GetType methods for OscillatorNode.

[PurpleHairEngineer]

@jdm My bad. I think I added the correct methods, although GetType was called Type in biquadfilternode.rs, so I kept the same style and just called it Type.

[ferjm]

Thanks for the PR :)

There are still a few build errors that need to be fixed.

[PurpleHairEngineer]

@ferjm I fixed a few of the build errors, but I can't figure out why it can't find the get and set methods: error[E0599]: no method named get found for type dom::bindings::codegen::Bindings::OscillatorNodeBinding::OscillatorType in the current scope. This is on line 120 of oscillatornode.rs now (and line 125 for the set method).

[ferjm]

dom::bindings::codegen::Bindings::OscillatorNodeBinding::OscillatorType is an enum and it has no get or set methods.

You likely got these method calls from the BiquadFilterNode.filter related code that you were taking as example. If you look closer into that code, you could see that the we are using a Cell to provide interior mutability because the setters and getters of DOM struct members are getting a reference (&self) and not a mutable reference (&mut self). Cells expose the get and set methods you were calling.

We need exactly the same thing for OscillatorNode.oscillator_type. This is, to wrap it in a Cell to get interior mutability.

[PurpleHairEngineer]

@ferjm There are build errors for this, but a main reason seems to be that the OscillatorNodeMessage hasn't been implemented yet (see servo/media#242). Is there anything else I can do for this pull request before that one is merged?

[ferjm]

servo/media#242 was just merged. You should be able to update servo-media here to get the latest changes by running ./mach cargo-update -p servo-media.

[PurpleHairEngineer]

@ferjm Could you please take a look at this? I made the changes you suggested. I am getting build errors, but they all seem to be for files that are not modified by this pull request.

[PurpleHairEngineer]

@ferjm Now that I've committed and pushed the changes to Cargo.lock, it says this branch has merge conflicts that need to be resolved. I'm not sure how to resolve them because I'm not sure what the correct version of Cargo.lock is?

[jdm]

@PurpleHairEngineer I recommend performing a rebase, and when it encounters the merge conflict with Cargo.lock then run git rebase --skip, and once the rebase is complete then run the cargo update command that @ferjm mentioned. This will mean that the Cargo.lock will be in a pristine state, and then the update will be applied on top of it.

[PurpleHairEngineer]

@jdm I ran git rebase -i origin/master and closed the text editor without changing anything. I got the message Successfully rebased and updated refs/heads/OscillatorNodeType. It did not encounter the merge conflict. Did I enter the wrong command?

[ferjm]

@PurpleHairEngineer did you fetch upstream before rebasing?

git fetch upstream
git rebase upstream/master
(once the Cargo.lock conflict appears) git rebase --skip

These commands assume that you have a remote named upstream. To check if you have an upstream remote, run git remote -v, you should see something like upstream git@github.com:servo/servo.git (fetch). If you don't have it, you can run git remote add upstream git@github.com:servo/servo.git. And then run the fetch and rebase commands above.

In any case, I recommend waiting for #23294 to be merged, as updating servo-media before that one will throw additional build errors.

[ferjm]

@PurpleHairEngineer #23294 was already merged, so this PR should be ready for merge after rebasing. Let me know if you need any help with the rebase or if you want me to do it? Thanks.

[ferjm]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit 969d61a with merge 9c4e301...

[ferjm]

I went ahead and rebased and squashed the commits

[bors-servo]

💔 Test failed - linux-rel-wpt

[CYBAI]

{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "OscillatorNode interface: attribute type", 
    "test": "/webaudio/idlharness.https.window.html", 
    "line": 160167, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "OscillatorNode interface: new OscillatorNode(context) must inherit property \"type\" with the proper type", 
    "test": "/webaudio/idlharness.https.window.html", 
    "line": 160173, 
    "action": "test_result", 
    "expected": "FAIL"
}

[ferjm]

@bors-servo r+

[bors-servo]

📌 Commit f8bcdde has been approved by ferjm

[bors-servo]

⌛ Testing commit f8bcdde with merge 1cbb04c...

[bors-servo]

☀️ Test successful - arm64, linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: ferjm
Pushing 1cbb04c to master...