Add static AsString functions for PeerConnectionInterface enums

Changes one preexisting enum-to-string function to use the new format.

Also changes the RTC_LOG macros that created collisions with ToString,
for tidiness, and documents the recommended function form.

Bug: webrtc:13272
Change-Id: Ic8bb54ed31402ba32675b142d796cf276ee78df5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235722
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35296}

api/peer_connection_interface.h

@@ -76,6 +76,7 @@
 #include <vector>
 
 #include "absl/base/attributes.h"
+#include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 #include "api/adaptation/resource.h"
 #include "api/async_dns_resolver.h"
@@ -180,6 +181,7 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
     kHaveRemotePrAnswer,
     kClosed,
   };
+  static constexpr absl::string_view AsString(SignalingState);
 
   // See https://w3c.github.io/webrtc-pc/#dom-rtcicegatheringstate
   enum IceGatheringState {
@@ -187,6 +189,7 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
     kIceGatheringGathering,
     kIceGatheringComplete
   };
+  static constexpr absl::string_view AsString(IceGatheringState state);
 
   // See https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnectionstate
   enum class PeerConnectionState {
@@ -197,6 +200,7 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
     kFailed,
     kClosed,
   };
+  static constexpr absl::string_view AsString(PeerConnectionState state);
 
   // See https://w3c.github.io/webrtc-pc/#dom-rtciceconnectionstate
   enum IceConnectionState {
@@ -209,6 +213,7 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
     kIceConnectionClosed,
     kIceConnectionMax,
   };
+  static constexpr absl::string_view AsString(IceConnectionState state);
 
   // TLS certificate policy.
   enum TlsCertPolicy {
@@ -1563,6 +1568,88 @@ RTC_EXPORT rtc::scoped_refptr<PeerConnectionFactoryInterface>
 CreateModularPeerConnectionFactory(
     PeerConnectionFactoryDependencies dependencies);
 
+// https://w3c.github.io/webrtc-pc/#dom-rtcsignalingstate
+inline constexpr absl::string_view PeerConnectionInterface::AsString(
+    SignalingState state) {
+  switch (state) {
+    case SignalingState::kStable:
+      return "stable";
+    case SignalingState::kHaveLocalOffer:
+      return "have-local-offer";
+    case SignalingState::kHaveLocalPrAnswer:
+      return "have-local-pranswer";
+    case SignalingState::kHaveRemoteOffer:
+      return "have-remote-offer";
+    case SignalingState::kHaveRemotePrAnswer:
+      return "have-remote-pranswer";
+    case SignalingState::kClosed:
+      return "closed";
+  }
+  RTC_CHECK_NOTREACHED();
+  return "";
+}
+
+// https://w3c.github.io/webrtc-pc/#dom-rtcicegatheringstate
+inline constexpr absl::string_view PeerConnectionInterface::AsString(
+    IceGatheringState state) {
+  switch (state) {
+    case IceGatheringState::kIceGatheringNew:
+      return "new";
+    case IceGatheringState::kIceGatheringGathering:
+      return "gathering";
+    case IceGatheringState::kIceGatheringComplete:
+      return "complete";
+  }
+  RTC_CHECK_NOTREACHED();
+  return "";
+}
+
+// https://w3c.github.io/webrtc-pc/#dom-rtciceconnectionstate
+inline constexpr absl::string_view PeerConnectionInterface::AsString(
+    PeerConnectionState state) {
+  switch (state) {
+    case PeerConnectionState::kNew:
+      return "new";
+    case PeerConnectionState::kConnecting:
+      return "connecting";
+    case PeerConnectionState::kConnected:
+      return "connected";
+    case PeerConnectionState::kDisconnected:
+      return "disconnected";
+    case PeerConnectionState::kFailed:
+      return "failed";
+    case PeerConnectionState::kClosed:
+      return "closed";
+  }
+  RTC_CHECK_NOTREACHED();
+  return "";
+}
+
+inline constexpr absl::string_view PeerConnectionInterface::AsString(
+    IceConnectionState state) {
+  switch (state) {
+    case kIceConnectionNew:
+      return "new";
+    case kIceConnectionChecking:
+      return "checking";
+    case kIceConnectionConnected:
+      return "connected";
+    case kIceConnectionCompleted:
+      return "completed";
+    case kIceConnectionFailed:
+      return "failed";
+    case kIceConnectionDisconnected:
+      return "disconnected";
+    case kIceConnectionClosed:
+      return "closed";
+    case kIceConnectionMax:
+      RTC_CHECK_NOTREACHED();
+      return "";
+  }
+  RTC_CHECK_NOTREACHED();
+  return "";
+}
+
 }  // namespace webrtc
 
 #endif  // API_PEER_CONNECTION_INTERFACE_H_

api/rtc_error.h

