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}
diff --git a/webrtc/p2p/base/dtlstransport.h b/webrtc/p2p/base/dtlstransport.h
index 276b05f..9f2903e 100644
--- a/webrtc/p2p/base/dtlstransport.h
+++ b/webrtc/p2p/base/dtlstransport.h
@@ -199,7 +199,7 @@
DtlsTransportChannelWrapper* CreateTransportChannel(int component) override {
DtlsTransportChannelWrapper* channel = new DtlsTransportChannelWrapper(
- Base::CreateTransportChannel(component));
+ this, Base::CreateTransportChannel(component));
channel->SetSslMaxProtocolVersion(ssl_max_version_);
return channel;
}
diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc
index 59ef2bc..d6b5bce 100644
--- a/webrtc/p2p/base/dtlstransportchannel.cc
+++ b/webrtc/p2p/base/dtlstransportchannel.cc
@@ -89,8 +89,10 @@
}
DtlsTransportChannelWrapper::DtlsTransportChannelWrapper(
+ Transport* transport,
TransportChannelImpl* channel)
: TransportChannelImpl(channel->transport_name(), channel->component()),
+ transport_(transport),
worker_thread_(rtc::Thread::Current()),
channel_(channel),
downward_(NULL),
diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h
index f396a57..955b963 100644
--- a/webrtc/p2p/base/dtlstransportchannel.h
+++ b/webrtc/p2p/base/dtlstransportchannel.h
@@ -82,8 +82,10 @@
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 @@
}
// TransportChannelImpl calls.
+ Transport* GetTransport() override { return transport_; }
+
TransportChannelState GetState() const override {
return channel_->GetState();
}
@@ -214,8 +218,9 @@
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_.
diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h
index 82f4ebd..65c59be 100644
--- a/webrtc/p2p/base/faketransportcontroller.h
+++ b/webrtc/p2p/base/faketransportcontroller.h
@@ -45,9 +45,11 @@
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 @@
// 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 @@
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 @@
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);
diff --git a/webrtc/p2p/base/p2ptransport.cc b/webrtc/p2p/base/p2ptransport.cc
index 1ad2a6f..abc4c14 100644
--- a/webrtc/p2p/base/p2ptransport.cc
+++ b/webrtc/p2p/base/p2ptransport.cc
@@ -28,7 +28,7 @@
}
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) {
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 15d8f3d..74f1392 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -217,8 +217,10 @@
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),
diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h
index 8e36144..fde0026 100644
--- a/webrtc/p2p/base/p2ptransportchannel.h
+++ b/webrtc/p2p/base/p2ptransportchannel.h
@@ -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 @@
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 @@
: static_cast<uint32_t>(remote_ice_parameters_.size() - 1);
}
+ P2PTransport* transport_;
PortAllocator* allocator_;
rtc::Thread* worker_thread_;
bool incoming_only_;
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 74b96a3..38f12fa 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -306,7 +306,7 @@
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 @@
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, 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 @@
// 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, 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, 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 @@
// "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 @@
// 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 @@
// 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 @@
// 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 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 @@
// 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 @@
// 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 @@
// 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();
diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h
index 1fa088a..8d4d4bb 100644
--- a/webrtc/p2p/base/transportchannelimpl.h
+++ b/webrtc/p2p/base/transportchannelimpl.h
@@ -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 @@
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;
diff --git a/webrtc/p2p/quic/quicsession_unittest.cc b/webrtc/p2p/quic/quicsession_unittest.cc
index 765a47f..6733796 100644
--- a/webrtc/p2p/quic/quicsession_unittest.cc
+++ b/webrtc/p2p/quic/quicsession_unittest.cc
@@ -286,9 +286,9 @@
// 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
