p2p: avoid duplicate UDP allocations during gathering

Like TCP, a running sequence that has not reached its UDP phase yet
covers UDP even before its port exists.

Bug: webrtc:566467875
Change-Id: I6b01f21a219fa26ce509ab9dda6cc180443c3562
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/506060
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#48742}

p2p/client/basic_port_allocator.cc

diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
index 6c0828f..1950d73 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -1320,18 +1320,14 @@
   // already have a port of the corresponding type. Look for a port that
   // matches this AllocationSequence's network, is the right protocol, and
   // hasn't encountered an error.
-  // TODO(deadbeef): This doesn't take into account that there may be another
-  // AllocationSequence that's ABOUT to allocate a UDP port, but hasn't yet.
-  // This can happen if, say, there's a network change event right before an
-  // application-triggered ICE restart. Hopefully this problem will just go
-  // away if we get rid of the gathering "phases" though, which is planned.
-  //
   //
   // PORTALLOCATOR_DISABLE_UDP is used to disable a Port from gathering the host
   // candidate (and srflx candidate if Port::SharedSocket()), and we do not want
   // to disable the gathering of these candidates just becaue of an existing
   // Port over PROTO_UDP, namely a TurnPort over UDP.
-  if (absl::c_any_of(session_->ports_,
+  if ((state_ == kRunning && phase_ == PHASE_UDP &&
+       !IsFlagSet(PORTALLOCATOR_DISABLE_UDP)) ||
+      absl::c_any_of(session_->ports_,
                      [this](const BasicPortAllocatorSession::PortData& p) {
                        return !p.pruned() && p.port()->Network() == network_ &&
                               p.port()->GetProtocol() == PROTO_UDP &&

p2p/client/basic_port_allocator_unittest.cc

diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc
index f9a8465..24e7a1d 100644
--- a/p2p/client/basic_port_allocator_unittest.cc
+++ b/p2p/client/basic_port_allocator_unittest.cc
@@ -1126,6 +1126,58 @@
             1);
 }
 
+TEST_F(BasicPortAllocatorTest,
+       NetworkChangeBeforeFirstPhaseDoesNotDuplicateUdp) {
+  ResetWithNoServersOrNat();
+  ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  time_controller_.AdvanceTime(TimeDelta::Millis(1000));
+  candidate_allocation_done_ = false;
+
+  // Both notifications run DoAllocate() before the sequence created by the
+  // first one gets to its UDP phase.
+  AddInterface(kClientAddr);
+  network_manager_.NotifyNetworksChanged();
+  ASSERT_TRUE(waiter_.Until([&] { return candidate_allocation_done_; }));
+  EXPECT_EQ(CountPorts(ports_, IceCandidateType::kHost, PROTO_UDP, kClientAddr),
+            1);
+  EXPECT_EQ(CountPorts(ports_, IceCandidateType::kHost, PROTO_TCP, kClientAddr),
+            1);
+}
+
+TEST_F(BasicPortAllocatorTest, ConcurrentSessionsDoNotDuplicatePorts) {
+  constexpr int kNumSessions = 10;
+  ResetWithNoServersOrNat();
+  AddInterface(kClientAddr);
+
+  // Each session subscribes to the shared network manager, whose
+  // StartUpdating() signals all sessions. Start each one a task after the
+  // previous one so these signals hit the others at every stage of gathering.
+  std::vector<std::unique_ptr<PortAllocatorSession>> sessions;
+  absl::AnyInvocable<void()> start_next = [&] {
+    sessions.push_back(CreateSession("session", ICE_CANDIDATE_COMPONENT_RTP));
+    sessions.back()->StartGettingPorts();
+    if (sessions.size() < kNumSessions) {
+      thread_->PostTask([&] { start_next(); });
+    }
+  };
+  start_next();
+  ASSERT_TRUE(waiter_.Until([&] {
+    return sessions.size() == kNumSessions &&
+           absl::c_all_of(sessions, [](const auto& session) {
+             return session->CandidatesAllocationDone();
+           });
+  }));
+
+  for (const auto& session : sessions) {
+    std::vector<PortInterface*> ports = session->ReadyPorts();
+    EXPECT_EQ(
+        CountPorts(ports, IceCandidateType::kHost, PROTO_UDP, kClientAddr), 1);
+    EXPECT_EQ(
+        CountPorts(ports, IceCandidateType::kHost, PROTO_TCP, kClientAddr), 1);
+  }
+}
+
 // Test that when the same network interface is brought down and up, the
 // port allocator session will restart a new allocation sequence if
 // it is not stopped.