p2p: Fix bug causing old candidates on ice restart

This patch fixes a bug where old candidates was
generated if doing GATHER_CONTINUALLY.

The problem was that the old port allocator session
was never stopped, and when the new sessio is created
it will attach to the network that will signal OnNetworkChanged().

The patch adds explicit stop of old sessions.

The problem was not possible to trigger using fake_network
as this "incorrectly" called SignalNetworkChanged directly
rather than after a Thread->Post() like network.cc does it.

Bug: webrtc:12210
Change-Id: Ief3f961bd97f06f4c4194ecbc3200c635ba63cf6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/194961
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32675}

p2p/base/p2p_transport_channel.cc

@@ -843,6 +843,13 @@ void P2PTransportChannel::MaybeStartGathering() {
                                 static_cast<int>(IceRestartState::MAX_VALUE));
     }
 
+    for (const auto& session : allocator_sessions_) {
+      if (session->IsStopped()) {
+        continue;
+      }
+      session->StopGettingPorts();
+    }
+
     // Time for a new allocator.
     std::unique_ptr<PortAllocatorSession> pooled_session =
         allocator_->TakePooledSession(transport_name(), component(),

p2p/base/p2p_transport_channel_unittest.cc

@@ -6108,4 +6108,36 @@ INSTANTIATE_TEST_SUITE_P(GatherAfterConnectedTest,
                          GatherAfterConnectedTest,
                          ::testing::Values(true, false));
 
+// Tests no candidates are generated with old ice ufrag/passwd after an ice
+// restart even if continual gathering is enabled.
+TEST_F(P2PTransportChannelTest, TestIceNoOldCandidatesAfterIceRestart) {
+  rtc::ScopedFakeClock clock;
+  AddAddress(0, kAlternateAddrs[0]);
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+
+  // gathers continually.
+  IceConfig config = CreateIceConfig(1000, GATHER_CONTINUALLY);
+  CreateChannels(config, config);
+
+  EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()),
+                             kDefaultTimeout, clock);
+
+  PauseCandidates(0);
+
+  ep1_ch1()->SetIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+
+  EXPECT_TRUE_SIMULATED_WAIT(GetEndpoint(0)->saved_candidates_.size() > 0,
+                             kDefaultTimeout, clock);
+
+  for (const auto& cd : GetEndpoint(0)->saved_candidates_) {
+    for (const auto& c : cd->candidates) {
+      EXPECT_EQ(c.username(), kIceUfrag[3]);
+    }
+  }
+
+  DestroyChannels();
+}
+
 }  // namespace cricket

rtc_base/fake_network.h

@@ -70,10 +70,11 @@ class FakeNetworkManager : public NetworkManagerBase,
     ++start_count_;
     if (start_count_ == 1) {
       sent_first_update_ = false;
-      rtc::Thread::Current()->Post(RTC_FROM_HERE, this);
+      rtc::Thread::Current()->Post(RTC_FROM_HERE, this, kUpdateNetworksMessage);
     } else {
       if (sent_first_update_) {
-        SignalNetworksChanged();
+        rtc::Thread::Current()->Post(RTC_FROM_HERE, this,
+                                     kSignalNetworksMessage);
       }
     }
   }
@@ -81,7 +82,15 @@ class FakeNetworkManager : public NetworkManagerBase,
   void StopUpdating() override { --start_count_; }
 
   // MessageHandler interface.
-  void OnMessage(Message* msg) override { DoUpdateNetworks(); }
+  void OnMessage(Message* msg) override {
+    if (msg->message_id == kUpdateNetworksMessage) {
+      DoUpdateNetworks();
+    } else if (msg->message_id == kSignalNetworksMessage) {
+      SignalNetworksChanged();
+    } else {
+      RTC_CHECK(false);
+    }
+  }
 
   using NetworkManagerBase::set_default_local_addresses;
   using NetworkManagerBase::set_enumeration_permission;
@@ -129,6 +138,9 @@ class FakeNetworkManager : public NetworkManagerBase,
   int start_count_ = 0;
   bool sent_first_update_ = false;
 
+  static constexpr uint32_t kUpdateNetworksMessage = 1;
+  static constexpr uint32_t kSignalNetworksMessage = 2;
+
   std::unique_ptr<webrtc::MdnsResponderInterface> mdns_responder_;
 };