Closed Bug 890543 Opened 6 years ago Closed 6 years ago

AudioBuffer doesn't need to inherit from nsISupports

Categories

(Core :: Web Audio, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25
Tracking Status
firefox24 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

Details

Attachments

(1 file)

Attached patch Patch (v1)Splinter Review
No description provided.
Attachment #771672 - Flags: review?(continuation)
Comment on attachment 771672 [details] [diff] [review]
Patch (v1)

Review of attachment 771672 [details] [diff] [review]:
-----------------------------------------------------------------

AudioBuffer's Unlink doesn't really need to |tmp->ClearJSChannels();|, but no big deal.
Attachment #771672 - Flags: review?(continuation) → review+
Huh, weird I didn't notice that before when I reviewed the patch that added it. ;)
Why do you say that is not necessary?
https://hg.mozilla.org/integration/mozilla-inbound/rev/33937005b8e4

checkin-needed for Aurora, a=webaudio.
Keywords: checkin-needed
The array and wrappercache are cleared even without it, as far as I can tell.  It doesn't really matter if we DROP the object, as long as we do it in the destructor.
https://hg.mozilla.org/mozilla-central/rev/33937005b8e4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
(In reply to comment #5)
> The array and wrappercache are cleared even without it, as far as I can tell. 
> It doesn't really matter if we DROP the object, as long as we do it in the
> destructor.

If there is no downside to doing that, I'd like to keep the code as it is today, since I think it's more responsible to drop the object once we're done with it, instead of in the future.  If there's a downside, however, please let me know and/or file a bug.  Thanks!
You need to log in before you can comment on or make changes to this bug.