[Perfect Negotiation] Fire onnegotiationneeded when chain is empty.
This CL generates "negotiationneeded" events if negotiation is needed
when the Operations Chain becomes empty. This is only implemented in
Unified Plan to avoid Plan B regressions (the event is pretty useless
in Plan B as it fires repeatedly).
In order to implement the spec-compliant behavior of only firing the
event when the chain is empty, this CL introduces
PeerConnectionObserver::OnNegotiationNeededEvent() and
PeerConnectionInterface::ShouldFireNegotiationNeededEvent() to allow
validating the event before firing it. This is needed because the event
must not be fired until a task has been posted and subsequently chained
operations could invalidate it in the meantime.
Test coverage is added for both legacy and modern "negotiationneeded"
events.
Bug: chromium:1060083
Change-Id: I1dbaa8f6ddb1c6e7c8abd8da3b92efcb64060383
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180620
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31989}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 09317b8..3ada740 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -1003,6 +1003,16 @@
virtual void SetRemoteDescription(SetSessionDescriptionObserver* observer,
SessionDescriptionInterface* desc) {}
+ // According to spec, we must only fire "negotiationneeded" if the Operations
+ // Chain is empty. This method takes care of validating an event previously
+ // generated with PeerConnectionObserver::OnNegotiationNeededEvent() to make
+ // sure that even if there was a delay (e.g. due to a PostTask) between the
+ // event being generated and the time of firing, the Operations Chain is empty
+ // and the event is still valid to be fired.
+ virtual bool ShouldFireNegotiationNeededEvent(uint32_t event_id) {
+ return true;
+ }
+
virtual PeerConnectionInterface::RTCConfiguration GetConfiguration() = 0;
// Sets the PeerConnection's global configuration to |config|.
@@ -1176,7 +1186,17 @@
// Triggered when renegotiation is needed. For example, an ICE restart
// has begun.
- virtual void OnRenegotiationNeeded() = 0;
+ // TODO(hbos): Delete in favor of OnNegotiationNeededEvent() when downstream
+ // projects have migrated.
+ virtual void OnRenegotiationNeeded() {}
+ // Used to fire spec-compliant onnegotiationneeded events, which should only
+ // fire when the Operations Chain is empty. The observer is responsible for
+ // queuing a task (e.g. Chromium: jump to main thread) to maybe fire the
+ // event. The event identified using |event_id| must only fire if
+ // PeerConnection::ShouldFireNegotiationNeededEvent() returns true since it is
+ // possible for the event to become invalidated by operations subsequently
+ // chained.
+ virtual void OnNegotiationNeededEvent(uint32_t event_id) {}
// Called any time the legacy IceConnectionState changes.
//