[Presentation WebAPI] Fix collaboration issues with control channel

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: selin, Assigned: selin)

Tracking

(Blocks 1 bug)

unspecified
FxOS-S7 (18Sep)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

(Whiteboard: [ft:conndevices])

Attachments

(2 attachments, 4 obsolete attachments)

1. |nsIPresentationChannelDescription| interface defines an attribute |nsIArray tcpAddress| [1], but doesn't explicitly point out what type of the elements in the nsIArray should be. So TCPPresentationServer.js [2][3] uses wide strings whereas PresentationSessionInfo.cpp [4] uses narrow strings. This would cause the sender or receiver not able to get the correct peer address when trying to use the control channel.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/interfaces/nsIPresentationControlChannel.idl?offset=0#21
[2] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/provider/TCPPresentationServer.js#268
[3] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/provider/TCPPresentationServer.js#316
[4] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#95

2. The sender tries to establish the control channel and then send the offer [5]. However, the channel might not become ready to use at the moment so the offer would fail to be sent out [6]. Instead, we may send the offer once |notifyOpened| gets invoked when the control channel becomes ready.

[5] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#395
[6] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/provider/TCPPresentationServer.js#382-385
[7] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/interfaces/nsIPresentationControlChannel.idl#53
Assignee: nobody → selin
Whiteboard: [ft:conndevices]
Target Milestone: --- → FxOS-S7 (18Sep)
Use nsISupportsCString for the addresses in the channel description.
Attachment #8657000 - Flags: review?(fabrice)
Status: NEW → ASSIGNED
Summary: [Presentation API] Fix collaboration issues with control channel → [Presentation WebAPI] Fix collaboration issues with control channel
Attachment #8657001 - Flags: review?(bugs)
Comment on attachment 8657001 [details] [diff] [review]
Part 2 - Adjust the timing to send offer, v1

I really wish there was some documentation (including graphs/pictures) about our Presentation API implementation and different setups (OOP vs non-OOP etc) to help reviewing. I feel like reviewing these patches is always more difficult than it should be (and if reviewing feels harder than it should be, it probably means the quality of reviews is lower than it should be).
Perhaps this also hints that class names aren't quite right?
The spec uses terminology "controlling browsing context" and "presenting browsing context", not "requester" and "responder".
Attachment #8657001 - Flags: review?(bugs) → review+
Attachment #8657000 - Flags: review?(fabrice) → review+
You need to log in before you can comment on or make changes to this bug.