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

Revert "[Sheriff] Revert "[Perfect Negotiation] Surface session descriptions at the right time.""

This reverts commit 3dbd8ec2.

Reason for revert: I don't think this is the culprit and it looks like this test has been flaking for a couple of weeks, see:
https://analysis.chromium.org/p/chromium/flake-portal/flakes/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyWAsSBUZsYWtlIk1jaHJvbWl1bUBibGlua193ZWJfdGVzdHNAZXh0ZXJuYWwvd3B0L3dlYnJ0Yy9wcm90b2NvbC9jcnlwdG8tc3VpdGUuaHR0cHMuaHRtbAw

More details here: https://bugs.chromium.org/p/chromium/issues/detail?id=1122106

Original change's description:
> [Sheriff] Revert "[Perfect Negotiation] Surface session descriptions at the right time."
> 
> This reverts commit 3bc91ccc.
> 
> Reason for revert: Failure of WebKit Linux ASAN build, and that's the suspect for root cause. See https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20ASAN/17329
> 
> Original change's description:
> > [Perfect Negotiation] Surface session descriptions at the right time.
> > 
> > This fixes another timing related issue that blocks Perfect Negotiation.
> > 
> > Before this CL, current/pending local/remote description attributes do
> > blocking-invokes on the webrtc thread, fetching the most up-to-date
> > states. This can prematurely expose the result of "in-parallel"
> > operations that have not surfaced yet, such as:
> > - setLocalDescription
> > - setRemoteDescription
> > - addIceCandidate
> > 
> > This CL fixes that by copying the SDP states when any of these
> > operations complete on the WebRTC thread and carry them over in the
> > PostTask to the main thread. Here, we store these snapshots in
> > "internal slots" (variables living on the main thread). With this CL,
> > reading SDP attributes from RTCPeerConnection is non-blocking and
> > spec-compliant.
> > 
> > WPT test coverage added for the exact timing of SLD/SRD and other test
> > expectations are updated. addIceCandidate updating the SDP is already
> > covered by old WPTs.
> > 
> > Bug: chromium:1110347
> > Change-Id: Id41ec354465525c6cedf631fe2209fe097148f60
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2323359
> > Commit-Queue: Henrik Boström <hbos@chromium.org>
> > Reviewed-by: Harald Alvestrand <hta@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#801798}
> 
> TBR=hta@chromium.org,hbos@chromium.org
> 
> Change-Id: I4af12fd1430773ea3d134cfc37b9c5e736f20ed0
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: chromium:1110347
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377715
> Reviewed-by: Amr Aboelkher <amraboelkher@google.com>
> Commit-Queue: Amr Aboelkher <amraboelkher@google.com>
> Cr-Commit-Position: refs/heads/master@{#801822}

TBR=hta@chromium.org,hbos@chromium.org,amraboelkher@google.com

Change-Id: Ia0f82e0d1b4660cf0d3641aaa446127092c6e6f7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1110347
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379192Reviewed-by: default avatarHenrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802114}
parent 26b5f5d5
...@@ -35,6 +35,11 @@ class MockRTCPeerConnectionHandlerClient ...@@ -35,6 +35,11 @@ class MockRTCPeerConnectionHandlerClient
const String& url, const String& url,
int error_code, int error_code,
const String& error_text)); const String& error_text));
MOCK_METHOD4(DidChangeSessionDescriptions,
void(RTCSessionDescriptionPlatform*,
RTCSessionDescriptionPlatform*,
RTCSessionDescriptionPlatform*,
RTCSessionDescriptionPlatform*));
MOCK_METHOD1(DidChangeSignalingState, MOCK_METHOD1(DidChangeSignalingState,
void(webrtc::PeerConnectionInterface::SignalingState state)); void(webrtc::PeerConnectionInterface::SignalingState state));
MOCK_METHOD1(DidChangeIceGatheringState, MOCK_METHOD1(DidChangeIceGatheringState,
......
...@@ -286,36 +286,6 @@ void MockRTCPeerConnectionHandlerPlatform::SetRemoteDescription( ...@@ -286,36 +286,6 @@ void MockRTCPeerConnectionHandlerPlatform::SetRemoteDescription(
RTCVoidRequest*, RTCVoidRequest*,
RTCSessionDescriptionPlatform*) {} RTCSessionDescriptionPlatform*) {}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::LocalDescription() {
return nullptr;
}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::RemoteDescription() {
return nullptr;
}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::CurrentLocalDescription() {
return nullptr;
}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::CurrentRemoteDescription() {
return nullptr;
}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::PendingLocalDescription() {
return nullptr;
}
RTCSessionDescriptionPlatform*
MockRTCPeerConnectionHandlerPlatform::PendingRemoteDescription() {
return nullptr;
}
const webrtc::PeerConnectionInterface::RTCConfiguration& const webrtc::PeerConnectionInterface::RTCConfiguration&
MockRTCPeerConnectionHandlerPlatform::GetConfiguration() const { MockRTCPeerConnectionHandlerPlatform::GetConfiguration() const {
static const webrtc::PeerConnectionInterface::RTCConfiguration configuration; static const webrtc::PeerConnectionInterface::RTCConfiguration configuration;
......
...@@ -48,12 +48,6 @@ class MockRTCPeerConnectionHandlerPlatform : public RTCPeerConnectionHandler { ...@@ -48,12 +48,6 @@ class MockRTCPeerConnectionHandlerPlatform : public RTCPeerConnectionHandler {
RTCSessionDescriptionPlatform*) override; RTCSessionDescriptionPlatform*) override;
void SetRemoteDescription(RTCVoidRequest*, void SetRemoteDescription(RTCVoidRequest*,
RTCSessionDescriptionPlatform*) override; RTCSessionDescriptionPlatform*) override;
RTCSessionDescriptionPlatform* LocalDescription() override;
RTCSessionDescriptionPlatform* RemoteDescription() override;
RTCSessionDescriptionPlatform* CurrentLocalDescription() override;
RTCSessionDescriptionPlatform* CurrentRemoteDescription() override;
RTCSessionDescriptionPlatform* PendingLocalDescription() override;
RTCSessionDescriptionPlatform* PendingRemoteDescription() override;
const webrtc::PeerConnectionInterface::RTCConfiguration& GetConfiguration() const webrtc::PeerConnectionInterface::RTCConfiguration& GetConfiguration()
const override; const override;
webrtc::RTCErrorType SetConfiguration( webrtc::RTCErrorType SetConfiguration(
......
...@@ -746,6 +746,10 @@ RTCPeerConnection::RTCPeerConnection( ...@@ -746,6 +746,10 @@ RTCPeerConnection::RTCPeerConnection(
MediaConstraints constraints, MediaConstraints constraints,
ExceptionState& exception_state) ExceptionState& exception_state)
: ExecutionContextLifecycleObserver(context), : ExecutionContextLifecycleObserver(context),
pending_local_description_(nullptr),
current_local_description_(nullptr),
pending_remote_description_(nullptr),
current_remote_description_(nullptr),
signaling_state_( signaling_state_(
webrtc::PeerConnectionInterface::SignalingState::kStable), webrtc::PeerConnectionInterface::SignalingState::kStable),
ice_gathering_state_(webrtc::PeerConnectionInterface::kIceGatheringNew), ice_gathering_state_(webrtc::PeerConnectionInterface::kIceGatheringNew),
...@@ -1459,28 +1463,17 @@ ScriptPromise RTCPeerConnection::setLocalDescription( ...@@ -1459,28 +1463,17 @@ ScriptPromise RTCPeerConnection::setLocalDescription(
return ScriptPromise::CastUndefined(script_state); return ScriptPromise::CastUndefined(script_state);
} }
RTCSessionDescription* RTCPeerConnection::localDescription() { RTCSessionDescription* RTCPeerConnection::localDescription() const {
auto* platform_session_description = peer_handler_->LocalDescription(); return pending_local_description_ ? pending_local_description_
if (!platform_session_description) : current_local_description_;
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
RTCSessionDescription* RTCPeerConnection::currentLocalDescription() { RTCSessionDescription* RTCPeerConnection::currentLocalDescription() const {
auto* platform_session_description = peer_handler_->CurrentLocalDescription(); return current_local_description_;
if (!platform_session_description)
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
RTCSessionDescription* RTCPeerConnection::pendingLocalDescription() { RTCSessionDescription* RTCPeerConnection::pendingLocalDescription() const {
auto* platform_session_description = peer_handler_->PendingLocalDescription(); return pending_local_description_;
if (!platform_session_description)
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
ScriptPromise RTCPeerConnection::setRemoteDescription( ScriptPromise RTCPeerConnection::setRemoteDescription(
...@@ -1587,30 +1580,17 @@ ScriptPromise RTCPeerConnection::setRemoteDescription( ...@@ -1587,30 +1580,17 @@ ScriptPromise RTCPeerConnection::setRemoteDescription(
return ScriptPromise::CastUndefined(script_state); return ScriptPromise::CastUndefined(script_state);
} }
RTCSessionDescription* RTCPeerConnection::remoteDescription() { RTCSessionDescription* RTCPeerConnection::remoteDescription() const {
auto* platform_session_description = peer_handler_->RemoteDescription(); return pending_remote_description_ ? pending_remote_description_
if (!platform_session_description) : current_remote_description_;
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
RTCSessionDescription* RTCPeerConnection::currentRemoteDescription() { RTCSessionDescription* RTCPeerConnection::currentRemoteDescription() const {
auto* platform_session_description = return current_remote_description_;
peer_handler_->CurrentRemoteDescription();
if (!platform_session_description)
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
RTCSessionDescription* RTCPeerConnection::pendingRemoteDescription() { RTCSessionDescription* RTCPeerConnection::pendingRemoteDescription() const {
auto* platform_session_description = return pending_remote_description_;
peer_handler_->PendingRemoteDescription();
if (!platform_session_description)
return nullptr;
return RTCSessionDescription::Create(platform_session_description);
} }
RTCConfiguration* RTCPeerConnection::getConfiguration( RTCConfiguration* RTCPeerConnection::getConfiguration(
...@@ -2034,7 +2014,7 @@ String RTCPeerConnection::connectionState() const { ...@@ -2034,7 +2014,7 @@ String RTCPeerConnection::connectionState() const {
} }
base::Optional<bool> RTCPeerConnection::canTrickleIceCandidates() const { base::Optional<bool> RTCPeerConnection::canTrickleIceCandidates() const {
if (closed_ || !peer_handler_->RemoteDescription()) { if (closed_ || !remoteDescription()) {
return base::nullopt; return base::nullopt;
} }
webrtc::PeerConnectionInterface* native_connection = webrtc::PeerConnectionInterface* native_connection =
...@@ -2894,6 +2874,31 @@ void RTCPeerConnection::DidFailICECandidate(const String& address, ...@@ -2894,6 +2874,31 @@ void RTCPeerConnection::DidFailICECandidate(const String& address,
address, port, host_candidate, url, error_code, error_text)); address, port, host_candidate, url, error_code, error_text));
} }
void RTCPeerConnection::DidChangeSessionDescriptions(
RTCSessionDescriptionPlatform* pending_local_description,
RTCSessionDescriptionPlatform* current_local_description,
RTCSessionDescriptionPlatform* pending_remote_description,
RTCSessionDescriptionPlatform* current_remote_description) {
DCHECK(!closed_);
DCHECK(GetExecutionContext()->IsContextThread());
pending_local_description_ =
pending_local_description
? RTCSessionDescription::Create(pending_local_description)
: nullptr;
current_local_description_ =
current_local_description
? RTCSessionDescription::Create(current_local_description)
: nullptr;
pending_remote_description_ =
pending_remote_description
? RTCSessionDescription::Create(pending_remote_description)
: nullptr;
current_remote_description_ =
current_remote_description
? RTCSessionDescription::Create(current_remote_description)
: nullptr;
}
void RTCPeerConnection::DidChangeSignalingState( void RTCPeerConnection::DidChangeSignalingState(
webrtc::PeerConnectionInterface::SignalingState new_state) { webrtc::PeerConnectionInterface::SignalingState new_state) {
DCHECK(!closed_); DCHECK(!closed_);
...@@ -3545,6 +3550,10 @@ void RTCPeerConnection::DispatchScheduledEvents() { ...@@ -3545,6 +3550,10 @@ void RTCPeerConnection::DispatchScheduledEvents() {
} }
void RTCPeerConnection::Trace(Visitor* visitor) const { void RTCPeerConnection::Trace(Visitor* visitor) const {
visitor->Trace(pending_local_description_);
visitor->Trace(current_local_description_);
visitor->Trace(pending_remote_description_);
visitor->Trace(current_remote_description_);
visitor->Trace(tracks_); visitor->Trace(tracks_);
visitor->Trace(rtp_senders_); visitor->Trace(rtp_senders_);
visitor->Trace(rtp_receivers_); visitor->Trace(rtp_receivers_);
......
...@@ -183,9 +183,9 @@ class MODULES_EXPORT RTCPeerConnection final ...@@ -183,9 +183,9 @@ class MODULES_EXPORT RTCPeerConnection final
const RTCSessionDescriptionInit*, const RTCSessionDescriptionInit*,
V8VoidFunction*, V8VoidFunction*,
V8RTCPeerConnectionErrorCallback* = nullptr); V8RTCPeerConnectionErrorCallback* = nullptr);
RTCSessionDescription* localDescription(); RTCSessionDescription* localDescription() const;
RTCSessionDescription* currentLocalDescription(); RTCSessionDescription* currentLocalDescription() const;
RTCSessionDescription* pendingLocalDescription(); RTCSessionDescription* pendingLocalDescription() const;
ScriptPromise setRemoteDescription(ScriptState*, ScriptPromise setRemoteDescription(ScriptState*,
const RTCSessionDescriptionInit*, const RTCSessionDescriptionInit*,
...@@ -195,9 +195,9 @@ class MODULES_EXPORT RTCPeerConnection final ...@@ -195,9 +195,9 @@ class MODULES_EXPORT RTCPeerConnection final
const RTCSessionDescriptionInit*, const RTCSessionDescriptionInit*,
V8VoidFunction*, V8VoidFunction*,
V8RTCPeerConnectionErrorCallback* = nullptr); V8RTCPeerConnectionErrorCallback* = nullptr);
RTCSessionDescription* remoteDescription(); RTCSessionDescription* remoteDescription() const;
RTCSessionDescription* currentRemoteDescription(); RTCSessionDescription* currentRemoteDescription() const;
RTCSessionDescription* pendingRemoteDescription(); RTCSessionDescription* pendingRemoteDescription() const;
String signalingState() const; String signalingState() const;
...@@ -337,6 +337,11 @@ class MODULES_EXPORT RTCPeerConnection final ...@@ -337,6 +337,11 @@ class MODULES_EXPORT RTCPeerConnection final
const String& url, const String& url,
int error_code, int error_code,
const String& error_text) override; const String& error_text) override;
void DidChangeSessionDescriptions(
RTCSessionDescriptionPlatform* pending_local_description,
RTCSessionDescriptionPlatform* current_local_description,
RTCSessionDescriptionPlatform* pending_remote_description,
RTCSessionDescriptionPlatform* current_remote_description) override;
void DidChangeSignalingState( void DidChangeSignalingState(
webrtc::PeerConnectionInterface::SignalingState) override; webrtc::PeerConnectionInterface::SignalingState) override;
void DidChangeIceGatheringState( void DidChangeIceGatheringState(
...@@ -571,6 +576,10 @@ class MODULES_EXPORT RTCPeerConnection final ...@@ -571,6 +576,10 @@ class MODULES_EXPORT RTCPeerConnection final
HeapHashSet<Member<RTCIceTransport>> ActiveIceTransports() const; HeapHashSet<Member<RTCIceTransport>> ActiveIceTransports() const;
Member<RTCSessionDescription> pending_local_description_;
Member<RTCSessionDescription> current_local_description_;
Member<RTCSessionDescription> pending_remote_description_;
Member<RTCSessionDescription> current_remote_description_;
webrtc::PeerConnectionInterface::SignalingState signaling_state_; webrtc::PeerConnectionInterface::SignalingState signaling_state_;
webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state_; webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state_;
webrtc::PeerConnectionInterface::IceConnectionState ice_connection_state_; webrtc::PeerConnectionInterface::IceConnectionState ice_connection_state_;
......
...@@ -154,13 +154,6 @@ class MODULES_EXPORT RTCPeerConnectionHandler { ...@@ -154,13 +154,6 @@ class MODULES_EXPORT RTCPeerConnectionHandler {
virtual void SetRemoteDescription(blink::RTCVoidRequest* request, virtual void SetRemoteDescription(blink::RTCVoidRequest* request,
RTCSessionDescriptionPlatform* description); RTCSessionDescriptionPlatform* description);
virtual RTCSessionDescriptionPlatform* LocalDescription();
virtual RTCSessionDescriptionPlatform* RemoteDescription();
virtual RTCSessionDescriptionPlatform* CurrentLocalDescription();
virtual RTCSessionDescriptionPlatform* CurrentRemoteDescription();
virtual RTCSessionDescriptionPlatform* PendingLocalDescription();
virtual RTCSessionDescriptionPlatform* PendingRemoteDescription();
virtual const webrtc::PeerConnectionInterface::RTCConfiguration& virtual const webrtc::PeerConnectionInterface::RTCConfiguration&
GetConfiguration() const; GetConfiguration() const;
virtual webrtc::RTCErrorType SetConfiguration( virtual webrtc::RTCErrorType SetConfiguration(
...@@ -259,6 +252,15 @@ class MODULES_EXPORT RTCPeerConnectionHandler { ...@@ -259,6 +252,15 @@ class MODULES_EXPORT RTCPeerConnectionHandler {
class SetLocalDescriptionRequest; class SetLocalDescriptionRequest;
friend class SetLocalDescriptionRequest; friend class SetLocalDescriptionRequest;
void OnSessionDescriptionsUpdated(
std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
current_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_remote_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
current_remote_description);
void OnSignalingChange( void OnSignalingChange(
webrtc::PeerConnectionInterface::SignalingState new_state); webrtc::PeerConnectionInterface::SignalingState new_state);
void OnIceConnectionChange( void OnIceConnectionChange(
......
...@@ -676,8 +676,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescription) { ...@@ -676,8 +676,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescription) {
pc_handler_->SetLocalDescription(nullptr /*RTCVoidRequest*/, description); pc_handler_->SetLocalDescription(nullptr /*RTCVoidRequest*/, description);
RunMessageLoopsUntilIdle(); RunMessageLoopsUntilIdle();
EXPECT_EQ(description->GetType(), pc_handler_->LocalDescription()->GetType());
EXPECT_EQ(description->Sdp(), pc_handler_->LocalDescription()->Sdp());
std::string sdp_string; std::string sdp_string;
ASSERT_TRUE(mock_peer_connection_->local_description()); ASSERT_TRUE(mock_peer_connection_->local_description());
...@@ -711,8 +709,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescriptionParseError) { ...@@ -711,8 +709,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescriptionParseError) {
mock_dependency_factory_->SetFailToCreateSessionDescription(true); mock_dependency_factory_->SetFailToCreateSessionDescription(true);
pc_handler_->SetLocalDescription(nullptr /*RTCVoidRequest*/, description); pc_handler_->SetLocalDescription(nullptr /*RTCVoidRequest*/, description);
RunMessageLoopsUntilIdle(); RunMessageLoopsUntilIdle();
// A description that failed to be applied shouldn't be stored.
EXPECT_TRUE(!pc_handler_->LocalDescription());
} }
TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) { TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) {
...@@ -730,9 +726,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) { ...@@ -730,9 +726,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) {
pc_handler_->SetRemoteDescription(nullptr /*RTCVoidRequest*/, description); pc_handler_->SetRemoteDescription(nullptr /*RTCVoidRequest*/, description);
RunMessageLoopsUntilIdle(); RunMessageLoopsUntilIdle();
EXPECT_EQ(description->GetType(),
pc_handler_->RemoteDescription()->GetType());
EXPECT_EQ(description->Sdp(), pc_handler_->RemoteDescription()->Sdp());
std::string sdp_string; std::string sdp_string;
ASSERT_TRUE(mock_peer_connection_->remote_description()); ASSERT_TRUE(mock_peer_connection_->remote_description());
...@@ -766,8 +759,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescriptionParseError) { ...@@ -766,8 +759,6 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescriptionParseError) {
mock_dependency_factory_->SetFailToCreateSessionDescription(true); mock_dependency_factory_->SetFailToCreateSessionDescription(true);
pc_handler_->SetRemoteDescription(nullptr /*RTCVoidRequest*/, description); pc_handler_->SetRemoteDescription(nullptr /*RTCVoidRequest*/, description);
RunMessageLoopsUntilIdle(); RunMessageLoopsUntilIdle();
// A description that failed to be applied shouldn't be stored.
EXPECT_TRUE(!pc_handler_->RemoteDescription());
} }
TEST_F(RTCPeerConnectionHandlerTest, setConfiguration) { TEST_F(RTCPeerConnectionHandlerTest, setConfiguration) {
......
...@@ -6,10 +6,19 @@ ...@@ -6,10 +6,19 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/check.h" #include "base/check.h"
#include "third_party/webrtc/pc/sdp_utils.h"
namespace blink { namespace blink {
std::unique_ptr<webrtc::SessionDescriptionInterface> CopySessionDescription(
const webrtc::SessionDescriptionInterface* description) {
if (!description)
return nullptr;
std::string sdp;
description->ToString(&sdp);
return std::unique_ptr<webrtc::SessionDescriptionInterface>(
webrtc::CreateSessionDescription(description->type(), sdp, nullptr));
}
WebRtcSetDescriptionObserver::States::States() WebRtcSetDescriptionObserver::States::States()
: signaling_state( : signaling_state(
webrtc::PeerConnectionInterface::SignalingState::kClosed) {} webrtc::PeerConnectionInterface::SignalingState::kClosed) {}
...@@ -19,7 +28,9 @@ WebRtcSetDescriptionObserver::States::States(States&& other) ...@@ -19,7 +28,9 @@ WebRtcSetDescriptionObserver::States::States(States&& other)
sctp_transport_state(std::move(other.sctp_transport_state)), sctp_transport_state(std::move(other.sctp_transport_state)),
transceiver_states(std::move(other.transceiver_states)), transceiver_states(std::move(other.transceiver_states)),
pending_local_description(std::move(other.pending_local_description)), pending_local_description(std::move(other.pending_local_description)),
current_local_description(std::move(other.current_local_description)) {} current_local_description(std::move(other.current_local_description)),
pending_remote_description(std::move(other.pending_remote_description)),
current_remote_description(std::move(other.current_remote_description)) {}
WebRtcSetDescriptionObserver::States::~States() = default; WebRtcSetDescriptionObserver::States::~States() = default;
...@@ -30,6 +41,8 @@ operator=(States&& other) { ...@@ -30,6 +41,8 @@ operator=(States&& other) {
transceiver_states = std::move(other.transceiver_states); transceiver_states = std::move(other.transceiver_states);
pending_local_description = std::move(other.pending_local_description); pending_local_description = std::move(other.pending_local_description);
current_local_description = std::move(other.current_local_description); current_local_description = std::move(other.current_local_description);
pending_remote_description = std::move(other.pending_remote_description);
current_remote_description = std::move(other.current_remote_description);
return *this; return *this;
} }
...@@ -81,22 +94,26 @@ void WebRtcSetDescriptionObserverHandlerImpl::OnSetDescriptionComplete( ...@@ -81,22 +94,26 @@ void WebRtcSetDescriptionObserverHandlerImpl::OnSetDescriptionComplete(
transceiver_state_surfacer.Initialize(pc_, track_adapter_map_, transceiver_state_surfacer.Initialize(pc_, track_adapter_map_,
std::move(transceivers)); std::move(transceivers));
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_local_description = pc_->pending_local_description() pending_local_description =
? webrtc::CloneSessionDescription( CopySessionDescription(pc_->pending_local_description());
pc_->pending_local_description()) std::unique_ptr<webrtc::SessionDescriptionInterface>
: nullptr; current_local_description =
CopySessionDescription(pc_->current_local_description());
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
current_local_description = pc_->current_local_description() pending_remote_description =
? webrtc::CloneSessionDescription( CopySessionDescription(pc_->pending_remote_description());
pc_->current_local_description()) std::unique_ptr<webrtc::SessionDescriptionInterface>
: nullptr; current_remote_description =
CopySessionDescription(pc_->current_remote_description());
main_task_runner_->PostTask( main_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&WebRtcSetDescriptionObserverHandlerImpl:: FROM_HERE, base::BindOnce(&WebRtcSetDescriptionObserverHandlerImpl::
OnSetDescriptionCompleteOnMainThread, OnSetDescriptionCompleteOnMainThread,
this, std::move(error), pc_->signaling_state(), this, std::move(error), pc_->signaling_state(),
std::move(transceiver_state_surfacer), std::move(transceiver_state_surfacer),
std::move(pending_local_description), std::move(pending_local_description),
std::move(current_local_description))); std::move(current_local_description),
std::move(pending_remote_description),
std::move(current_remote_description)));
} }
void WebRtcSetDescriptionObserverHandlerImpl:: void WebRtcSetDescriptionObserverHandlerImpl::
...@@ -107,7 +124,11 @@ void WebRtcSetDescriptionObserverHandlerImpl:: ...@@ -107,7 +124,11 @@ void WebRtcSetDescriptionObserverHandlerImpl::
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_local_description, pending_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
current_local_description) { current_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_remote_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
current_remote_description) {
CHECK(main_task_runner_->BelongsToCurrentThread()); CHECK(main_task_runner_->BelongsToCurrentThread());
WebRtcSetDescriptionObserver::States states; WebRtcSetDescriptionObserver::States states;
states.signaling_state = signaling_state; states.signaling_state = signaling_state;
...@@ -116,6 +137,8 @@ void WebRtcSetDescriptionObserverHandlerImpl:: ...@@ -116,6 +137,8 @@ void WebRtcSetDescriptionObserverHandlerImpl::
states.transceiver_states = transceiver_state_surfacer.ObtainStates(); states.transceiver_states = transceiver_state_surfacer.ObtainStates();
states.pending_local_description = std::move(pending_local_description); states.pending_local_description = std::move(pending_local_description);
states.current_local_description = std::move(current_local_description); states.current_local_description = std::move(current_local_description);
states.pending_remote_description = std::move(pending_remote_description);
states.current_remote_description = std::move(current_remote_description);
observer_->OnSetDescriptionComplete(std::move(error), std::move(states)); observer_->OnSetDescriptionComplete(std::move(error), std::move(states));
} }
......
...@@ -30,6 +30,14 @@ ...@@ -30,6 +30,14 @@
namespace blink { namespace blink {
// Copies the session description.
// Note: At the time of writing, third_party/webrtc/pc/sdp_utils.h's
// webrtc::CloneSessionDescription() creates a copy that does not include
// candidates added with AddIceCandidate. This is why we need our own copy
// function, which copies everything.
std::unique_ptr<webrtc::SessionDescriptionInterface> CopySessionDescription(
const webrtc::SessionDescriptionInterface* description);
// The blink layer correspondent of the setLocalDescription() observer // The blink layer correspondent of the setLocalDescription() observer
// (webrtc::SetSessionDescriptionObserver) and setRemoteDescription() observer // (webrtc::SetSessionDescriptionObserver) and setRemoteDescription() observer
// (webrtc::SetRemoteDescriptionObserverInterface). The implementation should // (webrtc::SetRemoteDescriptionObserverInterface). The implementation should
...@@ -51,15 +59,14 @@ class MODULES_EXPORT WebRtcSetDescriptionObserver ...@@ -51,15 +59,14 @@ class MODULES_EXPORT WebRtcSetDescriptionObserver
webrtc::PeerConnectionInterface::SignalingState signaling_state; webrtc::PeerConnectionInterface::SignalingState signaling_state;
blink::WebRTCSctpTransportSnapshot sctp_transport_state; blink::WebRTCSctpTransportSnapshot sctp_transport_state;
std::vector<blink::RtpTransceiverState> transceiver_states; std::vector<blink::RtpTransceiverState> transceiver_states;
// For now, the session descriptions are only surfaced for the sake of
// showing up in chrome://webrtc-internals/ when implicit
// setLocalDescription() resolves.
// TODO(https://crbug.com/788558): Surface all states to blink at the same,
// time, including [current/pending][Local/Remote]Description.
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_local_description; pending_local_description;
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
current_local_description; current_local_description;
std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_remote_description;
std::unique_ptr<webrtc::SessionDescriptionInterface>
current_remote_description;
DISALLOW_COPY_AND_ASSIGN(States); DISALLOW_COPY_AND_ASSIGN(States);
}; };
...@@ -117,7 +124,11 @@ class MODULES_EXPORT WebRtcSetDescriptionObserverHandlerImpl ...@@ -117,7 +124,11 @@ class MODULES_EXPORT WebRtcSetDescriptionObserverHandlerImpl
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_local_description, pending_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface> std::unique_ptr<webrtc::SessionDescriptionInterface>
current_local_description); current_local_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
pending_remote_description,
std::unique_ptr<webrtc::SessionDescriptionInterface>
current_remote_description);
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> signaling_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> signaling_task_runner_;
......
...@@ -43,8 +43,9 @@ ...@@ -43,8 +43,9 @@
namespace blink { namespace blink {
class RTCIceCandidatePlatform; class RTCIceCandidatePlatform;
class RTCRtpTransceiverPlatform;
class RTCRtpReceiverPlatform; class RTCRtpReceiverPlatform;
class RTCRtpTransceiverPlatform;
class RTCSessionDescriptionPlatform;
struct PLATFORM_EXPORT WebRTCSctpTransportSnapshot { struct PLATFORM_EXPORT WebRTCSctpTransportSnapshot {
rtc::scoped_refptr<webrtc::SctpTransportInterface> transport; rtc::scoped_refptr<webrtc::SctpTransportInterface> transport;
...@@ -66,6 +67,11 @@ class PLATFORM_EXPORT RTCPeerConnectionHandlerClient { ...@@ -66,6 +67,11 @@ class PLATFORM_EXPORT RTCPeerConnectionHandlerClient {
const String& url, const String& url,
int error_code, int error_code,
const String& error_text) = 0; const String& error_text) = 0;
virtual void DidChangeSessionDescriptions(
RTCSessionDescriptionPlatform* pending_local_description,
RTCSessionDescriptionPlatform* current_local_description,
RTCSessionDescriptionPlatform* pending_remote_description,
RTCSessionDescriptionPlatform* current_remote_description) = 0;
virtual void DidChangeSignalingState( virtual void DidChangeSignalingState(
webrtc::PeerConnectionInterface::SignalingState) = 0; webrtc::PeerConnectionInterface::SignalingState) = 0;
virtual void DidChangeIceGatheringState( virtual void DidChangeIceGatheringState(
......
<!doctype html>
<meta charset=utf-8>
<title></title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
'use strict';
promise_test(async t => {
const pc = new RTCPeerConnection();
t.add_cleanup(() => pc.close());
const offer = await pc.createOffer();
assert_equals(pc.pendingLocalDescription, null,
'pendingLocalDescription is null before setLocalDescription');
const promise = pc.setLocalDescription(offer);
assert_equals(pc.pendingLocalDescription, null,
'pendingLocalDescription is still null while promise pending');
await promise;
assert_not_equals(pc.pendingLocalDescription, null,
'pendingLocalDescription is not null after await');
}, "pendingLocalDescription is surfaced at the right time");
promise_test(async t => {
const pc = new RTCPeerConnection();
t.add_cleanup(() => pc.close());
const offer = await pc.createOffer();
assert_equals(pc.pendingRemoteDescription, null,
'pendingRemoteDescription is null before setRemoteDescription');
const promise = pc.setRemoteDescription(offer);
assert_equals(pc.pendingRemoteDescription, null,
'pendingRemoteDescription is still null while promise pending');
await promise;
assert_not_equals(pc.pendingRemoteDescription, null,
'pendingRemoteDescription is not null after await');
}, "pendingRemoteDescription is surfaced at the right time");
promise_test(async t => {
const pc1 = new RTCPeerConnection();
t.add_cleanup(() => pc1.close());
const pc2 = new RTCPeerConnection();
t.add_cleanup(() => pc2.close());
const offer = await pc1.createOffer();
await pc1.setLocalDescription(offer);
await pc2.setRemoteDescription(offer);
const answer = await pc2.createAnswer();
assert_equals(pc2.currentLocalDescription, null,
'currentLocalDescription is null before setLocalDescription');
const promise = pc2.setLocalDescription(answer);
assert_equals(pc2.currentLocalDescription, null,
'currentLocalDescription is still null while promise pending');
await promise;
assert_not_equals(pc2.currentLocalDescription, null,
'currentLocalDescription is not null after await');
}, "currentLocalDescription is surfaced at the right time");
promise_test(async t => {
const pc1 = new RTCPeerConnection();
t.add_cleanup(() => pc1.close());
const pc2 = new RTCPeerConnection();
t.add_cleanup(() => pc2.close());
const offer = await pc1.createOffer();
await pc1.setLocalDescription(offer);
await pc2.setRemoteDescription(offer);
const answer = await pc2.createAnswer();
assert_equals(pc1.currentRemoteDescription, null,
'currentRemoteDescription is null before setRemoteDescription');
const promise = pc1.setRemoteDescription(answer);
assert_equals(pc1.currentRemoteDescription, null,
'currentRemoteDescription is still null while promise pending');
await promise;
assert_not_equals(pc1.currentRemoteDescription, null,
'currentRemoteDescription is not null after await');
}, "currentRemoteDescription is surfaced at the right time");
</script>
...@@ -5,6 +5,6 @@ PASS setLocalDescription() with answer not created by own createAnswer() should ...@@ -5,6 +5,6 @@ PASS setLocalDescription() with answer not created by own createAnswer() should
FAIL Calling setLocalDescription(answer) from stable state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: stable" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13 FAIL Calling setLocalDescription(answer) from stable state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: stable" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13
FAIL Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: have-local-offer" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13 FAIL Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: have-local-offer" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13
PASS Setting previously generated answer after a call to createOffer should work PASS Setting previously generated answer after a call to createOffer should work
FAIL setLocalDescription(answer) should update internal state with a queued task, in the right order assert_not_equals: pendingRemoteDescription should not be set synchronously after a call to sLD got disallowed value null PASS setLocalDescription(answer) should update internal state with a queued task, in the right order
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -6,6 +6,6 @@ FAIL Set created offer other than last offer should reject with InvalidModificat ...@@ -6,6 +6,6 @@ FAIL Set created offer other than last offer should reject with InvalidModificat
PASS Creating and setting offer multiple times should succeed PASS Creating and setting offer multiple times should succeed
PASS Setting previously generated offer after a call to createAnswer should work PASS Setting previously generated offer after a call to createAnswer should work
PASS Negotiation works when there has been a repeated setLocalDescription(offer) PASS Negotiation works when there has been a repeated setLocalDescription(offer)
FAIL setLocalDescription(offer) should update internal state with a queued task, in the right order assert_equals: pendingRemoteDescription should never be set due to sLD expected null but got object "[object RTCSessionDescription]" PASS setLocalDescription(offer) should update internal state with a queued task, in the right order
Harness: the test ran to completion. Harness: the test ran to completion.
This is a testharness.js-based test.
PASS setLocalDescription(rollback) from have-local-offer state should reset back to stable state
PASS setLocalDescription(rollback) from stable state should reject with InvalidStateError
PASS setLocalDescription(rollback) after setting answer description should reject with InvalidStateError
PASS setLocalDescription(rollback) should ignore invalid sdp content and succeed
FAIL setLocalDescription(rollback) should update internal state with a queued tassk, in the right order assert_not_equals: pendingLocalDescription should not be set synchronously after a call to sLD got disallowed value null
Harness: the test ran to completion.
...@@ -5,6 +5,6 @@ PASS setLocalDescription() with answer not created by own createAnswer() should ...@@ -5,6 +5,6 @@ PASS setLocalDescription() with answer not created by own createAnswer() should
FAIL Calling setLocalDescription(answer) from stable state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: stable" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13 FAIL Calling setLocalDescription(answer) from stable state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: stable" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13
FAIL Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: have-local-offer" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13 FAIL Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidModificationError promise_rejects_dom: function "function() { throw e }" threw object "OperationError: Failed to execute 'setLocalDescription' on 'RTCPeerConnection': Failed to set local answer sdp: Called in wrong state: have-local-offer" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13
FAIL Setting previously generated answer after a call to createOffer should work promise_test: Unhandled rejection with value: object "OperationError: Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Rollback not supported in Plan B" FAIL Setting previously generated answer after a call to createOffer should work promise_test: Unhandled rejection with value: object "OperationError: Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Rollback not supported in Plan B"
FAIL setLocalDescription(answer) should update internal state with a queued task, in the right order assert_not_equals: pendingRemoteDescription should not be set synchronously after a call to sLD got disallowed value null PASS setLocalDescription(answer) should update internal state with a queued task, in the right order
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -6,6 +6,6 @@ FAIL Set created offer other than last offer should reject with InvalidModificat ...@@ -6,6 +6,6 @@ FAIL Set created offer other than last offer should reject with InvalidModificat
PASS Creating and setting offer multiple times should succeed PASS Creating and setting offer multiple times should succeed
FAIL Setting previously generated offer after a call to createAnswer should work promise_test: Unhandled rejection with value: object "OperationError: Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Rollback not supported in Plan B" FAIL Setting previously generated offer after a call to createAnswer should work promise_test: Unhandled rejection with value: object "OperationError: Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Rollback not supported in Plan B"
FAIL Negotiation works when there has been a repeated setLocalDescription(offer) assert_equals: expected 1 but got 0 FAIL Negotiation works when there has been a repeated setLocalDescription(offer) assert_equals: expected 1 but got 0
FAIL setLocalDescription(offer) should update internal state with a queued task, in the right order assert_equals: pendingRemoteDescription should never be set due to sLD expected null but got object "[object RTCSessionDescription]" PASS setLocalDescription(offer) should update internal state with a queued task, in the right order
Harness: the test ran to completion. Harness: the test ran to completion.
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