If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Cannot call createOffer/setLocalDescription in "have-local-offer" state, nor createAnswer/setRemoteDescription in "have-remote-offer" state

NEW
Unassigned

Status

()

Core
WebRTC
P5
minor
Rank:
45
3 years ago
6 days ago

People

(Reporter: Alan Ford, Unassigned)

Tracking

(Blocks: 1 bug)

35 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
Testing with Firefox Nightly (v35, 2014-09-24), I try to call PeerConnection.createOffer again once ICE candidate gathering is complete.

Multiple calls to createOffer appears to be perfectly valid according to my reading of http://www.w3.org/TR/webrtc/, yet I get an error back:

Object { name: "INVALID_STATE", message: "Cannot create offer in state HAVE_LOCAL_OFFER" }
Besides the question if multiple calls of createOffer() are valid or not, if you are interested in getting access to the updated SDP with the ICE candidates in it, simply hold a reference to the SDP you got on your first createOffer() call, the SDP in there will be updated with the candidates.
(In reply to Alan Ford from comment #0)
I've verified that setLocalDescription is broken the same way: "Cannot set local SDP in state HAVE_LOCAL_OFFER"

The signalingstate diagram [1] shows that it is legal to call setLocalDescription() in "have-local-offer", so by extension I would think you should be able to call createOffer as well in this state.

[1] http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtcpeerstate-enum
(In reply to Nils Ohlmeier [:drno] from comment #1)
> Besides the question if multiple calls of createOffer() are valid or not, if
> you are interested in getting access to the updated SDP with the ICE
> candidates in it, simply hold a reference to the SDP you got on your first
> createOffer() call, the SDP in there will be updated with the candidates.

I'm pretty sure that doesn't work, as RTCSessionDescription just holds an immutable string. See [1].


[1] http://mxr.mozilla.org/mozilla-central/source/dom/media/PeerConnection.js?rev=c885916823da#194
(In reply to Jan-Ivar Bruaroey [:jib] from comment #2)
> (In reply to Alan Ford from comment #0)
> I've verified that setLocalDescription is broken the same way: "Cannot set
> local SDP in state HAVE_LOCAL_OFFER"
> 
> The signalingstate diagram [1] shows that it is legal to call
> setLocalDescription() in "have-local-offer", so by extension I would think
> you should be able to call createOffer as well in this state.

While I agree that there is no wording in the spec right now which describes how to handle multiple calls to createOffer(), and the diagram at least suggests multiple calls to setLocalDescription are possible I think multiple calls to createOffer() can be quite dangerous.
If you only call createOffer() multiple times without ever sending the offer to the far end it should probably save to do so. BUT if you send each of the offers created by createOffer() to the far end, you will easily at least confuse the offer answer process of SDP. This would get us into the space of SIP UPDATE's and stuff like that. SDP does not offer diff updates and therefore usually requires an answer to the first offer, before another offer can be made.
To avoid restrictions like described above (you can call createOffer() multiple times as long as you never send the offer anywhere), I think it is the safer choice to allow another call to createOffer() only from the Stable state.
To get the updated sdp, use pc.localDescription.
(In reply to Jan-Ivar Bruaroey [:jib] from comment #5)
> To get the updated sdp, use pc.localDescription.

Correct. Sorry it was not the reference to the offer, but the pc.localDescription which gets updated.
(In reply to Nils Ohlmeier [:drno] from comment #4)
> To avoid restrictions like described above (you can call createOffer()
> multiple times as long as you never send the offer anywhere), I think it is
> the safer choice to allow another call to createOffer() only from the Stable
> state.

If you're talking about what to *allow* then that needs to be taken up on the list.

Also I'm not following your rationale. Isn't whatever people do outside of WebRTC their business? Isn't how things look in stable state all that matters to us?
(In reply to Jan-Ivar Bruaroey [:jib] from comment #7)
> If you're talking about what to *allow* then that needs to be taken up on
> the list.

Indeed. I think this is a topic which should be taken to the list, so that a clarification gets added to the spec. 
 
> Also I'm not following your rationale. Isn't whatever people do outside of
> WebRTC their business? Isn't how things look in stable state all that
> matters to us?

Exactly we only specify the API. Therefore the API spec should NOT have restrictions like "you are not allowed to send this somewhere unless...".

Lets say we add to the spec: createOffer() can be called multiple times and it will give you the most recent offer no matter in which signaling state you are in.
I'm pretty sure someone would then write some JS code which calls createOffer() in a loop until ICE gathering has finished and every time send the latest offer to the other side (instead of registering a callback to onicecandidate and just send the ICE candidates to the other side). But that would create race conditions on the answerer side: if the answerer side created an Answer already, but has not called setLocalDescription, and then receives a new Offer, what do you?
You could probably "require" that setLocalDescription() needs to be called first with the answer. But why are these functions then separate (instead of createAnswer() calling internally setLocalDescription() itself already).
And if you send the Answer to the offerer how does he know for which of his offers the answer is meant?

This is probably no big deal if the diff between multiple offers is only new ICE candidates. But we can't guarantee that. It could be that the 2nd offer added another m line...
(In reply to Nils Ohlmeier [:drno] from comment #8)
> Indeed. I think this is a topic which should be taken to the list, so that a
> clarification gets added to the spec. 

Feel free, though I think the spec is perfectly clear. createOffer isn't even in the state diagram.

> Lets say we add to the spec: createOffer() can be called multiple times and
> it will give you the most recent offer no matter in which signaling state
> you are in.

The spec is written for browsers, not users. It puts restrictions on browser behavior, not user behavior. A user can call a function as many times as they want. Since there's no language in createOffer [1] to suggest it should behave differently based on any state but "closed", it wont. It better work each time people call it. That's our first bug.

It also says browsers MUST allow setLocalDescription in "has-local-offer". That's our second bug.

> I'm pretty sure someone would then write some JS code which calls
> createOffer() in a loop until ICE gathering has finished and every time send
> the latest offer to the other side (instead of registering a callback to
> onicecandidate and just send the ICE candidates to the other side). But that
> would create race conditions on the answerer side: if the answerer side
> created an Answer already, but has not called setLocalDescription, and then
> receives a new Offer, what do you?

The spec also says browsers MUST allow setRemoteDescription in "has-remote-offer" so you should be fine to call createAnswer again too, and there's probably our third bug.

As to a race: I don't see a race. People are free to use whatever signaling/synchronization mechanism they choose, and it is not up to us to question or debug that. It may be perfectly synchronized without any races. It is extrinsic. The perfect abstraction of this is the local-loop test-call. Therefore, unless you can show a race in local-loop call, I claim they're extrinsic and not our business.

[1] http://dev.w3.org/2011/webrtc/editor/webrtc.html#widl-RTCPeerConnection-createOffer-void-RTCSessionDescriptionCallback-successCallback-RTCPeerConnectionErrorCallback-failureCallback-RTCOfferOptions-options
(In reply to Jan-Ivar Bruaroey [:jib] from comment #9)
> (In reply to Nils Ohlmeier [:drno] from comment #8)
> > Indeed. I think this is a topic which should be taken to the list, so that a
> > clarification gets added to the spec. 
> 
> Feel free, though I think the spec is perfectly clear. createOffer isn't
> even in the state diagram.

I don't think the spec is perfectly clear, but there was agreement
at the last interim that it was safe to call createOffer() repeatedly.


> > Lets say we add to the spec: createOffer() can be called multiple times and
> > it will give you the most recent offer no matter in which signaling state
> > you are in.
> 
> The spec is written for browsers, not users. It puts restrictions on browser
> behavior, not user behavior. A user can call a function as many times as
> they want. Since there's no language in createOffer [1] to suggest it should
> behave differently based on any state but "closed", it wont. It better work
> each time people call it. That's our first bug.
> 
> It also says browsers MUST allow setLocalDescription in "has-local-offer".
> That's our second bug.
> 
> > I'm pretty sure someone would then write some JS code which calls
> > createOffer() in a loop until ICE gathering has finished and every time send
> > the latest offer to the other side (instead of registering a callback to
> > onicecandidate and just send the ICE candidates to the other side). But that
> > would create race conditions on the answerer side: if the answerer side
> > created an Answer already, but has not called setLocalDescription, and then
> > receives a new Offer, what do you?
> 
> The spec also says browsers MUST allow setRemoteDescription in
> "has-remote-offer" so you should be fine to call createAnswer again too, and
> there's probably our third bug.
> 
> As to a race: I don't see a race. People are free to use whatever
> signaling/synchronization mechanism they choose, and it is not up to us to
> question or debug that. It may be perfectly synchronized without any races.
> It is extrinsic. The perfect abstraction of this is the local-loop
> test-call. Therefore, unless you can show a race in local-loop call, I claim
> they're extrinsic and not our business.
> 
> [1]
> http://dev.w3.org/2011/webrtc/editor/webrtc.html#widl-RTCPeerConnection-
> createOffer-void-RTCSessionDescriptionCallback-successCallback-
> RTCPeerConnectionErrorCallback-failureCallback-RTCOfferOptions-options
(In reply to Alan Ford from comment #0)
> Object { name: "INVALID_STATE", message: "Cannot create offer in state
> HAVE_LOCAL_OFFER" }

Alan, I'm going to assume you called setLocalDescription as well here, otherwise I can't reproduce or explain why you're in "have-local-offer" state. Please let us know if that's not the case.

Renaming subject to include all three problems (unless we want to break things up).
Summary: Cannot call PeerConnect.createOffer once offer is created → Cannot call createOffer/setLocalDescription in "have-local-offer" state, nor createAnswer/setRemoteDescription in "have-remote-offer" state

Comment 12

3 years ago
> pc.createOffer(function(offer) {
>   pc.setLocalDescription(offer, function() {
>     pc.setLocalDescription(pc.localDescription, function() {
>       // Works in chrome
>     }, function(error) {
>       // in FF: { name: "INVALID_STATE", message: "Cannot create offer in state HAVE_LOCAL_OFFER" }
>     });
>   });
> })

The following should work according to "http://www.w3.org/TR/webrtc/" but it doesn't. I understand that setting the same offer as local desciption twice makes no sense but generating this error (although the local sdp hasnt changed) is a little harsh.
Blocks: 1165687
backlog: --- → webRTC+
Rank: 45
Priority: -- → P4

Updated

2 years ago
Duplicate of this bug: 1193724

Updated

2 years ago
Duplicate of this bug: 1081252
Mass change P4->P5 to align with new Mozilla triage process.
Priority: P4 → P5
You need to log in before you can comment on or make changes to this bug.