Remove kUnknown as a possible value for IceCandidateType.

Subsequently also tighten IceCandidateType error checking.
The Candidate type in `cricket` should be using something similar
(currently using a string for the type), so I'm making sure that
types that we have already, align with where we'd like to be overall.
Possibly we can move IceCandidateType to where Candidate is defined.

Bug: none
Change-Id: Iffeba7268f2a393e18a5f33249efae46e6e08252
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335980
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41640}

logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc

@@ -131,8 +131,6 @@ ConvertIceCandidatePairConfigType(IceCandidatePairConfigType type) {
 rtclog::IceCandidatePairConfig::IceCandidateType ConvertIceCandidateType(
     IceCandidateType type) {
   switch (type) {
-    case IceCandidateType::kUnknown:
-      return rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE;
     case IceCandidateType::kLocal:
       return rtclog::IceCandidatePairConfig::LOCAL;
     case IceCandidateType::kStun:
@@ -141,11 +139,11 @@ rtclog::IceCandidatePairConfig::IceCandidateType ConvertIceCandidateType(
       return rtclog::IceCandidatePairConfig::PRFLX;
     case IceCandidateType::kRelay:
       return rtclog::IceCandidatePairConfig::RELAY;
-    case IceCandidateType::kNumValues:
+    default:
+      // TODO(tommi): Remove the default handler when kNumValues is gone.
       RTC_DCHECK_NOTREACHED();
+      return rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE;
   }
-  RTC_DCHECK_NOTREACHED();
-  return rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE;
 }
 
 rtclog::IceCandidatePairConfig::Protocol ConvertIceCandidatePairProtocol(

logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc

@@ -198,8 +198,6 @@ ConvertToProtoFormat(IceCandidatePairConfigType type) {
 rtclog2::IceCandidatePairConfig::IceCandidateType ConvertToProtoFormat(
     IceCandidateType type) {
   switch (type) {
-    case IceCandidateType::kUnknown:
-      return rtclog2::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE;
     case IceCandidateType::kLocal:
       return rtclog2::IceCandidatePairConfig::LOCAL;
     case IceCandidateType::kStun:

logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.cc

@@ -14,12 +14,14 @@
 
 namespace webrtc {
 
-IceCandidatePairDescription::IceCandidatePairDescription() {
+IceCandidatePairDescription::IceCandidatePairDescription(
-  local_candidate_type = IceCandidateType::kUnknown;
+    IceCandidateType local_candidate_type,
+    IceCandidateType remote_candidate_type)
+    : local_candidate_type(local_candidate_type),
+      remote_candidate_type(remote_candidate_type) {
   local_relay_protocol = IceCandidatePairProtocol::kUnknown;
   local_network_type = IceCandidateNetworkType::kUnknown;
   local_address_family = IceCandidatePairAddressFamily::kUnknown;
-  remote_candidate_type = IceCandidateType::kUnknown;
   remote_address_family = IceCandidatePairAddressFamily::kUnknown;
   candidate_pair_protocol = IceCandidatePairProtocol::kUnknown;
 }

logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h

@@ -35,7 +35,6 @@ enum class IceCandidatePairConfigType {
 // TODO(qingsi): Change the names of candidate types to "host", "srflx", "prflx"
 // and "relay" after the naming is spec-compliant in the signaling part
 enum class IceCandidateType {
-  kUnknown,
   kLocal,
   kStun,
   kPrflx,
@@ -88,7 +87,8 @@ struct LoggedIceCandidatePairConfig {
 
 class IceCandidatePairDescription {
  public:
-  IceCandidatePairDescription();
+  IceCandidatePairDescription(IceCandidateType local_candidate_type,
+                              IceCandidateType remote_candidate_type);
   explicit IceCandidatePairDescription(
       const IceCandidatePairDescription& other);
 

logging/rtc_event_log/ice_logger.cc

@@ -26,7 +26,8 @@ void IceEventLog::LogCandidatePairConfig(
   if (event_log_ == nullptr) {
     return;
   }
-  candidate_pair_desc_by_id_[candidate_pair_id] = candidate_pair_desc;
+
+  candidate_pair_desc_by_id_.emplace(candidate_pair_id, candidate_pair_desc);
   event_log_->Log(std::make_unique<RtcEventIceCandidatePairConfig>(
       type, candidate_pair_id, candidate_pair_desc));
 }

logging/rtc_event_log/rtc_event_log_parser.cc

@@ -163,22 +163,29 @@ IceCandidatePairConfigType GetRuntimeIceCandidatePairConfigType(
   return IceCandidatePairConfigType::kAdded;
 }
 
-IceCandidateType GetRuntimeIceCandidateType(
+// Converts a log type (proto based) to a matching `IceCandidateType` value
-    rtclog::IceCandidatePairConfig::IceCandidateType type) {
+// and checks for validity of the log type (since the enums aren't a perfect
-  switch (type) {
+// match).
+bool GetRuntimeIceCandidateType(
+    rtclog::IceCandidatePairConfig::IceCandidateType log_type,
+    IceCandidateType& parsed_type) {
+  switch (log_type) {
     case rtclog::IceCandidatePairConfig::LOCAL:
-      return IceCandidateType::kLocal;
+      parsed_type = IceCandidateType::kLocal;
+      break;
     case rtclog::IceCandidatePairConfig::STUN:
-      return IceCandidateType::kStun;
+      parsed_type = IceCandidateType::kStun;
+      break;
     case rtclog::IceCandidatePairConfig::PRFLX:
-      return IceCandidateType::kPrflx;
+      parsed_type = IceCandidateType::kPrflx;
+      break;
     case rtclog::IceCandidatePairConfig::RELAY:
-      return IceCandidateType::kRelay;
+      parsed_type = IceCandidateType::kRelay;
-    case rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE:
+      break;
-      return IceCandidateType::kUnknown;
+    default:
+      return false;
   }
-  RTC_DCHECK_NOTREACHED();
+  return true;
-  return IceCandidateType::kUnknown;
 }
 
 IceCandidatePairProtocol GetRuntimeIceCandidatePairProtocol(
@@ -813,11 +820,32 @@ IceCandidateType GetRuntimeIceCandidateType(
       return IceCandidateType::kPrflx;
     case rtclog2::IceCandidatePairConfig::RELAY:
       return IceCandidateType::kRelay;
-    case rtclog2::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE:
+    default:
-      return IceCandidateType::kUnknown;
+      RTC_DCHECK_NOTREACHED();
+      return IceCandidateType::kLocal;
   }
-  RTC_DCHECK_NOTREACHED();
+}
-  return IceCandidateType::kUnknown;
+
+bool GetRuntimeIceCandidateType(
+    rtclog2::IceCandidatePairConfig::IceCandidateType log_type,
+    IceCandidateType& parsed_type) {
+  switch (log_type) {
+    case rtclog2::IceCandidatePairConfig::LOCAL:
+      parsed_type = IceCandidateType::kLocal;
+      break;
+    case rtclog2::IceCandidatePairConfig::STUN:
+      parsed_type = IceCandidateType::kStun;
+      break;
+    case rtclog2::IceCandidatePairConfig::PRFLX:
+      parsed_type = IceCandidateType::kPrflx;
+      break;
+    case rtclog2::IceCandidatePairConfig::RELAY:
+      parsed_type = IceCandidateType::kRelay;
+      break;
+    default:
+      return false;
+  }
+  return true;
 }
 
 IceCandidatePairProtocol GetRuntimeIceCandidatePairProtocol(
@@ -2142,8 +2170,8 @@ ParsedRtcEventLog::GetIceCandidatePairConfig(
   RTC_PARSE_CHECK_OR_RETURN(config.has_candidate_pair_id());
   res.candidate_pair_id = config.candidate_pair_id();
   RTC_PARSE_CHECK_OR_RETURN(config.has_local_candidate_type());
-  res.local_candidate_type =
+  RTC_PARSE_CHECK_OR_RETURN(GetRuntimeIceCandidateType(
-      GetRuntimeIceCandidateType(config.local_candidate_type());
+      config.local_candidate_type(), res.local_candidate_type));
   RTC_PARSE_CHECK_OR_RETURN(config.has_local_relay_protocol());
   res.local_relay_protocol =
       GetRuntimeIceCandidatePairProtocol(config.local_relay_protocol());
@@ -2154,8 +2182,8 @@ ParsedRtcEventLog::GetIceCandidatePairConfig(
   res.local_address_family =
       GetRuntimeIceCandidatePairAddressFamily(config.local_address_family());
   RTC_PARSE_CHECK_OR_RETURN(config.has_remote_candidate_type());
-  res.remote_candidate_type =
+  RTC_PARSE_CHECK_OR_RETURN(GetRuntimeIceCandidateType(
-      GetRuntimeIceCandidateType(config.remote_candidate_type());
+      config.remote_candidate_type(), res.remote_candidate_type));
   RTC_PARSE_CHECK_OR_RETURN(config.has_remote_address_family());
   res.remote_address_family =
       GetRuntimeIceCandidatePairAddressFamily(config.remote_address_family());
@@ -3498,8 +3526,8 @@ ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::StoreIceCandidatePairConfig(
   RTC_PARSE_CHECK_OR_RETURN(proto.has_candidate_pair_id());
   ice_config.candidate_pair_id = proto.candidate_pair_id();
   RTC_PARSE_CHECK_OR_RETURN(proto.has_local_candidate_type());
-  ice_config.local_candidate_type =
+  RTC_PARSE_CHECK_OR_RETURN(GetRuntimeIceCandidateType(
-      GetRuntimeIceCandidateType(proto.local_candidate_type());
+      proto.local_candidate_type(), ice_config.local_candidate_type));
   RTC_PARSE_CHECK_OR_RETURN(proto.has_local_relay_protocol());
   ice_config.local_relay_protocol =
       GetRuntimeIceCandidatePairProtocol(proto.local_relay_protocol());
@@ -3510,8 +3538,8 @@ ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::StoreIceCandidatePairConfig(
   ice_config.local_address_family =
       GetRuntimeIceCandidatePairAddressFamily(proto.local_address_family());
   RTC_PARSE_CHECK_OR_RETURN(proto.has_remote_candidate_type());
-  ice_config.remote_candidate_type =
+  RTC_PARSE_CHECK_OR_RETURN(GetRuntimeIceCandidateType(
-      GetRuntimeIceCandidateType(proto.remote_candidate_type());
+      proto.remote_candidate_type(), ice_config.remote_candidate_type));
   RTC_PARSE_CHECK_OR_RETURN(proto.has_remote_address_family());
   ice_config.remote_address_family =
       GetRuntimeIceCandidatePairAddressFamily(proto.remote_address_family());

logging/rtc_event_log/rtc_event_log_unittest_helper.cc

@@ -238,12 +238,10 @@ EventGenerator::NewIceCandidatePairConfig() {
       static_cast<IceCandidatePairProtocol>(prng_.Rand(
           static_cast<uint32_t>(IceCandidatePairProtocol::kNumValues) - 1));
 
-  IceCandidatePairDescription desc;
+  IceCandidatePairDescription desc(local_candidate_type, remote_candidate_type);
-  desc.local_candidate_type = local_candidate_type;
   desc.local_relay_protocol = protocol_type;
   desc.local_network_type = local_network_type;
   desc.local_address_family = local_address_family;
-  desc.remote_candidate_type = remote_candidate_type;
   desc.remote_address_family = remote_address_family;
   desc.candidate_pair_protocol = protocol_type;
 

p2p/base/connection.cc

@@ -82,10 +82,9 @@ webrtc::IceCandidateType GetRtcEventLogCandidateType(const Candidate& c) {
     return webrtc::IceCandidateType::kStun;
   } else if (c.is_prflx()) {
     return webrtc::IceCandidateType::kPrflx;
-  } else if (c.is_relay()) {
-    return webrtc::IceCandidateType::kRelay;
   }
-  return webrtc::IceCandidateType::kUnknown;
+  RTC_DCHECK(c.is_relay());
+  return webrtc::IceCandidateType::kRelay;
 }
 
 webrtc::IceCandidatePairProtocol GetProtocolByString(
@@ -1366,14 +1365,13 @@ const webrtc::IceCandidatePairDescription& Connection::ToLogDescription() {
   const Candidate& local = local_candidate();
   const Candidate& remote = remote_candidate();
   const rtc::Network* network = port()->Network();
-  log_description_ = webrtc::IceCandidatePairDescription();
+  log_description_ = webrtc::IceCandidatePairDescription(
-  log_description_->local_candidate_type = GetRtcEventLogCandidateType(local);
+      GetRtcEventLogCandidateType(local), GetRtcEventLogCandidateType(remote));
   log_description_->local_relay_protocol =
       GetProtocolByString(local.relay_protocol());
   log_description_->local_network_type = ConvertNetworkType(network->type());
   log_description_->local_address_family =
       GetAddressFamilyByInt(local.address().family());
-  log_description_->remote_candidate_type = GetRtcEventLogCandidateType(remote);
   log_description_->remote_address_family =
       GetAddressFamilyByInt(remote.address().family());
   log_description_->candidate_pair_protocol =

rtc_tools/rtc_event_log_to_text/converter.cc

@@ -231,8 +231,7 @@ bool Convert(std::string inputfile,
                 {IceCandidatePairConfigType::kNumValues, "NUM_VALUES"}};
 
         static const std::map<IceCandidateType, std::string>
-            candidate_type_name{{IceCandidateType::kUnknown, "UNKNOWN"},
+            candidate_type_name{{IceCandidateType::kLocal, "LOCAL"},
-                                {IceCandidateType::kLocal, "LOCAL"},
                                 {IceCandidateType::kStun, "STUN"},
                                 {IceCandidateType::kPrflx, "PRFLX"},
                                 {IceCandidateType::kRelay, "RELAY"},

rtc_tools/rtc_event_log_visualizer/analyzer.cc

@@ -238,7 +238,9 @@ TimeSeries CreateRtcpTypeTimeSeries(const std::vector<T>& rtcp_list,
 
 const char kUnknownEnumValue[] = "unknown";
 
+// TODO(tommi): This should be "host".
 const char kIceCandidateTypeLocal[] = "local";
+// TODO(tommi): This should be "srflx".
 const char kIceCandidateTypeStun[] = "stun";
 const char kIceCandidateTypePrflx[] = "prflx";
 const char kIceCandidateTypeRelay[] = "relay";
@@ -257,17 +259,18 @@ const char kNetworkTypeWifi[] = "wifi";
 const char kNetworkTypeVpn[] = "vpn";
 const char kNetworkTypeCellular[] = "cellular";
 
-std::string GetIceCandidateTypeAsString(webrtc::IceCandidateType type) {
+absl::string_view GetIceCandidateTypeAsString(IceCandidateType type) {
   switch (type) {
-    case webrtc::IceCandidateType::kLocal:
+    case IceCandidateType::kLocal:
       return kIceCandidateTypeLocal;
-    case webrtc::IceCandidateType::kStun:
+    case IceCandidateType::kStun:
       return kIceCandidateTypeStun;
-    case webrtc::IceCandidateType::kPrflx:
+    case IceCandidateType::kPrflx:
       return kIceCandidateTypePrflx;
-    case webrtc::IceCandidateType::kRelay:
+    case IceCandidateType::kRelay:
       return kIceCandidateTypeRelay;
     default:
+      RTC_DCHECK_NOTREACHED();
       return kUnknownEnumValue;
   }
 }
@@ -323,18 +326,15 @@ std::string GetCandidatePairLogDescriptionAsString(
   // and a remote relay candidate using TCP as the relay protocol on a cell
   // network, when the candidate pair communicates over UDP using IPv4.
   rtc::StringBuilder ss;
-  std::string local_candidate_type =
+  ss << GetIceCandidateTypeAsString(config.local_candidate_type);
-      GetIceCandidateTypeAsString(config.local_candidate_type);
+
-  std::string remote_candidate_type =
+  if (config.local_candidate_type == IceCandidateType::kRelay) {
-      GetIceCandidateTypeAsString(config.remote_candidate_type);
+    ss << "(" << GetProtocolAsString(config.local_relay_protocol) << ")";
-  if (config.local_candidate_type == webrtc::IceCandidateType::kRelay) {
-    local_candidate_type +=
-        "(" + GetProtocolAsString(config.local_relay_protocol) + ")";
   }
-  ss << local_candidate_type << ":"
+
-     << GetNetworkTypeAsString(config.local_network_type) << ":"
+  ss << ":" << GetNetworkTypeAsString(config.local_network_type) << ":"
      << GetAddressFamilyAsString(config.local_address_family) << "->"
-     << remote_candidate_type << ":"
+     << GetIceCandidateTypeAsString(config.remote_candidate_type) << ":"
      << GetAddressFamilyAsString(config.remote_address_family) << "@"
      << GetProtocolAsString(config.candidate_pair_protocol);
   return ss.Release();