1272200 - Fix Akita WebRTC connection failure

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[David Critchley (:dcritch)]

When attempting to connect with participant in room, connection fails. Can not establish conference.

Comment 1

[David Critchley (:dcritch)]

dmose and I paired on this for a while and did not come up with a fix or identify the reason the WebRTC connection is failing. I suspect it may be failing from an event that may be looking for a video element or other element in the DOM that is no longer there. There was a 1013 error that looked as though it may be the result of the failure, we were looking into that as well. Will be continuing to debug tomorrow, if you have any thoughts let me know.

Comment 2

[Manuel Casas Barrado [:mancas]]

If you mean the "no element found" error, I can see them in master too, but I'm looking into that

Comment 3

[Mark Banner (:standard8)]

I've just been looking at this as well. The "no element found" error seems to be standalone only. I'm not convinced that's an issue, especially as its on master as well.

Other items I've found:

- Connecting a master add-on (room owner) to akita standalone (guest), works fine.

- There's a few isDesktop & other settings missing in sidebar.jsx (when compared to conversation.jsx), however fixing those doesn't fix this issue.

- I did a review of the dispatcher messages comparing the owner side for master vs akita branches, and the messages were the same apart from the screen sharing items which is expected differences. Screen sharing is a semi-separate part and doesn't affect states or anything so I don't think that's going to affect this, though I could be wrong.

- I've also reviewed diffs between master & akita [1] but there's no changes for activeRoomStore/otSDKDriver, and no vendor changes either...

- The sidebar isn't running in the content process, but running a non-e10s window doesn't help either.

[1] https://github.com/mozilla/loop/compare/akita

Comment 4

[Mark Banner (:standard8)]

I've also just tested turning off screen sharing on master, but that didn't show up anything either...

Comment 5

[Mark Banner (:standard8)]

This seems to be something weird with permissions. If I set "media.navigator.permission.disabled" to true on the owner's side, then akita will connect.

The strange thing is, we get local video without that being set. Set we've got at least some gUM permission.

Comment 6

[David Critchley (:dcritch)]

connection is failing in getPermission() from PeerConnection.js while trying to setup the description in the sdp. Issue fixed by adding this line to the beginning of the getPermission():
return this._havePermission = Promise.resolve();

Is there a way to fix the issue from the addon? Perhaps setting this._havePermission?

Comment 7

[GitHub Autolander Bot]

Attachment: [loop] Standard8:bug-1272200-messagemanager > mozilla:akita

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8752158 [details] [review]
[loop] Standard8:bug-1272200-messagemanager > mozilla:akita

Thanks to Dave for finding the issue in PeerConnection.js. I followed that through a bit more - it turns out that PeerConnection.js#getPermission is an extension hook for add-ons to allow them to deny peer connections for security purposes.

In the e10s world, PeerConnection.js runs in the content process, and a observer notification is made to ContentWebRTC.jsm, this then sends a message to the parent process (via the message manager) to see if the peer connection is allowed or not.

However, what was happening was that because we didn't have the messagemanager items hooked up to our browser element, the message passing seemed to get lost, and PeerConnection never got to find out whether or not the permission was allowed or not.

Although I've tried hooking up this before, I think I'd generally been working with incomplete versions of dmose's about:loop* patches, and so changing the side bar browser to a remote one didn't work fully (or I'd not set up the right parameters).

In any case, hooking up the e10s functionality for the sidebar browser now makes the sidebar load in the content process, and gets WebRTC connections working :-)


In the patch there is:

- Hook up the required attributes on the browser element
- Drop the "-" in some of the attribute names, these were wrong.
- Create our own "loop" message manager group - this is separate to the social one that we were using. As we're now more separated, I think that's the right thing to do.
- Loads the generic browser content.js into the "loop" group (we need this to keep WebRTC & things working!)
- Forces about:looptoc to load in the content process (it might have been anyway, but this makes sure, and gets it done early).

Due to the move to our own message manager group, and move away to the chat box, there will be some hooks for peer connection monitoring and stats code that we need to move. I'll get a bug or two filed for those as follow-ups. They aren't needed straight away.

Comment 9

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8752158 [details] [review]
[loop] Standard8:bug-1272200-messagemanager > mozilla:akita

r=me with nit addressed. Nice to have finally webRTC working again. Great teamwork, guys!

Comment 10

[Mark Banner (:standard8)]

https://github.com/mozilla/loop/commit/4952764333339f5b8fc8e62de6a9adc5fa73a7c7