Status

()

For bugs in Firefox Desktop, the Mozilla Foundation's web browser. For Firefox user interface issues in menus, developer tools, bookmarks, location bar, and preferences. Many Firefox bugs will either be filed here or in the Core product. (more info)

OK I optimistically tried to tackle this... without much success so far.
I commented out the require importing sdk/core/heritage and then modified the models.js file to use ES6 classes, but when it comes to actually running the Web Audio Editor panel, it looks as if the models are losing their own methods - they look as undefined.
The error is:
```
TypeError: gAudioNodes.get is not a function: _onNodeSet@chrome://devtools/content/webaudioeditor/views/properties.js:123:24
emit@resource://devtools/shared/event-emitter.js:194:13
InspectorView.setCurrentAudioNode<@chrome://devtools/content/webaudioeditor/views/inspector.js:87:7
_run@resource://devtools/shared/task.js:311:39
TaskImpl@resource://devtools/shared/task.js:273:3
asyncFunction@resource://devtools/shared/task.js:247:14
resetUI@chrome://devtools/content/webaudioeditor/views/inspector.js:110:5
reset@chrome://devtools/content/webaudioeditor/controller.js:147:5
WebAudioEditorController._onTabNavigated<@chrome://devtools/content/webaudioeditor/controller.js:188:9
_run@resource://devtools/shared/task.js:311:39
TaskImpl@resource://devtools/shared/task.js:273:3
asyncFunction@resource://devtools/shared/task.js:247:14
emit@resource://devtools/shared/event-emitter.js:194:13
_setupRemoteListeners/this._onTabNavigated@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:553:9
eventSource/proto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:130:9
onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1018:7
send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
```
gAudioNodes is an instance of AudioNodesCollection, so it *should* have the methods it declares. But no, they're undefined (as you can check with `dump(gAudioNodes.get)`.
Strangely, the instance *does* have the inherited EventTarget methods.
We discussed it might be better to leave this bug for later as we need to work on the EventEmitter stuff as well, so I'm leaving the changes I did here: https://github.com/mozilla/gecko/compare/central...sole:bug1378849
and deassigning myself from this bug for now.

I think I've got a fix. I've had to convert the models.js file away from EventTarget and use Event-Emitter instead, but that itself required other fixes because events aren't sent the same way.
Things look good locally, I'll push to try.

Actually, let's hold off, I just discussed with Julian and his work on bug 1137935 is conflicting.
He's changing all event-related things, and I've had to do that in this bug too, but just for the webaudio editor. The goal of this bug is only to remove the Heritage usage, but to do that I've had to change how events are handled. So let's block on Julian's bug, and when it's done, I can simply remove the Heritage usage.

Comment on attachment 8898350[details]Bug 1378849 - Stop using add-on SDK APIs in the webaudio editor;
https://reviewboard.mozilla.org/r/169718/#review177320
Apart from the question about the OldEventEmitter being included, this looks good to me - it's what I was trying to do but with an EventEmitter that doesn't break the world when it's extended!
Even if the try build has green in our tests, did you try with a Web Audio page? You could try this http://sole.github.io/test_cases/web_audio/thx_cutting_out/ and see if the graph is rendered (press any of the buttons to create the context and start the sound)
::: devtools/client/webaudioeditor/includes.js:11
(Diff revision 2)
>
> const { loader, require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
> const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
> const { Task } = require("devtools/shared/task");
> -const { Class } = require("sdk/core/heritage");
> const OldEventEmitter = require("devtools/shared/old-event-emitter");
Is the old event emitter still used?

(In reply to Soledad Penades [:sole] [:spenades] from comment #8)
> it's what I was trying to do but with an EventEmitter that
> doesn't break the world when it's extended!
Thanks Julian for having migrated all the events! It made this bug so much easier to fix.
> Even if the try build has green in our tests, did you try with a Web Audio
> page? You could try this
> http://sole.github.io/test_cases/web_audio/thx_cutting_out/ and see if the
> graph is rendered (press any of the buttons to create the context and start
> the sound)
Just tested, this seems to be working well. I can see nodes coming and going. And I can click on them to inspect them.
> ::: devtools/client/webaudioeditor/includes.js:11
> (Diff revision 2)
> >
> > const { loader, require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
> > const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
> > const { Task } = require("devtools/shared/task");
> > -const { Class } = require("sdk/core/heritage");
> > const OldEventEmitter = require("devtools/shared/old-event-emitter");
>
> Is the old event emitter still used?
Huh, I didn't even realize this was here, I was merely focusing on removing the heritage dependency. The goal is to get rid of it of course, but I think we're still in the transition period.
I think we should fix this as part of bug 1384546.