859600 - Make AudioContext an EventTarget

Status: RESOLVED FIXED

Description

[:Ehsan Akhgari (Out of office 5/10-5/21)]

https://dvcs.w3.org/hg/audio/rev/f4b0a9d88309

Comment 1

[:Ehsan Akhgari (Out of office 5/10-5/21)]

Attachment: WIP

This crashes when AddRefing an AudioContext:

Assertion failure: e->mObject == owner (wrong entry), at ../../../dist/include/nsISupportsImpl.h:145

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
nsCycleCollectingAutoRefCnt::incr (this=0x124f61e58, owner=0x124f61e10) at nsISupportsImpl.h:145
145	      MOZ_ASSERT(e->mObject == owner, "wrong entry");
(gdb) bt
#0  nsCycleCollectingAutoRefCnt::incr (this=0x124f61e58, owner=0x124f61e10) at nsISupportsImpl.h:145
#1  0x0000000102639d54 in mozilla::dom::AudioContext::AddRef (this=0x124f61e10) at AudioContext.h:61
#2  0x0000000102639bd8 in nsRefPtr<mozilla::dom::AudioContext>::nsRefPtr (this=0x10d193de8, aRawPtr=0x124f61e10) at nsAutoPtr.h:904
#3  0x0000000102638f8d in nsRefPtr<mozilla::dom::AudioContext>::nsRefPtr (this=0x10d193de8, aRawPtr=0x124f61e10) at nsAutoPtr.h:905
#4  0x00000001026386a4 in mozilla::dom::AudioBuffer::AudioBuffer (this=0x10d193dc0, aContext=0x124f61e10, aLength=2048, aSampleRate=8000) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioBuffer.cpp:51
#5  0x00000001026385fd in mozilla::dom::AudioBuffer::AudioBuffer (this=0x10d193dc0, aContext=0x124f61e10, aLength=2048, aSampleRate=8000) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioBuffer.cpp:55
#6  0x000000010263f45f in mozilla::dom::AudioContext::CreateBuffer (this=0x124f61e10, aJSContext=0x123683f80, aNumberOfChannels=2, aLength=2048, aSampleRate=8000, aRv=@0x7fff5fbf4690) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioContext.cpp:100
#7  0x0000000103bca42d in mozilla::dom::AudioContextBinding::createBuffer (cx=0x123683f80, obj={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbf4790}, self=0x124f61e10, argc=3, vp=0x10da321c0) at AudioContextBinding.cpp:226

Am I doing something wrong with the CC macros here?

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 741663 [details] [diff] [review]
WIP

Hmm.  My interpretation is that it looks like nsDOMEventTargetHelper is nsISupports, so you are going to have to reCOMtaminate AudioContext.

- Instead of NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING I think you want NS_DECL_ISUPPORTS_INHERITED.
- Define a QI again with the right CC stuff.

Probably other stuff.

Comment 3

[:Ehsan Akhgari (Out of office 5/10-5/21)]

I am a fool, and you are a hero! :-)

Comment 4

[:Ehsan Akhgari (Out of office 5/10-5/21)]

Attachment: Patch (v1)

Comment 5

[Paul Adenot (:padenot)]

Comment on attachment 741671 [details] [diff] [review]
Patch (v1)

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

I can't review that. While I think I understand what is going on, I have no idea if it is correct, sorry.

Comment 6

[Andrew McCreight [:mccr8]]

Maybe Olli could review it?

Comment 7

[:Ehsan Akhgari (Out of office 5/10-5/21)]

Comment on attachment 741671 [details] [diff] [review]
Patch (v1)

Sure.

Comment 8

[Olli Pettay [:smaug]]

Comment on attachment 741671 [details] [diff] [review]
Patch (v1)

As far as I see, you can drop mWindow. nsDOMEventTargetHelper knows the owner
and has GetOwner().

With that, r=me

Comment 9

[:Ehsan Akhgari (Out of office 5/10-5/21)]

Sweet!

https://hg.mozilla.org/integration/mozilla-inbound/rev/120f3f229643

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/120f3f229643

Comment 11

[:Ehsan Akhgari (Out of office 5/10-5/21)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.