From 57fb3154b5411934b80051ad827db4e54d00f381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20H=C3=B6glund?= Date: Fri, 29 Sep 2017 12:02:10 +0200 Subject: [PATCH] Clean up libjingle API dependencies. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL moves candidate.h into the public API, since it has been implicitly included before. This is a straightforward way of solving the circular dependencies involving that file. For instance, libjingle_peerconnection_api includes candidate.h from jsepicecandidate.h, but _api can't depend on rtc_p2p, which depends on _api. In fact, _api can't depend on much at all since it's a very high level abstraction; instead, things should depend on it. Furthermore, we have the case where deprecated headers include headers in internal modules. I just have to turn off include checking for those, but that's not a big deal. This CL punts the problem of callfactoryinterface.h being implicitly included, and pulling in most of the call module with it. This should be addressed in a follow-up CL. Bug: webrtc:7504 Change-Id: I1b1729408158418333ccdf702bf529386090f0d7 Reviewed-on: https://webrtc-review.googlesource.com/2020 Commit-Queue: Patrik Höglund Reviewed-by: Fredrik Solenberg Reviewed-by: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#20034} --- api/BUILD.gn | 47 ++++++++++++++++++++------- {p2p/base => api}/candidate.h | 30 +++++------------ api/jsepicecandidate.h | 2 +- api/jsepsessiondescription.h | 2 +- api/mediastreaminterface.h | 1 - p2p/BUILD.gn | 1 - p2p/base/fakecandidatepair.h | 2 +- p2p/base/icetransportinternal.h | 2 +- p2p/base/jseptransport.cc | 2 +- p2p/base/jseptransport.h | 2 +- p2p/base/p2ptransportchannel.cc | 2 +- p2p/base/p2ptransportchannel.h | 2 +- p2p/base/port.h | 2 +- p2p/base/transportcontroller.h | 2 +- p2p/base/transportinfo.h | 2 +- pc/jsepsessiondescription_unittest.cc | 2 +- pc/rtcstatscollector.cc | 2 +- pc/webrtcsdp.cc | 2 +- pc/webrtcsession.h | 2 +- rtc_base/BUILD.gn | 1 + rtc_base/network.h | 6 ---- rtc_base/network_constants.h | 36 ++++++++++++++++++++ rtc_base/networkmonitor.h | 11 +------ 23 files changed, 97 insertions(+), 66 deletions(-) rename {p2p/base => api}/candidate.h (92%) create mode 100644 rtc_base/network_constants.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 86a63e4dee..6c0c6fd6d7 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -34,12 +34,9 @@ rtc_source_set("call_api") { } rtc_static_library("libjingle_peerconnection_api") { - # Cannot have GN check enabled since that would introduce dependency cycles - # TODO(kjellander): Remove (bugs.webrtc.org/7504) - check_includes = false cflags = [] sources = [ - "datachannel.h", + "candidate.h", "datachannelinterface.h", "dtmfsenderinterface.h", "jsep.h", @@ -47,17 +44,14 @@ rtc_static_library("libjingle_peerconnection_api") { "jsepsessiondescription.h", "mediaconstraintsinterface.cc", "mediaconstraintsinterface.h", - "mediastream.h", "mediastreaminterface.cc", "mediastreaminterface.h", "mediastreamproxy.h", - "mediastreamtrack.h", "mediastreamtrackproxy.h", "mediatypes.cc", "mediatypes.h", "notifier.h", "peerconnectionfactoryproxy.h", - "peerconnectioninterface.h", "peerconnectionproxy.h", "proxy.h", "rtcerror.cc", @@ -65,15 +59,11 @@ rtc_static_library("libjingle_peerconnection_api") { "rtpparameters.cc", "rtpparameters.h", "rtpreceiverinterface.h", - "rtpsender.h", "rtpsenderinterface.h", "statstypes.cc", "statstypes.h", - "streamcollection.h", "umametrics.h", "videosourceproxy.h", - "videotracksource.h", - "webrtcsdp.h", ] if (!build_with_chromium && is_clang) { @@ -81,8 +71,15 @@ rtc_static_library("libjingle_peerconnection_api") { suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] } + public_deps = [ + ":libjingle_api_deprecated_headers", + ":peerconnection_and_implicit_call_api", + ] + deps = [ + ":optional", ":rtc_stats_api", + ":video_frame_api", "..:webrtc_common", "../rtc_base:rtc_base", "../rtc_base:rtc_base_approved", @@ -97,6 +94,34 @@ rtc_static_library("libjingle_peerconnection_api") { } } +rtc_source_set("peerconnection_and_implicit_call_api") { + # The peerconnectioninterface.h file pulls in call/callfactoryinterface.h + # and the entire call module with it. We need to either get rid of this + # dependency or pull most of call/ into the API. For now, silence the warnings + # this creates since it creates a circular dependency (call very much depends + # on API). See bugs.webrtc.org/7504. + check_includes = false + sources = [ + "peerconnectioninterface.h", + ] +} + +rtc_source_set("libjingle_api_deprecated_headers") { + # We need to include headers from undeclared targets here, since they cause + # circular dependencies. These deprecated headers are going away anyway. + # See http://bugs.webrtc.org/5883. + check_includes = false + sources = [ + "datachannel.h", + "mediastream.h", + "mediastreamtrack.h", + "rtpsender.h", + "streamcollection.h", + "videotracksource.h", + "webrtcsdp.h", + ] +} + rtc_source_set("ortc_api") { check_includes = false # TODO(deadbeef): Remove (bugs.webrtc.org/6828) sources = [ diff --git a/p2p/base/candidate.h b/api/candidate.h similarity index 92% rename from p2p/base/candidate.h rename to api/candidate.h index 87f24d22b8..78b20827e5 100644 --- a/p2p/base/candidate.h +++ b/api/candidate.h @@ -8,31 +8,28 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef P2P_BASE_CANDIDATE_H_ -#define P2P_BASE_CANDIDATE_H_ +#ifndef API_CANDIDATE_H_ +#define API_CANDIDATE_H_ #include -#include #include #include -#include -#include #include -#include "p2p/base/p2pconstants.h" #include "rtc_base/checks.h" #include "rtc_base/helpers.h" -#include "rtc_base/network.h" +#include "rtc_base/network_constants.h" #include "rtc_base/socketaddress.h" namespace cricket { // Candidate for ICE based connection discovery. +// TODO(phoglund): remove things in here that are not needed in the public API. class Candidate { public: - // TODO: Match the ordering and param list as per RFC 5245 + // TODO(pthatcher): Match the ordering and param list as per RFC 5245 // candidate-attribute syntax. http://tools.ietf.org/html/rfc5245#section-15.1 Candidate() : id_(rtc::CreateRandomString(8)), @@ -173,11 +170,12 @@ class Candidate { related_address_ = related_address; } const std::string& tcptype() const { return tcptype_; } - void set_tcptype(const std::string& tcptype){ + void set_tcptype(const std::string& tcptype) { tcptype_ = tcptype; } // The name of the transport channel of this candidate. + // TODO(phoglund): remove. const std::string& transport_name() const { return transport_name_; } void set_transport_name(const std::string& transport_name) { transport_name_ = transport_name; @@ -290,18 +288,6 @@ class Candidate { std::string url_; }; -// Used during parsing and writing to map component to channel name -// and back. This is primarily for converting old G-ICE candidate -// signalling to new ICE candidate classes. -class CandidateTranslator { - public: - virtual ~CandidateTranslator() {} - virtual bool GetChannelNameFromComponent( - int component, std::string* channel_name) const = 0; - virtual bool GetComponentFromChannelName( - const std::string& channel_name, int* component) const = 0; -}; - } // namespace cricket -#endif // P2P_BASE_CANDIDATE_H_ +#endif // API_CANDIDATE_H_ diff --git a/api/jsepicecandidate.h b/api/jsepicecandidate.h index 2965e866af..dae6121ead 100644 --- a/api/jsepicecandidate.h +++ b/api/jsepicecandidate.h @@ -18,8 +18,8 @@ #include #include +#include "api/candidate.h" #include "api/jsep.h" -#include "p2p/base/candidate.h" #include "rtc_base/constructormagic.h" namespace webrtc { diff --git a/api/jsepsessiondescription.h b/api/jsepsessiondescription.h index 21adfeefc0..6b115ee2e9 100644 --- a/api/jsepsessiondescription.h +++ b/api/jsepsessiondescription.h @@ -18,9 +18,9 @@ #include #include +#include "api/candidate.h" #include "api/jsep.h" #include "api/jsepicecandidate.h" -#include "p2p/base/candidate.h" #include "rtc_base/constructormagic.h" namespace cricket { diff --git a/api/mediastreaminterface.h b/api/mediastreaminterface.h index 4e5fde1822..fa074a0533 100644 --- a/api/mediastreaminterface.h +++ b/api/mediastreaminterface.h @@ -27,7 +27,6 @@ // TODO(zhihuang): Remove unrelated headers once downstream applications stop // relying on them; they were previously transitively included by // mediachannel.h, which is no longer a dependency of this file. -#include "media/base/streamparams.h" #include "media/base/videosinkinterface.h" #include "media/base/videosourceinterface.h" #include "rtc_base/ratetracker.h" diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 9223f865b4..0e1e3ae52f 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -25,7 +25,6 @@ rtc_static_library("rtc_p2p") { "base/asyncstuntcpsocket.h", "base/basicpacketsocketfactory.cc", "base/basicpacketsocketfactory.h", - "base/candidate.h", "base/common.h", "base/dtlstransport.cc", "base/dtlstransport.h", diff --git a/p2p/base/fakecandidatepair.h b/p2p/base/fakecandidatepair.h index 17387892fb..c49ac479bc 100644 --- a/p2p/base/fakecandidatepair.h +++ b/p2p/base/fakecandidatepair.h @@ -13,7 +13,7 @@ #include -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/candidatepairinterface.h" namespace cricket { diff --git a/p2p/base/icetransportinternal.h b/p2p/base/icetransportinternal.h index 5c67df9da6..48f8806277 100644 --- a/p2p/base/icetransportinternal.h +++ b/p2p/base/icetransportinternal.h @@ -13,7 +13,7 @@ #include -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/candidatepairinterface.h" #include "p2p/base/jseptransport.h" #include "p2p/base/packettransportinternal.h" diff --git a/p2p/base/jseptransport.cc b/p2p/base/jseptransport.cc index 74094fa07f..fbb6287ab2 100644 --- a/p2p/base/jseptransport.cc +++ b/p2p/base/jseptransport.cc @@ -13,7 +13,7 @@ #include #include // for std::pair -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/dtlstransport.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/p2ptransportchannel.h" diff --git a/p2p/base/jseptransport.h b/p2p/base/jseptransport.h index 3486342a84..4cf9695ece 100644 --- a/p2p/base/jseptransport.h +++ b/p2p/base/jseptransport.h @@ -16,8 +16,8 @@ #include #include +#include "api/candidate.h" #include "api/optional.h" -#include "p2p/base/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/sessiondescription.h" #include "p2p/base/transportinfo.h" diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc index 44f8eee8ef..c02bac7e66 100644 --- a/p2p/base/p2ptransportchannel.cc +++ b/p2p/base/p2ptransportchannel.cc @@ -14,8 +14,8 @@ #include #include +#include "api/candidate.h" #include "api/umametrics.h" -#include "p2p/base/candidate.h" #include "p2p/base/candidatepairinterface.h" #include "p2p/base/common.h" #include "p2p/base/relayport.h" // For RELAY_PORT_TYPE. diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h index b6a84bec94..56ca57e11e 100644 --- a/p2p/base/p2ptransportchannel.h +++ b/p2p/base/p2ptransportchannel.h @@ -26,7 +26,7 @@ #include #include -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/candidatepairinterface.h" #include "p2p/base/icetransportinternal.h" #include "p2p/base/portallocator.h" diff --git a/p2p/base/port.h b/p2p/base/port.h index 65f4d79f12..61fd750691 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -17,8 +17,8 @@ #include #include +#include "api/candidate.h" #include "api/optional.h" -#include "p2p/base/candidate.h" #include "p2p/base/candidatepairinterface.h" #include "p2p/base/jseptransport.h" #include "p2p/base/packetlossestimator.h" diff --git a/p2p/base/transportcontroller.h b/p2p/base/transportcontroller.h index b7c38f3a07..2a65eef35b 100644 --- a/p2p/base/transportcontroller.h +++ b/p2p/base/transportcontroller.h @@ -16,7 +16,7 @@ #include #include -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/dtlstransport.h" #include "p2p/base/jseptransport.h" #include "p2p/base/p2ptransportchannel.h" diff --git a/p2p/base/transportinfo.h b/p2p/base/transportinfo.h index 3657b8dce5..3502423c12 100644 --- a/p2p/base/transportinfo.h +++ b/p2p/base/transportinfo.h @@ -14,7 +14,7 @@ #include #include -#include "p2p/base/candidate.h" +#include "api/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/transportdescription.h" #include "rtc_base/helpers.h" diff --git a/pc/jsepsessiondescription_unittest.cc b/pc/jsepsessiondescription_unittest.cc index 781b59690a..8e3c10f7c5 100644 --- a/pc/jsepsessiondescription_unittest.cc +++ b/pc/jsepsessiondescription_unittest.cc @@ -11,10 +11,10 @@ #include #include +#include "api/candidate.h" #include "api/jsepicecandidate.h" #include "api/jsepsessiondescription.h" #include "api/webrtcsdp.h" -#include "p2p/base/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/sessiondescription.h" #include "pc/mediasession.h" diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc index 8e6e420be1..6a234a4b1d 100644 --- a/pc/rtcstatscollector.cc +++ b/pc/rtcstatscollector.cc @@ -15,10 +15,10 @@ #include #include +#include "api/candidate.h" #include "api/mediastreaminterface.h" #include "api/peerconnectioninterface.h" #include "media/base/mediachannel.h" -#include "p2p/base/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/port.h" #include "pc/peerconnection.h" diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc index 648d78c76b..47e429863e 100644 --- a/pc/webrtcsdp.cc +++ b/pc/webrtcsdp.cc @@ -20,6 +20,7 @@ #include #include +#include "api/candidate.h" #include "api/jsepicecandidate.h" #include "api/jsepsessiondescription.h" // for RtpExtension @@ -29,7 +30,6 @@ #include "media/base/mediaconstants.h" #include "media/base/rtputils.h" #include "media/sctp/sctptransportinternal.h" -#include "p2p/base/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/port.h" #include "pc/mediasession.h" diff --git a/pc/webrtcsession.h b/pc/webrtcsession.h index 7e8d5e4779..fd6e592d5b 100644 --- a/pc/webrtcsession.h +++ b/pc/webrtcsession.h @@ -16,11 +16,11 @@ #include #include +#include "api/candidate.h" #include "api/optional.h" #include "api/peerconnectioninterface.h" #include "api/statstypes.h" #include "call/call.h" -#include "p2p/base/candidate.h" #include "p2p/base/transportcontroller.h" #include "pc/datachannel.h" #include "pc/mediasession.h" diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 93f8990ccc..354c320024 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -526,6 +526,7 @@ rtc_static_library("rtc_base_generic") { "nethelpers.h", "network.cc", "network.h", + "network_constants.h", "networkmonitor.cc", "networkmonitor.h", "nullsocketserver.cc", diff --git a/rtc_base/network.h b/rtc_base/network.h index b3b43d834f..6bddf4626a 100644 --- a/rtc_base/network.h +++ b/rtc_base/network.h @@ -38,12 +38,6 @@ class Network; class NetworkMonitorInterface; class Thread; -static const uint16_t kNetworkCostMax = 999; -static const uint16_t kNetworkCostHigh = 900; -static const uint16_t kNetworkCostUnknown = 50; -static const uint16_t kNetworkCostLow = 10; -static const uint16_t kNetworkCostMin = 0; - // By default, ignore loopback interfaces on the host. const int kDefaultNetworkIgnoreMask = ADAPTER_TYPE_LOOPBACK; diff --git a/rtc_base/network_constants.h b/rtc_base/network_constants.h new file mode 100644 index 0000000000..b4c8beaf31 --- /dev/null +++ b/rtc_base/network_constants.h @@ -0,0 +1,36 @@ +/* + * Copyright 2004 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef RTC_BASE_NETWORK_CONSTANTS_H_ +#define RTC_BASE_NETWORK_CONSTANTS_H_ + +#include + +namespace rtc { + +static const uint16_t kNetworkCostMax = 999; +static const uint16_t kNetworkCostHigh = 900; +static const uint16_t kNetworkCostUnknown = 50; +static const uint16_t kNetworkCostLow = 10; +static const uint16_t kNetworkCostMin = 0; + +enum AdapterType { + // This enum resembles the one in Chromium net::ConnectionType. + ADAPTER_TYPE_UNKNOWN = 0, + ADAPTER_TYPE_ETHERNET = 1 << 0, + ADAPTER_TYPE_WIFI = 1 << 1, + ADAPTER_TYPE_CELLULAR = 1 << 2, + ADAPTER_TYPE_VPN = 1 << 3, + ADAPTER_TYPE_LOOPBACK = 1 << 4 +}; + +} // namespace rtc + +#endif // RTC_BASE_NETWORK_CONSTANTS_H_ diff --git a/rtc_base/networkmonitor.h b/rtc_base/networkmonitor.h index d98b803169..254b22575a 100644 --- a/rtc_base/networkmonitor.h +++ b/rtc_base/networkmonitor.h @@ -12,6 +12,7 @@ #define RTC_BASE_NETWORKMONITOR_H_ #include "rtc_base/logging.h" +#include "rtc_base/network_constants.h" #include "rtc_base/sigslot.h" #include "rtc_base/thread.h" @@ -27,16 +28,6 @@ enum class NetworkBindingResult { NETWORK_CHANGED = -4 }; -enum AdapterType { - // This enum resembles the one in Chromium net::ConnectionType. - ADAPTER_TYPE_UNKNOWN = 0, - ADAPTER_TYPE_ETHERNET = 1 << 0, - ADAPTER_TYPE_WIFI = 1 << 1, - ADAPTER_TYPE_CELLULAR = 1 << 2, - ADAPTER_TYPE_VPN = 1 << 3, - ADAPTER_TYPE_LOOPBACK = 1 << 4 -}; - class NetworkBinderInterface { public: // Binds a socket to the network that is attached to |address| so that all