Commit 1e8ff9d4 authored by Henrik Boström's avatar Henrik Boström Committed by Commit Bot

Keep track of SDP semantics, not including kDefault.

SDP semantics can be passed in to the PC as kPlanB, kUnifiedPlan or
kDefault.

This CL does the translation from kDefault to kPlanB (default) or
kUnifiedPlan (if a flag is used to overwrite the default) at
construction once and for all. This eliminates assumptions about what
kDefault may mean in different parts of the code or layers.

RTCPeerConnectionHandler is also updated to keep track of the SDP
semantics. Also passes down "sdp_semantics_requested" for the sake of
UMA counters which is the exception where we do care if "kDefault" was
used or not by the application.

This CL also updates some Plan B-only methods to DCHECK SDP semantics
or renames them to "...PlanB()".

// Bots have already been successful, skipping remaining bot which is
// inactive.
NOTRY=True

Bug: 777617
Change-Id: If88017360fa577297796bac4faf4e7bc56bd99d0
Reviewed-on: https://chromium-review.googlesource.com/1141880
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576171}
parent e0c66ca5
......@@ -20,10 +20,10 @@ MockWebRTCPeerConnectionHandlerClient()
.WillByDefault(
testing::Invoke(this, &MockWebRTCPeerConnectionHandlerClient::
didGenerateICECandidateWorker));
ON_CALL(*this, DidAddReceiverForMock(_))
ON_CALL(*this, DidAddReceiverPlanBForMock(_))
.WillByDefault(testing::Invoke(
this, &MockWebRTCPeerConnectionHandlerClient::didAddReceiverWorker));
ON_CALL(*this, DidRemoveReceiverForMock(_))
ON_CALL(*this, DidRemoveReceiverPlanBForMock(_))
.WillByDefault(testing::Invoke(
this,
&MockWebRTCPeerConnectionHandlerClient::didRemoveReceiverWorker));
......
......@@ -31,22 +31,22 @@ class MockWebRTCPeerConnectionHandlerClient
void(webrtc::PeerConnectionInterface::SignalingState state));
MOCK_METHOD1(DidChangeICEGatheringState, void(ICEGatheringState state));
MOCK_METHOD1(DidChangeICEConnectionState, void(ICEConnectionState state));
void DidAddReceiver(
void DidAddReceiverPlanB(
std::unique_ptr<blink::WebRTCRtpReceiver> web_rtp_receiver) override {
DidAddReceiverForMock(&web_rtp_receiver);
DidAddReceiverPlanBForMock(&web_rtp_receiver);
}
void DidRemoveReceiver(
void DidRemoveReceiverPlanB(
std::unique_ptr<blink::WebRTCRtpReceiver> web_rtp_receiver) override {
DidRemoveReceiverForMock(&web_rtp_receiver);
DidRemoveReceiverPlanBForMock(&web_rtp_receiver);
}
MOCK_METHOD1(DidAddRemoteDataChannel, void(blink::WebRTCDataChannelHandler*));
MOCK_METHOD0(ReleasePeerConnectionHandler, void());
// Move-only arguments do not play nicely with MOCK, the workaround is to
// EXPECT_CALL with these instead.
MOCK_METHOD1(DidAddReceiverForMock,
MOCK_METHOD1(DidAddReceiverPlanBForMock,
void(std::unique_ptr<blink::WebRTCRtpReceiver>*));
MOCK_METHOD1(DidRemoveReceiverForMock,
MOCK_METHOD1(DidRemoveReceiverPlanBForMock,
void(std::unique_ptr<blink::WebRTCRtpReceiver>*));
void didGenerateICECandidateWorker(
......
......@@ -26,6 +26,7 @@
#include "content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h"
#include "ipc/ipc_platform_file.h"
#include "third_party/blink/public/platform/web_media_stream_source.h"
#include "third_party/blink/public/platform/web_rtc_configuration.h"
#include "third_party/blink/public/platform/web_rtc_peer_connection_handler.h"
#include "third_party/blink/public/platform/web_rtc_stats_request.h"
#include "third_party/blink/public/platform/web_rtc_stats_response.h"
......@@ -107,8 +108,10 @@ class CONTENT_EXPORT RTCPeerConnectionHandler
const base::WeakPtr<PeerConnectionTracker>& peer_connection_tracker);
// blink::WebRTCPeerConnectionHandler implementation
bool Initialize(const blink::WebRTCConfiguration& server_configuration,
const blink::WebMediaConstraints& options) override;
bool Initialize(
const blink::WebRTCConfiguration& server_configuration,
const blink::WebMediaConstraints& options,
blink::WebRTCSdpSemantics original_sdp_semantics_value) override;
void CreateOffer(const blink::WebRTCSessionDescriptionRequest& request,
const blink::WebMediaConstraints& options) override;
......@@ -197,8 +200,8 @@ class CONTENT_EXPORT RTCPeerConnectionHandler
void OnIceGatheringChange(
webrtc::PeerConnectionInterface::IceGatheringState new_state);
void OnRenegotiationNeeded();
void OnAddReceiver(RtpReceiverState receiver_state);
void OnRemoveReceiver(uintptr_t receiver_id);
void OnAddReceiverPlanB(RtpReceiverState receiver_state);
void OnRemoveReceiverPlanB(uintptr_t receiver_id);
void OnDataChannel(std::unique_ptr<RtcDataChannelHandler> handler);
void OnIceCandidate(const std::string& sdp,
const std::string& sdp_mid,
......@@ -279,6 +282,14 @@ class CONTENT_EXPORT RTCPeerConnectionHandler
// needs to reference it, and automatically disposed when there are no longer
// any components referencing it.
scoped_refptr<WebRtcMediaStreamTrackAdapterMap> track_adapter_map_;
// In Plan B, senders and receivers are added or removed independently of one
// another. In Unified Plan, senders and receivers are created in pairs as
// transceivers. Transceivers may become inactive, but are never removed.
// The value of this member affects the behavior of some methods and what
// information is surfaced from webrtc. After Initialize(), this is the actual
// mode used, meaning "kDefault" is no longer a valid value.
// TODO(hbos): Implement transceiver behaviors. https://crbug.com/777617
blink::WebRTCSdpSemantics sdp_semantics_;
// Content layer correspondents of |webrtc::RtpSenderInterface|.
std::vector<std::unique_ptr<RTCRtpSender>> rtp_senders_;
// Maps |RTCRtpReceiver::getId|s of |webrtc::RtpReceiverInterface|s to the
......
......@@ -280,6 +280,7 @@ class RTCPeerConnectionHandlerTest : public ::testing::Test {
pc_handler_ = CreateRTCPeerConnectionHandlerUnderTest();
mock_tracker_.reset(new NiceMock<MockPeerConnectionTracker>());
blink::WebRTCConfiguration config;
config.sdp_semantics = blink::WebRTCSdpSemantics::kPlanB;
blink::WebMediaConstraints constraints;
EXPECT_TRUE(pc_handler_->InitializeForTest(
config, constraints, mock_tracker_.get()->AsWeakPtr()));
......@@ -609,12 +610,12 @@ TEST_F(RTCPeerConnectionHandlerTest, NoCallbacksToClientAfterStop) {
pc_handler_->observer()->OnIceConnectionChange(
webrtc::PeerConnectionInterface::kIceConnectionDisconnected);
EXPECT_CALL(*mock_client_.get(), DidAddReceiverForMock(_)).Times(0);
EXPECT_CALL(*mock_client_.get(), DidAddReceiverPlanBForMock(_)).Times(0);
rtc::scoped_refptr<webrtc::MediaStreamInterface> remote_stream(
AddRemoteMockMediaStream("remote_stream", "video", "audio"));
InvokeOnAddStream(remote_stream);
EXPECT_CALL(*mock_client_.get(), DidRemoveReceiverForMock(_)).Times(0);
EXPECT_CALL(*mock_client_.get(), DidRemoveReceiverPlanBForMock(_)).Times(0);
InvokeOnRemoveStream(remote_stream);
EXPECT_CALL(*mock_client_.get(), DidAddRemoteDataChannel(_)).Times(0);
......@@ -764,6 +765,7 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescriptionParseError) {
TEST_F(RTCPeerConnectionHandlerTest, setConfiguration) {
blink::WebRTCConfiguration config;
config.sdp_semantics = blink::WebRTCSdpSemantics::kPlanB;
EXPECT_CALL(*mock_tracker_.get(),
TrackSetConfiguration(pc_handler_.get(), _));
......@@ -777,6 +779,7 @@ TEST_F(RTCPeerConnectionHandlerTest, setConfiguration) {
// blink error and false is returned.
TEST_F(RTCPeerConnectionHandlerTest, setConfigurationError) {
blink::WebRTCConfiguration config;
config.sdp_semantics = blink::WebRTCSdpSemantics::kPlanB;
mock_peer_connection_->set_setconfiguration_error_type(
webrtc::RTCErrorType::INVALID_MODIFICATION);
......@@ -1170,7 +1173,7 @@ TEST_F(RTCPeerConnectionHandlerTest, DISABLED_OnAddAndOnRemoveStream) {
AddRemoteMockMediaStream("remote_stream", "video", "audio"));
// Grab the added receivers when it's been successfully added to the PC.
std::vector<std::unique_ptr<blink::WebRTCRtpReceiver>> receivers_added;
EXPECT_CALL(*mock_client_.get(), DidAddReceiverForMock(_))
EXPECT_CALL(*mock_client_.get(), DidAddReceiverPlanBForMock(_))
.WillRepeatedly(
Invoke([&receivers_added](
std::unique_ptr<blink::WebRTCRtpReceiver>* receiver) {
......@@ -1190,7 +1193,7 @@ TEST_F(RTCPeerConnectionHandlerTest, DISABLED_OnAddAndOnRemoveStream) {
pc_handler_.get(),
PeerConnectionTracker::TransceiverUpdatedReason::kRemoveTrack, _, _))
.Times(2);
EXPECT_CALL(*mock_client_.get(), DidRemoveReceiverForMock(_))
EXPECT_CALL(*mock_client_.get(), DidRemoveReceiverPlanBForMock(_))
.WillRepeatedly(
Invoke([&receivers_removed](
std::unique_ptr<blink::WebRTCRtpReceiver>* receiver) {
......
This is a testharness.js-based test.
PASS Constructing RTCPeerConnection with expired certificate should reject with InvalidAccessError
FAIL Calling setConfiguration with different set of certs should reject with InvalidModificationError promise_test: Unhandled rejection with value: object "InvalidModificationError: Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way."
PASS RTCCertificate should have at least one fingerprint
PASS RTCPeerConnection({ certificates }) should generate offer SDP with fingerprint of provided certificate
FAIL RTCPeerConnection({ certificates }) should generate offer SDP with fingerprint of all provided certificates assert_true: Expect fingerprint line to be found in SDP expected true got false
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Default bundlePolicy should be balanced pc.getConfiguration is not a function
FAIL new RTCPeerConnection({ bundlePolicy: undefined }) should have bundlePolicy balanced pc.getConfiguration is not a function
FAIL new RTCPeerConnection({ bundlePolicy: 'balanced' }) should succeed pc.getConfiguration is not a function
FAIL new RTCPeerConnection({ bundlePolicy: 'max-compat' }) should succeed pc.getConfiguration is not a function
FAIL new RTCPeerConnection({ bundlePolicy: 'max-bundle' }) should succeed pc.getConfiguration is not a function
FAIL setConfiguration({}) with initial default bundlePolicy balanced should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
FAIL setConfiguration({}) with initial bundlePolicy balanced should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
FAIL setConfiguration({ bundlePolicy: balanced }) with initial default bundlePolicy balanced should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
FAIL setConfiguration({ bundlePolicy: 'balanced' }) with initial bundlePolicy balanced should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
FAIL setConfiguration({ bundlePolicy: 'max-compat' }) with initial bundlePolicy max-compat should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
FAIL setConfiguration({ bundlePolicy: 'max-bundle' }) with initial bundlePolicy max-bundle should succeed Failed to execute 'setConfiguration' on 'RTCPeerConnection': Attempted to modify the PeerConnection's configuration in an unsupported way.
PASS new RTCPeerConnection({ bundlePolicy: null }) should throw TypeError
PASS new RTCPeerConnection({ bundlePolicy: 'invalid' }) should throw TypeError
PASS setConfiguration({ bundlePolicy: 'max-compat' }) with initial bundlePolicy max-bundle should throw InvalidModificationError
PASS setConfiguration({}) with initial bundlePolicy max-bundle should throw InvalidModificationError
Harness: the test ran to completion.
......@@ -31,6 +31,7 @@
#ifndef THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_RTC_PEER_CONNECTION_HANDLER_H_
#define THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_RTC_PEER_CONNECTION_HANDLER_H_
#include "third_party/blink/public/platform/web_rtc_configuration.h"
#include "third_party/blink/public/platform/web_rtc_ice_candidate.h"
#include "third_party/blink/public/platform/web_rtc_rtp_transceiver.h"
#include "third_party/blink/public/platform/web_rtc_stats.h"
......@@ -63,7 +64,8 @@ class WebRTCPeerConnectionHandler {
virtual ~WebRTCPeerConnectionHandler() = default;
virtual bool Initialize(const WebRTCConfiguration&,
const WebMediaConstraints&) = 0;
const WebMediaConstraints&,
WebRTCSdpSemantics original_sdp_semantics_value) = 0;
virtual void CreateOffer(const WebRTCSessionDescriptionRequest&,
const WebMediaConstraints&) = 0;
......
......@@ -72,8 +72,8 @@ class BLINK_PLATFORM_EXPORT WebRTCPeerConnectionHandlerClient {
webrtc::PeerConnectionInterface::SignalingState) = 0;
virtual void DidChangeICEGatheringState(ICEGatheringState) = 0;
virtual void DidChangeICEConnectionState(ICEConnectionState) = 0;
virtual void DidAddReceiver(std::unique_ptr<WebRTCRtpReceiver>) = 0;
virtual void DidRemoveReceiver(std::unique_ptr<WebRTCRtpReceiver>) = 0;
virtual void DidAddReceiverPlanB(std::unique_ptr<WebRTCRtpReceiver>) = 0;
virtual void DidRemoveReceiverPlanB(std::unique_ptr<WebRTCRtpReceiver>) = 0;
virtual void DidAddRemoteDataChannel(WebRTCDataChannelHandler*) = 0;
virtual void ReleasePeerConnectionHandler() = 0;
virtual void ClosePeerConnection();
......
......@@ -487,11 +487,6 @@ RTCPeerConnection* RTCPeerConnection::Create(
ParseConfiguration(context, rtc_configuration, exception_state);
if (exception_state.HadException())
return nullptr;
// Override default SDP semantics if RuntimeEnabled=RTCUnifiedPlanByDefault.
if (!rtc_configuration.hasSdpSemantics() &&
RuntimeEnabledFeatures::RTCUnifiedPlanByDefaultEnabled()) {
configuration.sdp_semantics = WebRTCSdpSemantics::kUnifiedPlan;
}
// Make sure no certificates have expired.
if (configuration.certificates.size() > 0) {
......@@ -516,7 +511,7 @@ RTCPeerConnection* RTCPeerConnection::Create(
}
RTCPeerConnection* peer_connection = new RTCPeerConnection(
context, configuration, constraints, exception_state);
context, std::move(configuration), constraints, exception_state);
peer_connection->PauseIfNeeded();
if (exception_state.HadException())
return nullptr;
......@@ -525,7 +520,7 @@ RTCPeerConnection* RTCPeerConnection::Create(
}
RTCPeerConnection::RTCPeerConnection(ExecutionContext* context,
const WebRTCConfiguration& configuration,
WebRTCConfiguration configuration,
WebMediaConstraints constraints,
ExceptionState& exception_state)
: PausableObject(context),
......@@ -545,6 +540,24 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context,
closed_(false),
has_data_channels_(false),
sdp_semantics_(configuration.sdp_semantics) {
// The original SDP semantics value is the value passed in by the caller,
// which if not specified has the value "kDefault". For |sdp_semantics_| this
// is translated to the actual SDP semantics used ("kPlanB" or
// "kUnifiedPlan"), but for the sake of UMA use counters it is of interest to
// know if the caller did not explicitly set it.
WebRTCSdpSemantics original_sdp_semantics_value = sdp_semantics_;
if (sdp_semantics_ == WebRTCSdpSemantics::kDefault) {
if (!RuntimeEnabledFeatures::RTCUnifiedPlanByDefaultEnabled()) {
// By default: The default SDP semantics is: "kPlanB".
sdp_semantics_ = configuration.sdp_semantics = WebRTCSdpSemantics::kPlanB;
} else {
// Override default SDP semantics to "kUnifiedPlan" with
// RuntimeEnabled=RTCUnifiedPlanByDefault.
sdp_semantics_ = configuration.sdp_semantics =
WebRTCSdpSemantics::kUnifiedPlan;
}
}
Document* document = ToDocument(GetExecutionContext());
InstanceCounters::IncrementCounter(
......@@ -582,7 +595,8 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context,
document->GetFrame()->Client()->DispatchWillStartUsingPeerConnectionHandler(
peer_handler_.get());
if (!peer_handler_->Initialize(configuration, constraints)) {
if (!peer_handler_->Initialize(configuration, constraints,
original_sdp_semantics_value)) {
closed_ = true;
stopped_ = true;
exception_state.ThrowDOMException(
......@@ -939,6 +953,12 @@ void RTCPeerConnection::setConfiguration(
WebRTCConfiguration configuration = ParseConfiguration(
ExecutionContext::From(script_state), rtc_configuration, exception_state);
// Overwrite "kDefault" with |sdp_semantics_| as set in the constructor to
// avoid duplicate logic for interpreting the default and SetConfiguration()
// failing if different layers have different notions of default.
if (configuration.sdp_semantics == blink::WebRTCSdpSemantics::kDefault) {
configuration.sdp_semantics = sdp_semantics_;
}
if (exception_state.HadException())
return;
......@@ -1453,11 +1473,7 @@ RTCRtpSender* RTCPeerConnection::addTrack(MediaStreamTrack* track,
DCHECK(track->Component());
if (ThrowExceptionIfSignalingStateClosed(signaling_state_, exception_state))
return nullptr;
// TODO(bugs.webrtc.org/8530): Take out WebRTCSdpSemantics::kDefault check
// once default is no longer interpreted as Plan B lower down.
if ((sdp_semantics_ == WebRTCSdpSemantics::kPlanB ||
sdp_semantics_ == WebRTCSdpSemantics::kDefault) &&
streams.size() >= 2) {
if (sdp_semantics_ == WebRTCSdpSemantics::kPlanB && streams.size() >= 2) {
// TODO(hbos): Update peer_handler_ to call the AddTrack() that returns the
// appropriate errors, and let the lower layers handle it.
exception_state.ThrowDOMException(
......@@ -1739,7 +1755,7 @@ void RTCPeerConnection::DidChangeICEConnectionState(
ChangeIceConnectionState(new_state);
}
void RTCPeerConnection::DidAddReceiver(
void RTCPeerConnection::DidAddReceiverPlanB(
std::unique_ptr<WebRTCRtpReceiver> web_receiver) {
DCHECK(!closed_);
DCHECK(GetExecutionContext()->IsContextThread());
......@@ -1794,7 +1810,7 @@ void RTCPeerConnection::DidAddReceiver(
new RTCTrackEvent(rtp_receiver, rtp_receiver->track(), streams));
}
void RTCPeerConnection::DidRemoveReceiver(
void RTCPeerConnection::DidRemoveReceiverPlanB(
std::unique_ptr<WebRTCRtpReceiver> web_receiver) {
DCHECK(!closed_);
DCHECK(GetExecutionContext()->IsContextThread());
......
......@@ -190,6 +190,11 @@ class MODULES_EXPORT RTCPeerConnection final
bool IsClosed() { return closed_; }
void close();
// Makes the peer connection aware of the track. This is used to map web
// tracks to blink tracks, as is necessary for plumbing. There is no need to
// unregister the track because Weak references are used.
void RegisterTrack(MediaStreamTrack*);
// We allow getStats after close, but not other calls or callbacks.
bool ShouldFireDefaultCallbacks() { return !closed_ && !stopped_; }
bool ShouldFireGetStatsCallback() { return !stopped_; }
......@@ -217,8 +222,8 @@ class MODULES_EXPORT RTCPeerConnection final
webrtc::PeerConnectionInterface::SignalingState) override;
void DidChangeICEGatheringState(ICEGatheringState) override;
void DidChangeICEConnectionState(ICEConnectionState) override;
void DidAddReceiver(std::unique_ptr<WebRTCRtpReceiver>) override;
void DidRemoveReceiver(std::unique_ptr<WebRTCRtpReceiver>) override;
void DidAddReceiverPlanB(std::unique_ptr<WebRTCRtpReceiver>) override;
void DidRemoveReceiverPlanB(std::unique_ptr<WebRTCRtpReceiver>) override;
void DidAddRemoteDataChannel(WebRTCDataChannelHandler*) override;
void ReleasePeerConnectionHandler() override;
void ClosePeerConnection() override;
......@@ -269,7 +274,7 @@ class MODULES_EXPORT RTCPeerConnection final
};
RTCPeerConnection(ExecutionContext*,
const WebRTCConfiguration&,
WebRTCConfiguration,
WebMediaConstraints,
ExceptionState&);
void Dispose();
......@@ -355,6 +360,13 @@ class MODULES_EXPORT RTCPeerConnection final
String last_answer_;
bool has_data_channels_; // For RAPPOR metrics
// In Plan B, senders and receivers are added or removed independently of one
// another. In Unified Plan, senders and receivers are created in pairs as
// transceivers. Transceivers may become inactive, but are never removed.
// The value of this member affects the behavior of some methods and what
// information is surfaced from webrtc. This has the value "kPlanB" or
// "kUnifiedPlan", if constructed with "kDefault" it is translated to one or
// the other.
WebRTCSdpSemantics sdp_semantics_;
};
......
......@@ -83,7 +83,8 @@ MockWebRTCPeerConnectionHandler::MockWebRTCPeerConnectionHandler() = default;
MockWebRTCPeerConnectionHandler::~MockWebRTCPeerConnectionHandler() = default;
bool MockWebRTCPeerConnectionHandler::Initialize(const WebRTCConfiguration&,
const WebMediaConstraints&) {
const WebMediaConstraints&,
WebRTCSdpSemantics) {
return true;
}
......
......@@ -17,7 +17,8 @@ class MockWebRTCPeerConnectionHandler : public WebRTCPeerConnectionHandler {
~MockWebRTCPeerConnectionHandler() override;
bool Initialize(const WebRTCConfiguration&,
const WebMediaConstraints&) override;
const WebMediaConstraints&,
WebRTCSdpSemantics original_sdp_semantics_value) override;
void CreateOffer(const WebRTCSessionDescriptionRequest&,
const WebMediaConstraints&) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment