Reset TURN port NONCE when a new socket is created.

webrtc/p2p/base/turnport_unittest.cc

Index: webrtc/p2p/base/turnport_unittest.cc
diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc
index 916162575fee803093aff79ea89afde02cae5902..1bca13c233d436262503b4cecebc68d9173917f3 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -605,6 +605,34 @@ TEST_F(TurnPortTest, TestTurnAllocateBadPassword) {
   ASSERT_EQ(0U, turn_port_->Candidates().size());
 }
 
+// Tests that TURN port NONCE will be reset when receiving an ALLOCATE MISMATCH
+// error.
+TEST_F(TurnPortTest, TestTurnAllocateNonceRestAfterAllocateMismatch) {

[Taylor Brandstetter, 2016/01/26 02:02:06]

"NonceReset" instead of "NonceRest"

[honghaiz3, 2016/01/27 01:45:23]

Done.

+  // Do a normal allocation first.
+  CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+  turn_port_->PrepareAddress();
+  EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
+  rtc::SocketAddress first_addr(turn_port_->socket()->GetLocalAddress());
+  // Clear connected_ flag on turnport to suppress the release of
+  // the allocation.
+  turn_port_->OnSocketClose(turn_port_->socket(), 0);

[Taylor Brandstetter, 2016/01/26 02:02:06]

I don't think it's ideal to call "OnSocketClose" in a unit test, since
OnSocketClose is effectively a private method (I don't see why it isn't one).
Eventually it could go away, or its implementation could change, breaking this
test. A more foolproof way to test this would be:

1. Create TurnPort and wait for it to be ready.
2. Set the drop probability of the virtual socket server to 100%.
3. Destroy the TurnPort (the allocation release message will be dropped).
4. Set the drop probability back to 0%.
5. Create a new TurnPort, ensuring it gets the same UDP port number, proceed
with test.

[honghaiz3, 2016/01/27 01:45:23]

Done.

+
+  // Force the socket server to assign the same port.
+  ss_->SetNextPortForTesting(first_addr.port());
+  turn_ready_ = false;
+  CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+
+  // Make sure the next timestamp will be different from |now|, so that the two
+  // NONCE generated by the server will be different.

[Taylor Brandstetter, 2016/01/26 02:02:06]

I would also add a comment saying: "It's expected that turn_port_ will set
next_nonce, but then get an allocate mismatch error and set an ever newer
nonce."

Just so the unit test expectation is clearer to someone like me.

[honghaiz3, 2016/01/27 01:45:23]

Done. Text is slightly different though. 

+  uint32_t now = rtc::Time() - 1;
+  turn_server_.server()->SetTimestampForNextNonce(now);
+  std::string next_nonce = turn_server_.server()->GenerateNonce(now);
+  turn_port_->PrepareAddress();
+
+  EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
+  EXPECT_NE(next_nonce, turn_port_->nonce());
+}
+
 // Tests that a new local address is created after
 // STUN_ERROR_ALLOCATION_MISMATCH.
 TEST_F(TurnPortTest, TestTurnAllocateMismatch) {