webrtc/p2p/base/transport.h - Issue 1246913005: TransportController refactoring

Unified Diff: webrtc/p2p/base/transport.h

Issue 1246913005: TransportController refactoring (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master

Patch Set: minor cleanup Created 5 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: webrtc/p2p/base/transport.h

diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h

index 6064bdb54db257c978b6edeaa1f4613b9cd9641f..d566bf0ea6686f11a41c4ae2eadae577c29d3365 100644

--- a/webrtc/p2p/base/transport.h

+++ b/webrtc/p2p/base/transport.h

@@ -15,15 +15,8 @@

// state changes (in order to update the manager's state), and forwards

// requests to begin connecting or to reset to each of the channels.

-// On Threading: Transport performs work on both the signaling and worker

-// threads. For subclasses, the rule is that all signaling related calls will

-// be made on the signaling thread and all channel related calls (including

-// signaling for a channel) will be made on the worker thread. When

-// information needs to be sent between the two threads, this class should do

-// the work (e.g., OnRemoteCandidate).

-//

-// Note: Subclasses must call DestroyChannels() in their own constructors.

-// It is not possible to do so here because the subclass constructor will

+// Note: Subclasses must call DestroyChannels() in their own destructors.

+// It is not possible to do so here because the subclass destructor will

// already have run.

#ifndef WEBRTC_P2P_BASE_TRANSPORT_H_

@@ -36,15 +29,10 @@

#include "webrtc/p2p/base/constants.h"

#include "webrtc/p2p/base/sessiondescription.h"

#include "webrtc/p2p/base/transportinfo.h"

-#include "webrtc/base/criticalsection.h"

#include "webrtc/base/messagequeue.h"

#include "webrtc/base/sigslot.h"

#include "webrtc/base/sslstreamadapter.h"

-namespace rtc {

-class Thread;

namespace cricket {

class PortAllocator;

@@ -135,21 +123,19 @@ bool IceCredentialsChanged(const std::string& old_ufrag,

const std::string& new_ufrag,

const std::string& new_pwd);

-class Transport : public rtc::MessageHandler,

- public sigslot::has_slots<> {

+class Transport : public sigslot::has_slots<> {

public:

- Transport(rtc::Thread* signaling_thread,

- rtc::Thread* worker_thread,

- const std::string& content_name,

+ enum CandidateGatheringState {

+ kGatheringNew = 0,

+ kGatheringGathering,

+ kGatheringDone,

pthatcher1 2015/08/10 20:40:17 Done => Complete Also, this enum should probably

Taylor Brandstetter 2015/08/11 01:20:08 Done.

+ };

+ Transport(const std::string& content_name,

const std::string& type,

PortAllocator* allocator);

virtual ~Transport();

- // Returns the signaling thread. The app talks to Transport on this thread.

- rtc::Thread* signaling_thread() { return signaling_thread_; }

- // Returns the worker thread. The actual networking is done on this thread.

- rtc::Thread* worker_thread() { return worker_thread_; }

// Returns the content_name of this transport.

const std::string& content_name() const { return content_name_; }

// Returns the type of this transport.

@@ -184,6 +170,19 @@ class Transport : public rtc::MessageHandler,

return (receiving_ == TRANSPORT_STATE_SOME ||

receiving_ == TRANSPORT_STATE_ALL);

}

+ bool local_description_set() const {

+ return local_description_set_;

+ }

+ bool remote_description_set() const {

+ return remote_description_set_;

+ }

+ // Checks if all channels are completed

+ bool Completed() const;

+ CandidateGatheringState candidate_gathering_state() const {

+ return candidate_gathering_state_;

+ }

sigslot::signal1<Transport*> SignalReadableState;

sigslot::signal1<Transport*> SignalWritableState;

@@ -203,10 +202,10 @@ class Transport : public rtc::MessageHandler,

void SetChannelReceivingTimeout(int timeout_ms);

// Must be called before applying local session description.

- void SetIdentity(rtc::SSLIdentity* identity);

+ virtual void SetIdentity(rtc::SSLIdentity* identity) {}

// Get a copy of the local identity provided by SetIdentity.

- bool GetIdentity(rtc::SSLIdentity** identity);

+ virtual bool GetIdentity(rtc::SSLIdentity** identity) { return false; }

// Get a copy of the remote certificate in use by the specified channel.

bool GetRemoteCertificate(rtc::SSLCertificate** cert);

@@ -215,10 +214,9 @@ class Transport : public rtc::MessageHandler,

// Create, destroy, and lookup the channels of this type by their components.

TransportChannelImpl* CreateChannel(int component);

- // Note: GetChannel may lead to race conditions, since the mutex is not held

- // after the pointer is returned.

TransportChannelImpl* GetChannel(int component);

- // Note: HasChannel does not lead to race conditions, unlike GetChannel.

bool HasChannel(int component) {

return (NULL != GetChannel(component));

}

@@ -250,20 +248,18 @@ class Transport : public rtc::MessageHandler,

bool GetStats(TransportStats* stats);

- // Before any stanza is sent, the manager will request signaling. Once

- // signaling is available, the client should call OnSignalingReady. Once

- // this occurs, the transport (or its channels) can send any waiting stanzas.

- // OnSignalingReady invokes OnTransportSignalingReady and then forwards this

- // signal to each channel.

- sigslot::signal1<Transport*> SignalRequestSignaling;

- void OnSignalingReady();

+ sigslot::signal1<Transport*> SignalCandidatesAllocationStarted;

// Handles sending of ready candidates and receiving of remote candidates.

sigslot::signal2<Transport*,

const std::vector<Candidate>&> SignalCandidatesReady;

sigslot::signal1<Transport*> SignalCandidatesAllocationDone;

- void OnRemoteCandidates(const std::vector<Candidate>& candidates);

+ // Called when one or more candidates are ready from the remote peer.

+ bool AddRemoteCandidates(

+ const std::vector<Candidate>& candidates, std::string* error);

+ void AddRemoteCandidate(const Candidate& candidate);

pthatcher1 2015/08/10 20:40:17 Why do we need both of these?

Taylor Brandstetter 2015/08/11 01:20:08 I'm not sure; this was left over from your origina

// If candidate is not acceptable, returns false and sets error.

// Call this before calling OnRemoteCandidates.

@@ -278,10 +274,12 @@ class Transport : public rtc::MessageHandler,

// Forwards the signal from TransportChannel to BaseSession.

sigslot::signal0<> SignalRoleConflict;

- virtual bool GetSslRole(rtc::SSLRole* ssl_role) const;

+ virtual bool GetSslRole(rtc::SSLRole* ssl_role) const { return false; }

// Must be called before channel is starting to connect.

- virtual bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version);

+ virtual bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {

+ return false;

+ }

protected:

// These are called by Create/DestroyChannel above in order to create or

@@ -289,9 +287,6 @@ class Transport : public rtc::MessageHandler,

virtual TransportChannelImpl* CreateTransportChannel(int component) = 0;

virtual void DestroyTransportChannel(TransportChannelImpl* channel) = 0;

- // Informs the subclass that we received the signaling ready message.

- virtual void OnTransportSignalingReady() {}

// The current local transport description, for use by derived classes

// when performing transport description negotiation.

const TransportDescription* local_description() const {

@@ -304,21 +299,15 @@ class Transport : public rtc::MessageHandler,

return remote_description_.get();

}

- virtual void SetIdentity_w(rtc::SSLIdentity* identity) {}

- virtual bool GetIdentity_w(rtc::SSLIdentity** identity) {

- return false;

- }

// Pushes down the transport parameters from the local description, such

// as the ICE ufrag and pwd.

// Derived classes can override, but must call the base as well.

- virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* channel,

+ virtual bool ApplyLocalTransportDescription(TransportChannelImpl* channel,

std::string* error_desc);

// Pushes down remote ice credentials from the remote description to the

// transport channel.

- virtual bool ApplyRemoteTransportDescription_w(TransportChannelImpl* ch,

+ virtual bool ApplyRemoteTransportDescription(TransportChannelImpl* ch,

std::string* error_desc);

// Negotiates the transport parameters based on the current local and remote

@@ -326,29 +315,20 @@ class Transport : public rtc::MessageHandler,

// should be activated.

// Derived classes can negotiate their specific parameters here, but must call

// the base as well.

- virtual bool NegotiateTransportDescription_w(ContentAction local_role,

+ virtual bool NegotiateTransportDescription(ContentAction local_role,

std::string* error_desc);

// Pushes down the transport parameters obtained via negotiation.

// Derived classes can set their specific parameters here, but must call the

// base as well.

- virtual bool ApplyNegotiatedTransportDescription_w(

+ virtual bool ApplyNegotiatedTransportDescription(

TransportChannelImpl* channel, std::string* error_desc);

- virtual bool GetSslRole_w(rtc::SSLRole* ssl_role) const {

- return false;

- }

- virtual bool SetSslMaxProtocolVersion_w(rtc::SSLProtocolVersion version) {

- return false;

- }

private:

struct ChannelMapEntry {

- ChannelMapEntry() : impl_(NULL), candidates_allocated_(false), ref_(0) {}

+ ChannelMapEntry() : impl_(NULL), ref_(0) {}

explicit ChannelMapEntry(TransportChannelImpl *impl)

: impl_(impl),

- candidates_allocated_(false),

ref_(0) {

}

@@ -361,14 +341,9 @@ class Transport : public rtc::MessageHandler,

TransportChannelImpl* get() const { return impl_; }

TransportChannelImpl* operator->() const { return impl_; }

- void set_candidates_allocated(bool status) {

- candidates_allocated_ = status;

- }

- bool candidates_allocated() const { return candidates_allocated_; }

- private:

- TransportChannelImpl *impl_;

- bool candidates_allocated_;

+ private:

+ TransportChannelImpl* impl_;

int ref_;

};

@@ -382,11 +357,9 @@ class Transport : public rtc::MessageHandler,

// Called when the receiving state of a channel changes.

void OnChannelReceivingState(TransportChannel* channel);

- // Called when a channel requests signaling.

- void OnChannelRequestSignaling(TransportChannelImpl* channel);

+ // Called when a channel starts allocating candidates for a new ICE session

+ void OnChannelCandidatesAllocationStarted(TransportChannelImpl* channel);

- // Called when a candidate is ready from remote peer.

- void OnRemoteCandidate(const Candidate& candidate);

// Called when a candidate is ready from channel.

void OnChannelCandidateReady(TransportChannelImpl* channel,

const Candidate& candidate);

@@ -398,55 +371,24 @@ class Transport : public rtc::MessageHandler,

// Called when the channel removes a connection.

void OnChannelConnectionRemoved(TransportChannelImpl* channel);

- // Dispatches messages to the appropriate handler (below).

- void OnMessage(rtc::Message* msg);

- // These are versions of the above methods that are called only on a

- // particular thread (s = signaling, w = worker). The above methods post or

- // send a message to invoke this version.

- TransportChannelImpl* CreateChannel_w(int component);

- void DestroyChannel_w(int component);

- void ConnectChannels_w();

- void ResetChannels_w();

- void DestroyAllChannels_w();

- void OnRemoteCandidate_w(const Candidate& candidate);

- void OnChannelReadableState_s();

- void OnChannelWritableState_s();

- void OnChannelReceivingState_s();

- void OnChannelRequestSignaling_s();

- void OnConnecting_s();

- void OnChannelRouteChange_s(const TransportChannel* channel,

- const Candidate& remote_candidate);

- void OnChannelCandidatesAllocationDone_s();

// Helper function that invokes the given function on every channel.

typedef void (TransportChannelImpl::* TransportChannelFunc)();

- void CallChannels_w(TransportChannelFunc func);

+ void CallChannels(TransportChannelFunc func);

// Computes the AND and OR of the channel's read/write/receiving state

// (argument picks the operation).

- TransportState GetTransportState_s(TransportStateType type);

- void OnChannelCandidateReady_s();

+ TransportState GetTransportState(TransportStateType type);

- void SetIceRole_w(IceRole role);

- void SetRemoteIceMode_w(IceMode mode);

- bool SetLocalTransportDescription_w(const TransportDescription& desc,

- ContentAction action,

- std::string* error_desc);

- bool SetRemoteTransportDescription_w(const TransportDescription& desc,

- ContentAction action,

- std::string* error_desc);

- bool GetStats_w(TransportStats* infos);

- bool GetRemoteCertificate_w(rtc::SSLCertificate** cert);

+ // Sends SignalCompleted if we are now in that state.

+ void CheckIfCompleted();

- void SetChannelReceivingTimeout_w(int timeout_ms);

+ // Sends SignalCandidatesAllocationDone if all channels are done

+ void CheckIfCandidatesAllocationDone();

- // Sends SignalCompleted if we are now in that state.

- void MaybeCompleted_w();

+ void UpdateReadableState();

+ void UpdateWritableState();

+ void UpdateReceivingState();

- rtc::Thread* const signaling_thread_;

- rtc::Thread* const worker_thread_;

const std::string content_name_;

const std::string type_;

PortAllocator* const allocator_;

@@ -463,14 +405,14 @@ class Transport : public rtc::MessageHandler,

int channel_receiving_timeout_;

rtc::scoped_ptr<TransportDescription> local_description_;

rtc::scoped_ptr<TransportDescription> remote_description_;

+ bool local_description_set_;

+ bool remote_description_set_;

+ CandidateGatheringState candidate_gathering_state_;

- // TODO(tommi): Make sure we only use this on the worker thread.

ChannelMap channels_;

// Buffers the ready_candidates so that SignalCanidatesReady can

// provide them in multiples.

std::vector<Candidate> ready_candidates_;

- // Protects changes to channels and messages

- rtc::CriticalSection crit_;

DISALLOW_COPY_AND_ASSIGN(Transport);

};

« webrtc/p2p/base/rawtransport.h ('K') | « webrtc/p2p/base/session_unittest.cc ('k') | webrtc/p2p/base/transport.cc » ('j') | webrtc/p2p/p2p.gyp » ('J')