diff --git a/pc/jseptransport2.cc b/pc/jseptransport2.cc index a4a03487bd..a86a37a452 100644 --- a/pc/jseptransport2.cc +++ b/pc/jseptransport2.cc @@ -253,12 +253,18 @@ webrtc::RTCError JsepTransport2::SetRemoteJsepTransportDescription( webrtc::RTCError JsepTransport2::AddRemoteCandidates( const Candidates& candidates) { - if (!local_description_ || !remote_description_) { + // Need a remote description for the remote ICE credentials; can't send + // connectivity checks without them. + // + // Assuming the application's signaling mechanism maintains ordering, it + // shouldn't be possible to hit this error; session descriptions are + // generated before candidates, so they should be received before candidates + // as well. + if (!remote_description_) { return webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE, mid() + " is not ready to use the remote candidate " - "because the local or remote description is " - "not set."); + "because the remote description is not set."); } for (const cricket::Candidate& candidate : candidates) { diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index cce90216c1..b3a2c45606 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -5340,25 +5340,26 @@ bool PeerConnection::UseCandidate(const IceCandidateInterface* candidate) { // Invoking BaseSession method to handle remote candidates. RTCError error = transport_controller_->AddRemoteCandidates(content.name, candidates); - if (error.ok()) { - // Candidates successfully submitted for checking. - if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew || - ice_connection_state_ == - PeerConnectionInterface::kIceConnectionDisconnected) { - // If state is New, then the session has just gotten its first remote ICE - // candidates, so go to Checking. - // If state is Disconnected, the session is re-using old candidates or - // receiving additional ones, so go to Checking. - // If state is Connected, stay Connected. - // TODO(bemasc): If state is Connected, and the new candidates are for a - // newly added transport, then the state actually _should_ move to - // checking. Add a way to distinguish that case. - SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking); - } - // TODO(bemasc): If state is Completed, go back to Connected. - } else if (error.message()) { + if (!error.ok()) { RTC_LOG(LS_WARNING) << error.message(); + return false; } + + // Candidates successfully submitted for checking. + if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew || + ice_connection_state_ == + PeerConnectionInterface::kIceConnectionDisconnected) { + // If state is New, then the session has just gotten its first remote ICE + // candidates, so go to Checking. + // If state is Disconnected, the session is re-using old candidates or + // receiving additional ones, so go to Checking. + // If state is Connected, stay Connected. + // TODO(bemasc): If state is Connected, and the new candidates are for a + // newly added transport, then the state actually _should_ move to + // checking. Add a way to distinguish that case. + SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking); + } + // TODO(bemasc): If state is Completed, go back to Connected. return true; } diff --git a/pc/peerconnection_ice_unittest.cc b/pc/peerconnection_ice_unittest.cc index e8f450fe9f..c19e31d0a1 100644 --- a/pc/peerconnection_ice_unittest.cc +++ b/pc/peerconnection_ice_unittest.cc @@ -398,6 +398,25 @@ TEST_P(PeerConnectionIceTest, CannotAddCandidateWhenRemoteDescriptionNotSet) { EXPECT_FALSE(caller->pc()->AddIceCandidate(&jsep_candidate)); } +// Should be able to add a candidate immediately after setting a remote offer, +// even if an answer hasn't been created yet. +TEST_P(PeerConnectionIceTest, CanAddCandidateWhenLocalDescriptionNotSet) { + const SocketAddress kCallerAddress("1.1.1.1", 0); + + auto caller = CreatePeerConnectionWithAudioVideo(); + auto callee = CreatePeerConnectionWithAudioVideo(); + caller->network()->AddInterface(kCallerAddress); + + // Apply offer, but not answer. + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + + // Wait for gathering to finish and try adding a candidate from the caller. + EXPECT_TRUE_WAIT(caller->IsIceGatheringDone(), kIceCandidatesTimeout); + ASSERT_LT(0u, caller->observer()->GetCandidatesByMline(0).size()); + EXPECT_TRUE(callee->pc()->AddIceCandidate( + caller->observer()->GetCandidatesByMline(0)[0])); +} + TEST_P(PeerConnectionIceTest, DuplicateIceCandidateIgnoredWhenAdded) { const SocketAddress kCalleeAddress("1.1.1.1", 1111);