Fix Akita WebRTC connection failure

RESOLVED FIXED

Status

P1
normal
Rank:
5
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dcritchley, Assigned: standard8)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [akita])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
When attempting to connect with participant in room, connection fails. Can not establish conference.
(Reporter)

Comment 1

2 years ago
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.
Flags: needinfo?(b.mcb)
If you mean the "no element found" error, I can see them in master too, but I'm looking into that
Flags: needinfo?(b.mcb)
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
I've also just tested turning off screen sharing on master, but that didn't show up anything either...
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.
(Reporter)

Comment 6

2 years ago
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?
Flags: needinfo?(standard8)
Flags: needinfo?(b.mcb)
Created attachment 8752158 [details] [review]
[loop] Standard8:bug-1272200-messagemanager > mozilla:akita
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.
Flags: needinfo?(standard8)
Flags: needinfo?(b.mcb)
Attachment #8752158 - Flags: review?(b.mcb)
Assignee: dcritchley → standard8
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!
Attachment #8752158 - Flags: review?(b.mcb) → review+
https://github.com/mozilla/loop/commit/4952764333339f5b8fc8e62de6a9adc5fa73a7c7
Status: NEW → RESOLVED
Iteration: --- → 50.2 - May 23
Last Resolved: 2 years ago
Rank: 5
Priority: -- → P1
Resolution: --- → FIXED

Updated

2 years ago
Blocks: 1271876
You need to log in before you can comment on or make changes to this bug.