Make AddIceCandidate's error type match the spec.

See https://crbug.com/webrtc/4409.

Bug: webrtc:4409
Change-Id: I4249444a385ac7c4b3da88125a0d7c88a88bceeb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248143
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35767}
This commit is contained in:
Henrik Boström 2022-01-21 15:18:08 +01:00 committed by WebRTC LUCI CQ
parent 5411b174c8
commit 347488e450
2 changed files with 37 additions and 13 deletions

View File

@ -800,7 +800,7 @@ TEST_P(PeerConnectionIceTest,
std::move(jsep_candidate), [&operation_completed](RTCError result) {
EXPECT_FALSE(result.ok());
EXPECT_EQ(result.message(),
std::string("Error processing ICE candidate"));
std::string("The remote description was null"));
operation_completed = true;
});
EXPECT_TRUE_WAIT(operation_completed, kWaitTimeout);

View File

@ -2497,18 +2497,42 @@ void SdpOfferAnswerHandler::AddIceCandidate(
: kAddIceCandidateFailClosed;
NoteAddIceCandidateResult(result);
operations_chain_callback();
if (result == kAddIceCandidateFailClosed) {
callback(RTCError(
RTCErrorType::INVALID_STATE,
"AddIceCandidate failed because the session was shut down"));
} else if (result != kAddIceCandidateSuccess &&
result != kAddIceCandidateFailNotReady) {
// Fail with an error type and message consistent with Chromium.
// TODO(hbos): Fail with error types according to spec.
callback(RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
"Error processing ICE candidate"));
} else {
callback(RTCError::OK());
switch (result) {
case AddIceCandidateResult::kAddIceCandidateSuccess:
case AddIceCandidateResult::kAddIceCandidateFailNotReady:
// Success!
callback(RTCError::OK());
break;
case AddIceCandidateResult::kAddIceCandidateFailClosed:
// Note that the spec says to just abort without resolving the
// promise in this case, but this layer must return an RTCError.
callback(RTCError(
RTCErrorType::INVALID_STATE,
"AddIceCandidate failed because the session was shut down"));
break;
case AddIceCandidateResult::kAddIceCandidateFailNoRemoteDescription:
// Spec: "If remoteDescription is null return a promise rejected
// with a newly created InvalidStateError."
callback(RTCError(RTCErrorType::INVALID_STATE,
"The remote description was null"));
break;
case AddIceCandidateResult::kAddIceCandidateFailNullCandidate:
// TODO(https://crbug.com/935898): Handle end-of-candidates instead
// of treating null candidate as an error.
callback(RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
"Error processing ICE candidate"));
break;
case AddIceCandidateResult::kAddIceCandidateFailNotValid:
case AddIceCandidateResult::kAddIceCandidateFailInAddition:
case AddIceCandidateResult::kAddIceCandidateFailNotUsable:
// Spec: "If candidate could not be successfully added [...] Reject
// p with a newly created OperationError and abort these steps."
// UNSUPPORTED_OPERATION maps to OperationError.
callback(RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
"Error processing ICE candidate"));
break;
default:
RTC_DCHECK_NOTREACHED();
}
});
}