Revert of Remove GetTransport() from TransportChannelImpl (patchset #3 id:40001 of https://codereview.webrtc.org/1691673002/ )

Reason for revert:
This CL is breaking a lot of FYI bots.
The specific change that breaks bots is the removal of a constructor parameter.

See, for example: https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win%20Builder/builds/3572/steps/compile/logs/stdio

Original issue's description:
> Remove GetTransport() from TransportChannelImpl
>
> This appears to be dead code because GetTransport() is not used by WebRTC. It also adds dead code to DtlsTransportChannelWrapper and P2PTransportChannel.
>
> BUG=
>
> Committed: https://crrev.com/ee18220ddd783fad9812f1c1c195bf187a631c3a
> Cr-Commit-Position: refs/heads/master@{#11662}

TBR=pthatcher@webrtc.org,mikescarlett@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.webrtc.org/1709953002

Cr-Commit-Position: refs/heads/master@{#11665}
This commit is contained in:
guidou 2016-02-18 01:57:49 -08:00 committed by Commit bot
parent 59c634b605
commit 74db777d64
10 changed files with 44 additions and 22 deletions

View File

@ -199,7 +199,7 @@ class DtlsTransport : public Base {
DtlsTransportChannelWrapper* CreateTransportChannel(int component) override {
DtlsTransportChannelWrapper* channel = new DtlsTransportChannelWrapper(
Base::CreateTransportChannel(component));
this, Base::CreateTransportChannel(component));
channel->SetSslMaxProtocolVersion(ssl_max_version_);
return channel;
}

View File

@ -89,8 +89,10 @@ bool StreamInterfaceChannel::OnPacketReceived(const char* data, size_t size) {
}
DtlsTransportChannelWrapper::DtlsTransportChannelWrapper(
Transport* transport,
TransportChannelImpl* channel)
: TransportChannelImpl(channel->transport_name(), channel->component()),
transport_(transport),
worker_thread_(rtc::Thread::Current()),
channel_(channel),
downward_(NULL),

View File

@ -82,8 +82,10 @@ class StreamInterfaceChannel : public rtc::StreamInterface {
class DtlsTransportChannelWrapper : public TransportChannelImpl {
public:
// The parameters here are:
// transport -- the DtlsTransport that created us
// channel -- the TransportChannel we are wrapping
explicit DtlsTransportChannelWrapper(TransportChannelImpl* channel);
DtlsTransportChannelWrapper(Transport* transport,
TransportChannelImpl* channel);
~DtlsTransportChannelWrapper() override;
void SetIceRole(IceRole role) override { channel_->SetIceRole(role); }
@ -157,6 +159,8 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl {
}
// TransportChannelImpl calls.
Transport* GetTransport() override { return transport_; }
TransportChannelState GetState() const override {
return channel_->GetState();
}
@ -214,8 +218,9 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl {
void OnConnectionRemoved(TransportChannelImpl* channel);
void Reconnect();
Transport* transport_; // The transport_ that created us.
rtc::Thread* worker_thread_; // Everything should occur on this thread.
// Underlying channel, not owned by this class.
// Underlying channel, owned by transport_.
TransportChannelImpl* const channel_;
rtc::scoped_ptr<rtc::SSLStreamAdapter> dtls_; // The DTLS stream
StreamInterfaceChannel* downward_; // Wrapper for channel_, owned by dtls_.

View File

@ -45,9 +45,11 @@ struct PacketMessageData : public rtc::MessageData {
class FakeTransportChannel : public TransportChannelImpl,
public rtc::MessageHandler {
public:
explicit FakeTransportChannel(const std::string& name,
explicit FakeTransportChannel(Transport* transport,
const std::string& name,
int component)
: TransportChannelImpl(name, component),
transport_(transport),
dtls_fingerprint_("", nullptr, 0) {}
~FakeTransportChannel() { Reset(); }
@ -65,6 +67,8 @@ class FakeTransportChannel : public TransportChannelImpl,
// synchronously "Send"-ing.
void SetAsync(bool async) { async_ = async; }
Transport* GetTransport() override { return transport_; }
TransportChannelState GetState() const override {
if (connection_count_ == 0) {
return had_connection_ ? TransportChannelState::STATE_FAILED
@ -309,6 +313,7 @@ class FakeTransportChannel : public TransportChannelImpl,
private:
enum State { STATE_INIT, STATE_CONNECTING, STATE_CONNECTED };
Transport* transport_;
FakeTransportChannel* dest_ = nullptr;
State state_ = STATE_INIT;
bool async_ = false;
@ -410,7 +415,7 @@ class FakeTransport : public Transport {
return nullptr;
}
FakeTransportChannel* channel =
new FakeTransportChannel(name(), component);
new FakeTransportChannel(this, name(), component);
channel->set_ssl_max_protocol_version(ssl_max_version_);
channel->SetAsync(async_);
SetChannelDestination(component, channel);

View File

@ -28,7 +28,7 @@ P2PTransport::~P2PTransport() {
}
TransportChannelImpl* P2PTransport::CreateTransportChannel(int component) {
return new P2PTransportChannel(name(), component, port_allocator());
return new P2PTransportChannel(name(), component, this, port_allocator());
}
void P2PTransport::DestroyTransportChannel(TransportChannelImpl* channel) {

View File

@ -217,8 +217,10 @@ static const int MIN_CHECK_RECEIVING_DELAY = 50; // ms
P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
int component,
P2PTransport* transport,
PortAllocator* allocator)
: TransportChannelImpl(transport_name, component),
transport_(transport),
allocator_(allocator),
worker_thread_(rtc::Thread::Current()),
incoming_only_(false),

View File

@ -27,6 +27,7 @@
#include "webrtc/p2p/base/p2ptransport.h"
#include "webrtc/p2p/base/portallocator.h"
#include "webrtc/p2p/base/portinterface.h"
#include "webrtc/p2p/base/transport.h"
#include "webrtc/p2p/base/transportchannelimpl.h"
#include "webrtc/base/asyncpacketsocket.h"
#include "webrtc/base/sigslot.h"
@ -66,10 +67,12 @@ class P2PTransportChannel : public TransportChannelImpl,
public:
P2PTransportChannel(const std::string& transport_name,
int component,
P2PTransport* transport,
PortAllocator* allocator);
virtual ~P2PTransportChannel();
// From TransportChannelImpl:
Transport* GetTransport() override { return transport_; }
TransportChannelState GetState() const override;
void SetIceRole(IceRole role) override;
IceRole GetIceRole() const override { return ice_role_; }
@ -262,6 +265,7 @@ class P2PTransportChannel : public TransportChannelImpl,
: static_cast<uint32_t>(remote_ice_parameters_.size() - 1);
}
P2PTransport* transport_;
PortAllocator* allocator_;
rtc::Thread* worker_thread_;
bool incoming_only_;

View File

@ -306,7 +306,7 @@ class P2PTransportChannelTestBase : public testing::Test,
const std::string& remote_ice_ufrag,
const std::string& remote_ice_pwd) {
cricket::P2PTransportChannel* channel = new cricket::P2PTransportChannel(
"test content name", component, GetAllocator(endpoint));
"test content name", component, NULL, GetAllocator(endpoint));
channel->SignalCandidateGathered.connect(
this, &P2PTransportChannelTestBase::OnCandidate);
channel->SignalReadPacket.connect(
@ -1910,7 +1910,7 @@ class P2PTransportChannelPingTest : public testing::Test,
TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("trigger checks", 1, &pa);
cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -1935,7 +1935,7 @@ TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("trigger checks", 1, &pa);
cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -1966,7 +1966,7 @@ TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {
// ufrag, its pwd and generation will be set properly.
TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("add candidate", 1, &pa);
cricket::P2PTransportChannel ch("add candidate", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -2016,7 +2016,7 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) {
TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("connection resurrection", 1, &pa);
cricket::P2PTransportChannel ch("connection resurrection", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -2069,7 +2069,7 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
PrepareChannel(&ch);
// Default receiving timeout and checking receiving delay should not be too
// small.
@ -2097,7 +2097,7 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {
// "best connection".
TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
ch.Connect();
@ -2154,7 +2154,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
// a ping response and set the ICE pwd in the remote candidate appropriately.
TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
ch.Connect();
@ -2236,7 +2236,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
// the "best connection".
TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
ch.Connect();
@ -2294,7 +2294,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
// be pruned. Otherwise, lower-priority connections are kept.
TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
ch.Connect();
@ -2332,7 +2332,7 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
// Test that GetState returns the state correctly.
TEST_F(P2PTransportChannelPingTest, TestGetState) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -2359,7 +2359,7 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
// right away, and it can become active and be pruned again.
TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceConfig(CreateIceConfig(1000, false));
ch.Connect();
@ -2402,7 +2402,7 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
// will all be deleted. We use Prune to simulate write_time_out.
TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
@ -2434,7 +2434,7 @@ TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {
// holds even if the transport channel did not lose the writability.
TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceConfig(CreateIceConfig(2000, false));
ch.Connect();

View File

@ -12,6 +12,7 @@
#define WEBRTC_P2P_BASE_TRANSPORTCHANNELIMPL_H_
#include <string>
#include "webrtc/p2p/base/transport.h"
#include "webrtc/p2p/base/transportchannel.h"
namespace buzz { class XmlElement; }
@ -35,6 +36,9 @@ class TransportChannelImpl : public TransportChannel {
int component)
: TransportChannel(transport_name, component) {}
// Returns the transport that created this channel.
virtual Transport* GetTransport() = 0;
// For ICE channels.
virtual IceRole GetIceRole() const = 0;
virtual void SetIceRole(IceRole role) = 0;

View File

@ -286,9 +286,9 @@ class QuicSessionTest : public ::testing::Test,
// establishes encryption with client.
void QuicSessionTest::CreateClientAndServerSessions() {
scoped_ptr<FakeTransportChannel> channel1(
new FakeTransportChannel("channel1", 0));
new FakeTransportChannel(nullptr, "channel1", 0));
scoped_ptr<FakeTransportChannel> channel2(
new FakeTransportChannel("channel2", 0));
new FakeTransportChannel(nullptr, "channel2", 0));
// Prevent channel1->OnReadPacket and channel2->OnReadPacket from calling
// themselves in a loop, which causes to future packets to be recursively