Merging BaseSession code into WebRtcSession.

talk/app/webrtc/webrtcsession.h

Index: talk/app/webrtc/webrtcsession.h
diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h
index 6e4a9e92b459385054206325e925dc87ae5b4641..7b5ed5617141765e2de9bc795fe10b2abf866bef 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -38,7 +38,7 @@
 #include "talk/app/webrtc/peerconnectioninterface.h"
 #include "talk/app/webrtc/statstypes.h"
 #include "talk/media/base/mediachannel.h"
-#include "webrtc/p2p/base/session.h"
+#include "webrtc/p2p/base/transportcontroller.h"
 #include "talk/session/media/mediasession.h"
 #include "webrtc/base/sigslot.h"
 #include "webrtc/base/sslidentity.h"
@@ -46,7 +46,6 @@
 
 namespace cricket {
 
-class BaseChannel;
 class ChannelManager;
 class DataChannel;
 class StatsReport;
@@ -112,13 +111,57 @@ class IceObserver {
   RTC_DISALLOW_COPY_AND_ASSIGN(IceObserver);
 };
 
-class WebRtcSession : public cricket::BaseSession,
-                      public AudioProviderInterface,
+// Statistics for all the transports of the session.
+typedef std::map<std::string, cricket::TransportStats> TransportStatsMap;
+typedef std::map<std::string, std::string> ProxyTransportMap;
+
+// TODO(pthatcher): Think of a better name for this.  We already have
+// a TransportStats in transport.h.  Perhaps TransportsStats?
+struct SessionStats {
+  ProxyTransportMap proxy_to_transport;
+  TransportStatsMap transport_stats;
+};
+
+// A WebRtcSession manages general session state. This includes negotiation
+// of both the application-level and network-level protocols:  the former
+// defines what will be sent and the latter defines how it will be sent.  Each
+// network-level protocol is represented by a Transport object.  Each Transport
+// participates in the network-level negotiation.  The individual streams of
+// packets are represented by TransportChannels.  The application-level protocol
+// is represented by SessionDecription objects.
+class WebRtcSession : public AudioProviderInterface,
                       public DataChannelFactory,
                       public VideoProviderInterface,
                       public DtmfProviderInterface,
-                      public DataChannelProviderInterface {
+                      public DataChannelProviderInterface,
+                      public rtc::MessageHandler,
+                      public sigslot::has_slots<> {
  public:
+  enum State {
+    STATE_INIT = 0,
+    STATE_SENTINITIATE,       // sent initiate, waiting for Accept or Reject
+    STATE_RECEIVEDINITIATE,   // received an initiate. Call Accept or Reject
+    STATE_SENTPRACCEPT,       // sent provisional Accept
+    STATE_SENTACCEPT,         // sent accept. begin connecting transport
+    STATE_RECEIVEDPRACCEPT,   // received provisional Accept, waiting for Accept
+    STATE_RECEIVEDACCEPT,     // received accept. begin connecting transport
+    STATE_SENTMODIFY,         // sent modify, waiting for Accept or Reject
+    STATE_RECEIVEDMODIFY,     // received modify, call Accept or Reject
+    STATE_SENTREJECT,         // sent reject after receiving initiate
+    STATE_RECEIVEDREJECT,     // received reject after sending initiate
+    STATE_SENTREDIRECT,       // sent direct after receiving initiate
+    STATE_SENTTERMINATE,      // sent terminate (any time / either side)
+    STATE_RECEIVEDTERMINATE,  // received terminate (any time / either side)
+    STATE_INPROGRESS,         // session accepted and in progress
+    STATE_DEINIT,             // session is being destroyed

[pthatcher1, 2015/10/09 03:28:57]

Not all of these are used any more, and almost all of them could use a name
update to reflect offer/answer instead of initiate/accept.  Can you delete the
unused ones and do a little renaming?

[Taylor Brandstetter, 2015/10/09 23:02:07]

Done.

+  };
+
+  enum Error {
+    ERROR_NONE = 0,       // no error
+    ERROR_CONTENT = 1,    // channel errors in SetLocalContent/SetRemoteContent
+    ERROR_TRANSPORT = 2,  // transport error of some kind
+  };
+
   WebRtcSession(cricket::ChannelManager* channel_manager,
                 rtc::Thread* signaling_thread,
                 rtc::Thread* worker_thread,
@@ -126,6 +169,14 @@ class WebRtcSession : public cricket::BaseSession,
                 MediaStreamSignaling* mediastream_signaling);
   virtual ~WebRtcSession();
 
+  // These are const to allow them to be called from const methods.
+  rtc::Thread* signaling_thread() const { return signaling_thread_; }
+  rtc::Thread* worker_thread() const { return worker_thread_; }
+  cricket::PortAllocator* port_allocator() const { return port_allocator_; }
+
+  // The ID of this session.
+  const std::string& id() const { return sid_; }
+
   bool Initialize(
       const PeerConnectionFactoryInterface::Options& options,
       const MediaConstraintsInterface* constraints,
@@ -135,6 +186,20 @@ class WebRtcSession : public cricket::BaseSession,
   // to STATE_RECEIVEDTERMINATE.
   void Terminate();
 
+  // Returns true if we were the initial offerer.
+  bool initiator() const { return initiator_; }

[pthatcher1, 2015/10/09 03:28:57]

Maybe call it offerer.

[Taylor Brandstetter, 2015/10/09 23:02:06]

Since there can be multiple offer/answer pairs, I'll call it "initial_offerer".

+
+  // Returns the current state of the session.  See the enum above for details.
+  // Each time the state changes, we will fire this signal.
+  State state() const { return state_; }
+  sigslot::signal2<WebRtcSession*, State> SignalState;
+
+  // Returns the last error in the session.  See the enum above for details.
+  // Each time the an error occurs, we will fire this signal.
+  Error error() const { return error_; }
+  const std::string& error_desc() const { return error_desc_; }
+  sigslot::signal2<WebRtcSession*, Error> SignalError;

[pthatcher1, 2015/10/09 03:28:57]

Is this still used?  I think we can just remove it.

[Taylor Brandstetter, 2015/10/09 23:02:07]

Done.

+
   void RegisterIceObserver(IceObserver* observer) {
     ice_observer_ = observer;
   }
@@ -159,10 +224,6 @@ class WebRtcSession : public cricket::BaseSession,
   // Get current ssl role from transport.
   bool GetSslRole(rtc::SSLRole* role);
 
-  // Generic error message callback from WebRtcSession.
-  // TODO - It may be necessary to supply error code as well.
-  sigslot::signal0<> SignalError;
-
   void CreateOffer(
       CreateSessionDescriptionObserver* observer,
       const PeerConnectionInterface::RTCOfferAnswerOptions& options);
@@ -181,20 +242,27 @@ class WebRtcSession : public cricket::BaseSession,
   cricket::IceConfig ParseIceConfig(
       const PeerConnectionInterface::RTCConfiguration& config) const;
 
+  void SetIceConfig(const cricket::IceConfig& ice_config);
+
+  // Start gathering candidates for any new transports, or transports doing an
+  // ICE restart.
+  void MaybeStartGathering();
+
   const SessionDescriptionInterface* local_description() const {
     return local_desc_.get();
   }
   const SessionDescriptionInterface* remote_description() const {
     return remote_desc_.get();
   }
+
   // TODO(pthatcher): Cleanup the distinction between
   // SessionDescription and SessionDescriptionInterface and remove
   // these if possible.
   const cricket::SessionDescription* base_local_description() const {
-    return BaseSession::local_description();
+    return local_desc_ ? local_desc_->description() : nullptr;

[pthatcher1, 2015/10/09 03:28:57]

base_ doesn't make sense as a prefix any more.  But I don't know what does. 
unwrapped?

[Taylor Brandstetter, 2015/10/09 23:02:06]

Actually, the only reason I even kept these around was for the "if
SessionDescriptionInterface is null, don't dereference it" logic. But I'm
realizing that that's only needed in a couple places. So I'm just removing these
methods.

   }
   const cricket::SessionDescription* base_remote_description() const {
-    return BaseSession::remote_description();
+    return remote_desc_ ? remote_desc_->description() : nullptr;
   }
 
   // Get the id used as a media stream track's "id" field from ssrc.
@@ -238,11 +306,10 @@ class WebRtcSession : public cricket::BaseSession,
 
   // Returns stats for all channels of all transports.
   // This avoids exposing the internal structures used to track them.
-  virtual bool GetTransportStats(cricket::SessionStats* stats);
+  virtual bool GetTransportStats(SessionStats* stats);
 
   // Get stats for a specific channel
-  bool GetChannelTransportStats(cricket::BaseChannel* ch,
-                                cricket::SessionStats* stats);
+  bool GetChannelTransportStats(cricket::BaseChannel* ch, SessionStats* stats);
 
   // virtual so it can be mocked in unit tests
   virtual bool GetLocalCertificate(
@@ -288,6 +355,19 @@ class WebRtcSession : public cricket::BaseSession,
     kAnswer,
   };
 
+  // Handles messages posted to us.
+  void OnMessage(rtc::Message* pmsg) override;
+
+  // Log session state.
+  void LogState(State old_state, State new_state);
+
+  // Updates the state, signaling if necessary.
+  virtual void SetState(State state);
+
+  // Updates the error state, signaling if necessary.
+  // TODO(ronghuawu): remove the SetError method that doesn't take |error_desc|.
+  virtual void SetError(Error error, const std::string& error_desc);
+
   bool UpdateSessionState(Action action, cricket::ContentSource source,
                           std::string* err_desc);
   static Action GetAction(const std::string& type);
@@ -297,6 +377,27 @@ class WebRtcSession : public cricket::BaseSession,
                                 cricket::ContentSource source,
                                 std::string* error_desc);
 
+  bool PushdownTransportDescription(cricket::ContentSource source,
+                                    cricket::ContentAction action,
+                                    std::string* error_desc);
+
+  // Helper methods to push local and remote transport descriptions.
+  bool PushdownLocalTransportDescription(
+      const cricket::SessionDescription* sdesc,
+      cricket::ContentAction action,
+      std::string* error_desc);
+  bool PushdownRemoteTransportDescription(
+      const cricket::SessionDescription* sdesc,
+      cricket::ContentAction action,
+      std::string* error_desc);
+
+  // Returns true and the TransportInfo of the given |content_name|
+  // from |description|. Returns false if it's not available.
+  static bool GetTransportDescription(
+      const cricket::SessionDescription* description,
+      const std::string& content_name,
+      cricket::TransportDescription* info);
+
   cricket::BaseChannel* GetChannel(const std::string& content_name);
   // Cause all the BaseChannels in the bundle group to have the same
   // transport channel.
@@ -383,6 +484,18 @@ class WebRtcSession : public cricket::BaseSession,
 
   void ReportNegotiatedCiphers(const cricket::TransportStats& stats);
 
+  rtc::Thread* const signaling_thread_;
+  rtc::Thread* const worker_thread_;
+  cricket::PortAllocator* const port_allocator_;
+
+  State state_ = STATE_INIT;
+  Error error_ = ERROR_NONE;
+  std::string error_desc_;
+
+  const std::string sid_;
+  bool initiator_ = false;
+
+  rtc::scoped_ptr<cricket::TransportController> transport_controller_;
   rtc::scoped_ptr<MediaControllerInterface> media_controller_;
   rtc::scoped_ptr<cricket::VoiceChannel> voice_channel_;
   rtc::scoped_ptr<cricket::VideoChannel> video_channel_;