Reland "Delay call to Destroy until after SignalDone has finished firing."

This is a reland of c7b8da453c

Original change's description:
> Delay call to Destroy until after SignalDone has finished firing.
> 
> Bug: chromium:905542
> Change-Id: I0def33a3ac21bd0da4b77ef05d42a0e7ef66caec
> Reviewed-on: https://webrtc-review.googlesource.com/c/112163
> Commit-Queue: Zach Stein <zstein@webrtc.org>
> Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#25807}

Bug: chromium:905542
Change-Id: I5dd861bcd3b2ebcb7ff3db0888da1520eaaaca72
Reviewed-on: https://webrtc-review.googlesource.com/c/112240
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Zach Stein <zstein@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25812}
This commit is contained in:
Zach Stein 2018-11-27 16:58:08 -08:00 committed by Commit Bot
parent 0d007d7c4f
commit 5ec2c94b0b
2 changed files with 18 additions and 4 deletions

View file

@ -1098,7 +1098,9 @@ void P2PTransportChannel::OnCandidateResolved(
Candidate candidate = p->candidate_;
resolvers_.erase(p);
AddRemoteCandidateWithResolver(candidate, resolver);
resolver->Destroy(false);
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, thread(),
rtc::Bind(&rtc::AsyncResolverInterface::Destroy, resolver, false));
}
void P2PTransportChannel::AddRemoteCandidateWithResolver(

View file

@ -43,6 +43,7 @@ namespace {
using rtc::SocketAddress;
using ::testing::_;
using ::testing::Assign;
using ::testing::DoAll;
using ::testing::InSequence;
using ::testing::InvokeWithoutArgs;
@ -4565,13 +4566,18 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, TestTcpTurn) {
}
// Test that a resolver is created, asked for a result, and destroyed
// when the address is a hostname.
// when the address is a hostname. The destruction should happen even
// if the channel is not destroyed.
TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) {
rtc::MockAsyncResolver mock_async_resolver;
EXPECT_CALL(mock_async_resolver, GetError()).WillOnce(Return(0));
EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _))
.WillOnce(Return(true));
EXPECT_CALL(mock_async_resolver, Destroy(_));
// Destroy is called asynchronously after the address is resolved,
// so we need a variable to wait on.
bool destroy_called = false;
EXPECT_CALL(mock_async_resolver, Destroy(_))
.WillOnce(Assign(&destroy_called, true));
webrtc::MockAsyncResolverFactory mock_async_resolver_factory;
EXPECT_CALL(mock_async_resolver_factory, Create())
.WillOnce(Return(&mock_async_resolver));
@ -4586,6 +4592,7 @@ TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) {
ASSERT_EQ_WAIT(1u, channel.remote_candidates().size(), kDefaultTimeout);
const RemoteCandidate& candidate = channel.remote_candidates()[0];
EXPECT_FALSE(candidate.address().IsUnresolvedIP());
WAIT(destroy_called, kShortTimeout);
}
// Test that if we signal a hostname candidate after the remote endpoint
@ -4643,7 +4650,11 @@ TEST_F(P2PTransportChannelTest,
EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _))
.WillOnce(DoAll(SetArgPointee<1>(local_address), Return(true)));
}
EXPECT_CALL(mock_async_resolver, Destroy(_));
// Destroy is called asynchronously after the address is resolved,
// so we need a variable to wait on.
bool destroy_called = false;
EXPECT_CALL(mock_async_resolver, Destroy(_))
.WillOnce(Assign(&destroy_called, true));
ResumeCandidates(0);
// Verify ep2's selected connection is updated to use the 'local' candidate.
EXPECT_EQ_WAIT(LOCAL_PORT_TYPE,
@ -4651,6 +4662,7 @@ TEST_F(P2PTransportChannelTest,
kMediumTimeout);
EXPECT_EQ(selected_connection, ep2_ch1()->selected_connection());
WAIT(destroy_called, kShortTimeout);
DestroyChannels();
}