@@ -178,11 +178,11 @@ inline std::ostream& operator<<(  // no-presubmit-check TODO(webrtc:8982)
 // Helper macro that can be used by implementations to create an error with a
 // message and log it. `message` should be a string literal or movable
 // std::string.
-#define LOG_AND_RETURN_ERROR_EX(type, message, severity)           \
-  {                                                                \
-    RTC_DCHECK(type != RTCErrorType::NONE);                        \
-    RTC_LOG(severity) << message << " (" << ToString(type) << ")"; \
-    return webrtc::RTCError(type, message);                        \
+#define LOG_AND_RETURN_ERROR_EX(type, message, severity)                     \
+  {                                                                          \
+    RTC_DCHECK(type != RTCErrorType::NONE);                                  \
+    RTC_LOG(severity) << message << " (" << ::webrtc::ToString(type) << ")"; \
+    return ::webrtc::RTCError(type, message);                                \
   }
 
 #define LOG_AND_RETURN_ERROR(type, message) \

g3doc/implementation_basics.md

@@ -82,7 +82,15 @@ in the (slow) process of being removed from the codebase.
 
 * RecursiveCriticalSection. Try to use [webrtc::Mutex][6] instead, and don't recurse.
 
+## Enum-To-String functions
+If there is a need to convert an enum to a string representation, such as for
+enums exposed at the Javascript API interface, the recommended way is to write
+a function named AsString, declared "static constexpr" and returning an
+absl::string_view. The declaration should be right after the enum declaration,
+in the same scope; the implementation (which must be marked "inline") should
+be at the end of the same header file.
 
+If the enum is not defined within a class, the "static" keyword is not needed.
 
 [1]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/api/units/timestamp.h;drc=b95d90b78a3491ef8e8aa0640dd521515ec881ca;l=29
 [2]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/rtc_base/thread.h;drc=1107751b6f11c35259a1c5c8a0f716e227b7e3b4;l=194

pc/sdp_offer_answer.cc

@@ -468,28 +468,6 @@ bool ValidateOfferAnswerOptions(
          IsValidOfferToReceiveMedia(rtc_options.offer_to_receive_video);
 }
 
-// Map internal signaling state name to spec name:
-//  https://w3c.github.io/webrtc-pc/#rtcsignalingstate-enum
-std::string GetSignalingStateString(
-    PeerConnectionInterface::SignalingState state) {
-  switch (state) {
-    case PeerConnectionInterface::kStable:
-      return "stable";
-    case PeerConnectionInterface::kHaveLocalOffer:
-      return "have-local-offer";
-    case PeerConnectionInterface::kHaveLocalPrAnswer:
-      return "have-local-pranswer";
-    case PeerConnectionInterface::kHaveRemoteOffer:
-      return "have-remote-offer";
-    case PeerConnectionInterface::kHaveRemotePrAnswer:
-      return "have-remote-pranswer";
-    case PeerConnectionInterface::kClosed:
-      return "closed";
-  }
-  RTC_NOTREACHED();
-  return "";
-}
-
 // This method will extract any send encodings that were sent by the remote
 // connection. This is currently only relevant for Simulcast scenario (where
 // the number of layers may be communicated by the server).
@@ -2477,9 +2455,9 @@ void SdpOfferAnswerHandler::ChangeSignalingState(
     return;
   }
   RTC_LOG(LS_INFO) << "Session: " << pc_->session_id() << " Old state: "
-                   << GetSignalingStateString(signaling_state_)
+                   << PeerConnectionInterface::AsString(signaling_state_)
                    << " New state: "
-                   << GetSignalingStateString(signaling_state);
+                   << PeerConnectionInterface::AsString(signaling_state);
   signaling_state_ = signaling_state;
   pc_->Observer()->OnSignalingChange(signaling_state_);
 }
@@ -2694,8 +2672,9 @@ RTCError SdpOfferAnswerHandler::Rollback(SdpType desc_type) {
   if (state != PeerConnectionInterface::kHaveLocalOffer &&
       state != PeerConnectionInterface::kHaveRemoteOffer) {
     return RTCError(RTCErrorType::INVALID_STATE,
-                    "Called in wrong signalingState: " +
-                        GetSignalingStateString(signaling_state()));
+                    (rtc::StringBuilder("Called in wrong signalingState: ")
+                     << (PeerConnectionInterface::AsString(signaling_state())))
+                        .Release());
   }
   RTC_DCHECK_RUN_ON(signaling_thread());
   RTC_DCHECK(IsUnifiedPlan());
@@ -3050,7 +3029,9 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription(
       (source == cricket::CS_REMOTE && !ExpectSetRemoteDescription(type))) {
     LOG_AND_RETURN_ERROR(
         RTCErrorType::INVALID_STATE,
-        "Called in wrong state: " + GetSignalingStateString(signaling_state()));
+        (rtc::StringBuilder("Called in wrong state: ")
+         << PeerConnectionInterface::AsString(signaling_state()))
+            .Release());
   }
 
   RTCError error = ValidateMids(*sdesc->description());