Implement (mostly) standards-compliant RTCIceTransportState.

In order to correctly implement RTCPeerConnectionState and RTCIceConnectionState the ice transports need to support RTCIceTransportState.
This CL adds an implementation parallel to the current non-standard IceTransportState. It's not currently used anywhere. The old implementation will remain in place until we're ready to switch RTCIceConnectionState over.

Bug: webrtc:9308
Change-Id: I30e2bbb5b4fafa410261bcd9d5e3b76c03435feb
Reviewed-on: https://webrtc-review.googlesource.com/c/103220
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25078}

api/transport/BUILD.gn

@@ -19,6 +19,13 @@ rtc_source_set("bitrate_settings") {
   ]
 }
 
+rtc_source_set("enums") {
+  visibility = [ "*" ]
+  sources = [
+    "enums.h",
+  ]
+}
+
 rtc_static_library("network_control") {
   sources = [
     "network_control.h",

api/transport/enums.h

@@ -0,0 +1,32 @@
+/*
+ *  Copyright 2018 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef API_TRANSPORT_ENUMS_H_
+#define API_TRANSPORT_ENUMS_H_
+
+namespace webrtc {
+
+// See https://w3c.github.io/webrtc-pc/#rtcicetransportstate
+// Note that kFailed is currently not a terminal state, and a transport might
+// incorrectly be marked as failed while gathering candidates, see
+// bugs.webrtc.org/8833
+enum class IceTransportState {
+  kNew,
+  kChecking,
+  kConnected,
+  kCompleted,
+  kFailed,
+  kDisconnected,
+  kClosed,
+};
+
+}  // namespace webrtc
+
+#endif  // API_TRANSPORT_ENUMS_H_

p2p/BUILD.gn

@@ -86,6 +86,7 @@ rtc_static_library("rtc_p2p") {
   deps = [
     "../api:libjingle_peerconnection_api",
     "../api:ortc_api",
+    "../api/transport:enums",
     "../logging:ice_log",
     "../rtc_base:checks",
     "../rtc_base:rtc_base",

p2p/base/fakeicetransport.h

@@ -114,6 +114,10 @@ class FakeIceTransport : public IceTransportInternal {
     return IceTransportState::STATE_CONNECTING;
   }
 
+  webrtc::IceTransportState GetIceTransportState() const override {
+    return webrtc::IceTransportState::kConnected;
+  }
+
   void SetIceRole(IceRole role) override { role_ = role; }
   IceRole GetIceRole() const override { return role_; }
   void SetIceTiebreaker(uint64_t tiebreaker) override {

p2p/base/icetransportinternal.h

@@ -15,6 +15,7 @@
 #include <vector>
 
 #include "api/candidate.h"
+#include "api/transport/enums.h"
 #include "p2p/base/candidatepairinterface.h"
 #include "p2p/base/packettransportinternal.h"
 #include "p2p/base/port.h"
@@ -188,7 +189,10 @@ class IceTransportInternal : public rtc::PacketTransportInternal {
   IceTransportInternal();
   ~IceTransportInternal() override;
 
+  // TODO(bugs.webrtc.org/9308): Remove GetState once all uses have been
+  // migrated to GetIceTransportState.
   virtual IceTransportState GetState() const = 0;
+  virtual webrtc::IceTransportState GetIceTransportState() const = 0;
 
   virtual int component() const = 0;
 
@@ -258,8 +262,13 @@ class IceTransportInternal : public rtc::PacketTransportInternal {
   sigslot::signal1<IceTransportInternal*> SignalRoleConflict;
 
   // Emitted whenever the transport state changed.
+  // TODO(bugs.webrtc.org/9308): Remove once all uses have migrated to the new
+  // IceTransportState.
   sigslot::signal1<IceTransportInternal*> SignalStateChanged;
 
+  // Emitted whenever the new standards-compliant transport state changed.
+  sigslot::signal1<IceTransportInternal*> SignalIceTransportStateChanged;
+
   // Invoked when the transport is being destroyed.
   sigslot::signal1<IceTransportInternal*> SignalDestroyed;
 };

p2p/base/mockicetransport.h

@@ -47,6 +47,10 @@ class MockIceTransport : public IceTransportInternal {
   IceTransportState GetState() const override {
     return IceTransportState::STATE_INIT;
   }
+  webrtc::IceTransportState GetIceTransportState() const override {
+    return webrtc::IceTransportState::kNew;
+  }
+
   const std::string& transport_name() const override { return transport_name_; }
   int component() const override { return 0; }
   void SetIceRole(IceRole role) override {}

p2p/base/p2ptransportchannel.cc

@@ -323,6 +323,10 @@ IceTransportState P2PTransportChannel::GetState() const {
   return state_;
 }
 
+webrtc::IceTransportState P2PTransportChannel::GetIceTransportState() const {
+  return standardized_state_;
+}
+
 const std::string& P2PTransportChannel::transport_name() const {
   return transport_name_;
 }
@@ -387,6 +391,41 @@ IceTransportState P2PTransportChannel::ComputeState() const {
   return IceTransportState::STATE_COMPLETED;
 }
 
+// Compute the current RTCIceTransportState as described in
+// https://www.w3.org/TR/webrtc/#dom-rtcicetransportstate
+// TODO(bugs.webrtc.org/9308): Return IceTransportState::kDisconnected when it
+// makes sense.
+// TODO(bugs.webrtc.org/9218): Avoid prematurely signalling kFailed once we have
+// implemented end-of-candidates signalling.
+webrtc::IceTransportState P2PTransportChannel::ComputeIceTransportState()
+    const {
+  bool has_connection = false;
+  for (Connection* connection : connections_) {
+    if (connection->active()) {
+      has_connection = true;
+      break;
+    }
+  }
+
+  switch (gathering_state_) {
+    case kIceGatheringComplete:
+      if (has_connection)
+        return webrtc::IceTransportState::kCompleted;
+      else
+        return webrtc::IceTransportState::kFailed;
+    case kIceGatheringNew:
+      return webrtc::IceTransportState::kNew;
+    case kIceGatheringGathering:
+      if (has_connection)
+        return webrtc::IceTransportState::kConnected;
+      else
+        return webrtc::IceTransportState::kChecking;
+    default:
+      RTC_NOTREACHED();
+      return webrtc::IceTransportState::kFailed;
+  }
+}
+
 void P2PTransportChannel::SetIceParameters(const IceParameters& ice_params) {
   RTC_DCHECK(network_thread_ == rtc::Thread::Current());
   RTC_LOG(LS_INFO) << "Set ICE ufrag: " << ice_params.ufrag
@@ -1827,6 +1866,9 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
 // example, we call this at the end of SortConnectionsAndUpdateState.
 void P2PTransportChannel::UpdateState() {
   IceTransportState state = ComputeState();
+  webrtc::IceTransportState current_standardized_state =
+      ComputeIceTransportState();
+
   if (state_ != state) {
     RTC_LOG(LS_INFO) << ToString()
                      << ": Transport channel state changed from "
@@ -1868,6 +1910,10 @@ void P2PTransportChannel::UpdateState() {
     SignalStateChanged(this);
   }
 
+  if (standardized_state_ != current_standardized_state) {
+    standardized_state_ = current_standardized_state;
+    SignalIceTransportStateChanged(this);
+  }
   // If our selected connection is "presumed writable" (TURN-TURN with no
   // CreatePermission required), act like we're already writable to the upper
   // layers, so they can start media quicker.

p2p/base/p2ptransportchannel.h

@@ -91,6 +91,8 @@ class P2PTransportChannel : public IceTransportInternal {
 
   // From TransportChannelImpl:
   IceTransportState GetState() const override;
+  webrtc::IceTransportState GetIceTransportState() const override;
+
   const std::string& transport_name() const override;
   int component() const override;
   bool writable() const override;
@@ -243,7 +245,13 @@ class P2PTransportChannel : public IceTransportInternal {
   void UpdateState();
   void HandleAllTimedOut();
   void MaybeStopPortAllocatorSessions();
+
+  // ComputeIceTransportState computes the RTCIceTransportState as described in
+  // https://w3c.github.io/webrtc-pc/#dom-rtcicetransportstate. ComputeState
+  // computes the value we currently export as RTCIceTransportState.
+  // TODO(bugs.webrtc.org/9308): Remove ComputeState once it's no longer used.
   IceTransportState ComputeState() const;
+  webrtc::IceTransportState ComputeIceTransportState() const;
 
   Connection* GetBestConnectionOnNetwork(rtc::Network* network) const;
   bool CreateConnections(const Candidate& remote_candidate,
@@ -407,7 +415,11 @@ class P2PTransportChannel : public IceTransportInternal {
   std::unique_ptr<webrtc::BasicRegatheringController> regathering_controller_;
   int64_t last_ping_sent_ms_ = 0;
   int weak_ping_interval_ = WEAK_PING_INTERVAL;
+  // TODO(jonasolsson): Remove state_ and rename standardized_state_ once state_
+  // is no longer used to compute the ICE connection state.
   IceTransportState state_ = IceTransportState::STATE_INIT;
+  webrtc::IceTransportState standardized_state_ =
+      webrtc::IceTransportState::kNew;
   IceConfig config_;
   int last_sent_packet_id_ = -1;  // -1 indicates no packet was sent before.
   bool started_pinging_ = false;

p2p/base/p2ptransportchannel_unittest.cc

@@ -4137,6 +4137,7 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
   clock.AdvanceTime(webrtc::TimeDelta::seconds(1));
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
+  EXPECT_EQ(webrtc::IceTransportState::kNew, ch.GetIceTransportState());
   PrepareChannel(&ch);
   ch.MaybeStartGathering();
   EXPECT_EQ(IceTransportState::STATE_INIT, ch.GetState());
@@ -4144,6 +4145,8 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
   Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock);
+  // Gathering complete with candidates.
+  EXPECT_EQ(webrtc::IceTransportState::kCompleted, ch.GetIceTransportState());
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
   // Now there are two connections, so the transport channel is connecting.
@@ -4156,6 +4159,7 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
   // Need to wait until the channel state is updated.
   EXPECT_EQ_SIMULATED_WAIT(IceTransportState::STATE_FAILED, ch.GetState(),
                            kShortTimeout, clock);
+  EXPECT_EQ(webrtc::IceTransportState::kFailed, ch.GetIceTransportState());
 }
 
 // Test that when a low-priority connection is pruned, it is not deleted