From 36e3147b219de57325e9b009fcf672a872083566 Mon Sep 17 00:00:00 2001 From: Qingsi Wang Date: Wed, 29 May 2019 11:37:26 -0700 Subject: [PATCH] Surface the standardized ICE connection state to mobile clients. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds the callback on changes of the ICE connection state following the standardized transitions (https://www.w3.org/TR/webrtc/#dom-rtciceconnectionstate) to the Android and the iOS SDKs. Bug: None Change-Id: I6133391fa54dd4e09016f29dddb85e4a0e270878 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138181 Reviewed-by: Kári Helgason Reviewed-by: Steve Anton Reviewed-by: Amit Hilbuch Reviewed-by: Alex Glaznev Commit-Queue: Qingsi Wang Cr-Commit-Position: refs/heads/master@{#28127} --- pc/peer_connection.cc | 11 +++++- .../api/org/webrtc/PeerConnection.java | 4 ++ .../src/org/webrtc/PeerConnectionTest.java | 39 +++++++++++++++++++ sdk/android/src/jni/pc/peer_connection.cc | 8 ++++ sdk/android/src/jni/pc/peer_connection.h | 2 + .../RTCPeerConnection+Private.h | 3 ++ .../api/peerconnection/RTCPeerConnection.h | 5 +++ .../api/peerconnection/RTCPeerConnection.mm | 10 +++++ 8 files changed, 80 insertions(+), 2 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 14c8683256..d0698d8c65 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -4125,10 +4125,17 @@ void PeerConnection::SetIceConnectionState(IceConnectionState new_state) { void PeerConnection::SetStandardizedIceConnectionState( PeerConnectionInterface::IceConnectionState new_state) { - if (standardized_ice_connection_state_ == new_state) + if (standardized_ice_connection_state_ == new_state) { return; - if (IsClosed()) + } + + if (IsClosed()) { return; + } + + RTC_LOG(LS_INFO) << "Changing standardized IceConnectionState " + << standardized_ice_connection_state_ << " => " << new_state; + standardized_ice_connection_state_ = new_state; Observer()->OnStandardizedIceConnectionChange(new_state); } diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java index 0dc5c63441..5f802fb7ca 100644 --- a/sdk/android/api/org/webrtc/PeerConnection.java +++ b/sdk/android/api/org/webrtc/PeerConnection.java @@ -98,6 +98,10 @@ public class PeerConnection { /** Triggered when the IceConnectionState changes. */ @CalledByNative("Observer") void onIceConnectionChange(IceConnectionState newState); + /* Triggered when the standard-compliant state transition of IceConnectionState happens. */ + @CalledByNative("Observer") + default void onStandardizedIceConnectionChange(IceConnectionState newState) {} + /** Triggered when the PeerConnectionState changes. */ @CalledByNative("Observer") default void onConnectionChange(PeerConnectionState newState) {} diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java index bf23d19256..a1a11a5e7d 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java +++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java @@ -83,6 +83,7 @@ public class PeerConnectionTest { private int expectedTracksAdded; private Queue expectedSignalingChanges = new ArrayDeque<>(); private Queue expectedIceConnectionChanges = new ArrayDeque<>(); + private Queue expectedStandardizedIceConnectionChanges = new ArrayDeque<>(); private Queue expectedConnectionChanges = new ArrayDeque<>(); private Queue expectedIceGatheringChanges = new ArrayDeque<>(); private Queue expectedAddStreamLabels = new ArrayDeque<>(); @@ -183,6 +184,12 @@ public class PeerConnectionTest { expectedIceConnectionChanges.add(newState); } + // TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression. + @SuppressWarnings("NoSynchronizedMethodCheck") + public synchronized void expectStandardizedIceConnectionChange(IceConnectionState newState) { + expectedStandardizedIceConnectionChanges.add(newState); + } + @Override // TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression. @SuppressWarnings("NoSynchronizedMethodCheck") @@ -201,6 +208,23 @@ public class PeerConnectionTest { assertEquals(expectedIceConnectionChanges.remove(), newState); } + @Override + // TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression. + @SuppressWarnings("NoSynchronizedMethodCheck") + public synchronized void onStandardizedIceConnectionChange(IceConnectionState newState) { + if (newState.equals(IceConnectionState.COMPLETED)) { + return; + } + + if (expectedIceConnectionChanges.isEmpty()) { + System.out.println( + name + "Got an unexpected standardized ICE connection change " + newState); + return; + } + + assertEquals(expectedStandardizedIceConnectionChanges.remove(), newState); + } + // TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression. @SuppressWarnings("NoSynchronizedMethodCheck") public synchronized void expectConnectionChange(PeerConnectionState newState) { @@ -977,6 +1001,8 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021). @@ -985,6 +1011,8 @@ public class PeerConnectionTest { // IceConnectionState.COMPLETED); answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); offeringPC.setRemoteDescription(sdpLatch, answerSdp); @@ -1214,11 +1242,15 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021). answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); offeringPC.setRemoteDescription(sdpLatch, answerSdp); @@ -1396,6 +1428,8 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021). @@ -1404,6 +1438,8 @@ public class PeerConnectionTest { // IceConnectionState.COMPLETED); answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CHECKING); + answeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CONNECTED); answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); offeringPC.setRemoteDescription(sdpLatch, answerSdp); @@ -1606,10 +1642,12 @@ public class PeerConnectionTest { // Note that this test isn't meant to test these events, but we must do // this or otherwise it will crash. offeringExpectations.expectIceConnectionChange(IceConnectionState.CLOSED); + offeringExpectations.expectStandardizedIceConnectionChange(IceConnectionState.CLOSED); offeringExpectations.expectSignalingChange(SignalingState.CLOSED); offeringExpectations.expectIceGatheringChange(IceGatheringState.COMPLETE); offeringPC.dispose(); expectations.expectIceConnectionChange(IceConnectionState.CLOSED); + expectations.expectStandardizedIceConnectionChange(IceConnectionState.CLOSED); expectations.expectSignalingChange(SignalingState.CLOSED); expectations.expectIceGatheringChange(IceGatheringState.COMPLETE); pcUnderTest.dispose(); @@ -1790,6 +1828,7 @@ public class PeerConnectionTest { assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS)); expectations.expectIceConnectionChange(IceConnectionState.CLOSED); + expectations.expectStandardizedIceConnectionChange(IceConnectionState.CLOSED); expectations.expectConnectionChange(PeerConnectionState.CLOSED); expectations.expectSignalingChange(SignalingState.CLOSED); pc.close(); diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index 4d963b0454..bd50f5b121 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -298,6 +298,14 @@ void PeerConnectionObserverJni::OnIceConnectionChange( Java_IceConnectionState_fromNativeIndex(env, new_state)); } +void PeerConnectionObserverJni::OnStandardizedIceConnectionChange( + PeerConnectionInterface::IceConnectionState new_state) { + JNIEnv* env = AttachCurrentThreadIfNeeded(); + Java_Observer_onStandardizedIceConnectionChange( + env, j_observer_global_, + Java_IceConnectionState_fromNativeIndex(env, new_state)); +} + void PeerConnectionObserverJni::OnConnectionChange( PeerConnectionInterface::PeerConnectionState new_state) { JNIEnv* env = AttachCurrentThreadIfNeeded(); diff --git a/sdk/android/src/jni/pc/peer_connection.h b/sdk/android/src/jni/pc/peer_connection.h index 91d4ef090d..884ce7c9b6 100644 --- a/sdk/android/src/jni/pc/peer_connection.h +++ b/sdk/android/src/jni/pc/peer_connection.h @@ -51,6 +51,8 @@ class PeerConnectionObserverJni : public PeerConnectionObserver { PeerConnectionInterface::SignalingState new_state) override; void OnIceConnectionChange( PeerConnectionInterface::IceConnectionState new_state) override; + void OnStandardizedIceConnectionChange( + PeerConnectionInterface::IceConnectionState new_state) override; void OnConnectionChange( PeerConnectionInterface::PeerConnectionState new_state) override; void OnIceConnectionReceivingChange(bool receiving) override; diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection+Private.h b/sdk/objc/api/peerconnection/RTCPeerConnection+Private.h index 3de6a9b794..b2c6ba892d 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection+Private.h +++ b/sdk/objc/api/peerconnection/RTCPeerConnection+Private.h @@ -39,6 +39,9 @@ class PeerConnectionDelegateAdapter : public PeerConnectionObserver { void OnIceConnectionChange(PeerConnectionInterface::IceConnectionState new_state) override; + void OnStandardizedIceConnectionChange( + PeerConnectionInterface::IceConnectionState new_state) override; + void OnConnectionChange(PeerConnectionInterface::PeerConnectionState new_state) override; void OnIceGatheringChange(PeerConnectionInterface::IceGatheringState new_state) override; diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.h b/sdk/objc/api/peerconnection/RTCPeerConnection.h index c641fdd545..6956555ce3 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection.h +++ b/sdk/objc/api/peerconnection/RTCPeerConnection.h @@ -126,6 +126,11 @@ RTC_OBJC_EXPORT * This is only called with RTCSdpSemanticsUnifiedPlan specified. */ @optional +/** Called any time the IceConnectionState changes following standardized + * transition. */ +- (void)peerConnection:(RTCPeerConnection *)peerConnection + didChangeStandardizedIceConnectionState:(RTCIceConnectionState)newState; + /** Called any time the PeerConnectionState changes. */ - (void)peerConnection:(RTCPeerConnection *)peerConnection didChangeConnectionState:(RTCPeerConnectionState)newState; diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm index 8d1ac46d06..8189aecd34 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm @@ -179,6 +179,16 @@ void PeerConnectionDelegateAdapter::OnIceConnectionChange( [peer_connection_.delegate peerConnection:peer_connection_ didChangeIceConnectionState:state]; } +void PeerConnectionDelegateAdapter::OnStandardizedIceConnectionChange( + PeerConnectionInterface::IceConnectionState new_state) { + if ([peer_connection_.delegate + respondsToSelector:@selector(peerConnection:didChangeStandardizedIceConnectionState:)]) { + RTCIceConnectionState state = [RTCPeerConnection iceConnectionStateForNativeState:new_state]; + [peer_connection_.delegate peerConnection:peer_connection_ + didChangeStandardizedIceConnectionState:state]; + } +} + void PeerConnectionDelegateAdapter::OnConnectionChange( PeerConnectionInterface::PeerConnectionState new_state) { if ([peer_connection_.delegate