Move the ICE state transition ASSERTS to a lower level.

webrtc/api/webrtcsession.cc

Index: webrtc/api/webrtcsession.cc
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 80352cf83246f3ff5f42a4bfa4be64f496a5e153..bcb6a1ad371c3e3bbf469531223e059c1e3836af 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -1466,46 +1466,8 @@ void WebRtcSession::SetIceConnectionState(
     return;
   }
 
-  // ASSERT that the requested transition is allowed.  Note that
-  // WebRtcSession does not implement "kIceConnectionClosed" (that is handled
-  // within PeerConnection).  This switch statement should compile away when
-  // ASSERTs are disabled.
   LOG(LS_INFO) << "Changing IceConnectionState " << ice_connection_state_
                << " => " << state;
-  switch (ice_connection_state_) {
-    case PeerConnectionInterface::kIceConnectionNew:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionChecking);
-      break;
-    case PeerConnectionInterface::kIceConnectionChecking:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionFailed ||
-             state == PeerConnectionInterface::kIceConnectionConnected);
-      break;
-    case PeerConnectionInterface::kIceConnectionConnected:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionDisconnected ||
-             state == PeerConnectionInterface::kIceConnectionChecking ||
-             state == PeerConnectionInterface::kIceConnectionCompleted);
-      break;
-    case PeerConnectionInterface::kIceConnectionCompleted:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionConnected ||
-             state == PeerConnectionInterface::kIceConnectionDisconnected);
-      break;
-    case PeerConnectionInterface::kIceConnectionFailed:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionNew);
-      break;
-    case PeerConnectionInterface::kIceConnectionDisconnected:
-      ASSERT(state == PeerConnectionInterface::kIceConnectionChecking ||
-             state == PeerConnectionInterface::kIceConnectionConnected ||
-             state == PeerConnectionInterface::kIceConnectionCompleted ||
-             state == PeerConnectionInterface::kIceConnectionFailed);
-      break;
-    case PeerConnectionInterface::kIceConnectionClosed:
-      ASSERT(false);
-      break;
-    default:
-      ASSERT(false);
-      break;

[pthatcher1, 2016/05/25 17:32:47]

With this move, don't we lose the check against the states changing because of
the aggregate of many transports channel states?  For example, if a transport
channel is added or removed, the state may change in a way that we've been
checking with an assert here, but which wouldn't be caught by an assertion in
the lower-level code.

[Taylor Brandstetter, 2016/05/25 20:54:09]

That's part of the point of this CL. Any state transition is possible for the
aggregate state (except closed -> something else) and so the ASSERTs here are
invalid.

On that note, I might as well add back an assert that the current state is not
Closed.

-  }
-
   ice_connection_state_ = state;
   if (ice_observer_) {
     ice_observer_->OnIceConnectionChange(ice_connection_state_);