[PeerConnection] Implement parameterless SetLocalDescription().
For background, motivation, requirements and implementation notes, see
https://docs.google.com/document/d/1XLwNN2kUIGGTwz9LQ0NwJNkcybi9oKnynUEZB1jGA14/edit?usp=sharing
The parameterless SetLocalDescription() will implicitly create an
offer or answer to be set by chaining create offer or answer with
setting the session description, as per spec:
https://w3c.github.io/webrtc-pc/#dom-peerconnection-setlocaldescription
Bug: chromium:980885
Change-Id: Ia430160869df18fd47b756b9adf9e7e23ba8e969
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157444
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29653}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index cc2fa46..55cc593 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -991,6 +991,11 @@
// that this method always takes ownership of it.
virtual void SetLocalDescription(SetSessionDescriptionObserver* observer,
SessionDescriptionInterface* desc) = 0;
+ // Implicitly creates an offer or answer (depending on the current signaling
+ // state) and performs SetLocalDescription() with the newly generated session
+ // description.
+ // TODO(hbos): Make pure virtual when implemented by downstream projects.
+ virtual void SetLocalDescription(SetSessionDescriptionObserver* observer) {}
// Sets the remote session description.
// The PeerConnection takes the ownership of |desc| even if it fails.
// The |observer| callback will be called when done.
diff --git a/api/peer_connection_proxy.h b/api/peer_connection_proxy.h
index 551af48..3b9cf79 100644
--- a/api/peer_connection_proxy.h
+++ b/api/peer_connection_proxy.h
@@ -100,6 +100,7 @@
SetLocalDescription,
SetSessionDescriptionObserver*,
SessionDescriptionInterface*)
+PROXY_METHOD1(void, SetLocalDescription, SetSessionDescriptionObserver*)
PROXY_METHOD2(void,
SetRemoteDescription,
SetSessionDescriptionObserver*,
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 7dc2e35..46a61ab 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -694,6 +694,85 @@
} // namespace
+// Used by parameterless SetLocalDescription() to create an offer or answer.
+// Upon completion of creating the session description, SetLocalDescription() is
+// invoked with the result.
+// For consistency with DoSetLocalDescription(), if the PeerConnection is
+// destroyed midst operation, we DO NOT inform the
+// |set_local_description_observer| that the operation failed.
+// TODO(hbos): If/when we process SLD messages in ~PeerConnection, the
+// consistent thing would be to inform the observer here.
+class PeerConnection::ImplicitCreateSessionDescriptionObserver
+ : public CreateSessionDescriptionObserver {
+ public:
+ ImplicitCreateSessionDescriptionObserver(
+ rtc::WeakPtr<PeerConnection> pc,
+ rtc::scoped_refptr<SetSessionDescriptionObserver>
+ set_local_description_observer)
+ : pc_(std::move(pc)),
+ set_local_description_observer_(
+ std::move(set_local_description_observer)) {}
+ ~ImplicitCreateSessionDescriptionObserver() override {
+ RTC_DCHECK(was_called_);
+ }
+
+ void SetOperationCompleteCallback(
+ std::function<void()> operation_complete_callback) {
+ operation_complete_callback_ = std::move(operation_complete_callback);
+ }
+
+ bool was_called() const { return was_called_; }
+
+ void OnSuccess(SessionDescriptionInterface* desc_ptr) override {
+ RTC_DCHECK(!was_called_);
+ std::unique_ptr<SessionDescriptionInterface> desc(desc_ptr);
+ was_called_ = true;
+
+ // Abort early if |pc_| is no longer valid.
+ if (!pc_) {
+ operation_complete_callback_();
+ return;
+ }
+ // DoSetLocalDescription() is currently implemented as a synchronous
+ // operation but where the |set_local_description_observer_|'s callbacks are
+ // invoked asynchronously in a post to PeerConnection::OnMessage().
+ pc_->DoSetLocalDescription(std::move(desc),
+ std::move(set_local_description_observer_));
+ // For backwards-compatability reasons, we declare the operation as
+ // completed here (rather than in PeerConnection::OnMessage()). This ensures
+ // that subsequent offer/answer operations can start immediately (without
+ // waiting for OnMessage()).
+ operation_complete_callback_();
+ }
+
+ void OnFailure(RTCError error) override {
+ RTC_DCHECK(!was_called_);
+ was_called_ = true;
+
+ // Abort early if |pc_| is no longer valid.
+ if (!pc_) {
+ operation_complete_callback_();
+ return;
+ }
+ // DoSetLocalDescription() reports its failures in a post. We do the
+ // same thing here for consistency.
+ pc_->PostSetSessionDescriptionFailure(
+ set_local_description_observer_,
+ RTCError(error.type(),
+ std::string("SetLocalDescription failed to create "
+ "session description - ") +
+ error.message()));
+ operation_complete_callback_();
+ }
+
+ private:
+ bool was_called_ = false;
+ rtc::WeakPtr<PeerConnection> pc_;
+ rtc::scoped_refptr<SetSessionDescriptionObserver>
+ set_local_description_observer_;
+ std::function<void()> operation_complete_callback_;
+};
+
class PeerConnection::LocalIceCredentialsToReplace {
public:
// Sets the ICE credentials that need restarting to the ICE credentials of
@@ -2382,15 +2461,68 @@
// operation but where the |observer|'s callbacks are invoked
// asynchronously in a post to OnMessage().
// For backwards-compatability reasons, we declare the operation as
- // completed here (rather than in OnMessage()). This ensures that:
- // - This operation is not keeping the PeerConnection alive past this
- // point.
- // - Subsequent offer/answer operations can start immediately (without
- // waiting for OnMessage()).
+ // completed here (rather than in OnMessage()). This ensures that
+ // subsequent offer/answer operations can start immediately (without
+ // waiting for OnMessage()).
operations_chain_callback();
});
}
+void PeerConnection::SetLocalDescription(
+ SetSessionDescriptionObserver* observer) {
+ RTC_DCHECK_RUN_ON(signaling_thread());
+ // The |create_sdp_observer| handles performing DoSetLocalDescription() with
+ // the resulting description as well as completing the operation.
+ rtc::scoped_refptr<ImplicitCreateSessionDescriptionObserver>
+ create_sdp_observer(
+ new rtc::RefCountedObject<ImplicitCreateSessionDescriptionObserver>(
+ weak_ptr_factory_.GetWeakPtr(),
+ rtc::scoped_refptr<SetSessionDescriptionObserver>(observer)));
+ // Chain this operation. If asynchronous operations are pending on the chain,
+ // this operation will be queued to be invoked, otherwise the contents of the
+ // lambda will execute immediately.
+ operations_chain_->ChainOperation(
+ [this_weak_ptr = weak_ptr_factory_.GetWeakPtr(),
+ create_sdp_observer](std::function<void()> operations_chain_callback) {
+ // The |create_sdp_observer| is responsible for completing the
+ // operation.
+ create_sdp_observer->SetOperationCompleteCallback(
+ std::move(operations_chain_callback));
+ // Abort early if |this_weak_ptr| is no longer valid. This triggers the
+ // same code path as if DoCreateOffer() or DoCreateAnswer() failed.
+ if (!this_weak_ptr) {
+ create_sdp_observer->OnFailure(RTCError(
+ RTCErrorType::INTERNAL_ERROR,
+ "SetLocalDescription failed because the session was shut down"));
+ return;
+ }
+ switch (this_weak_ptr->signaling_state()) {
+ case PeerConnectionInterface::kStable:
+ case PeerConnectionInterface::kHaveLocalOffer:
+ case PeerConnectionInterface::kHaveRemotePrAnswer:
+ // TODO(hbos): If [LastCreatedOffer] exists and still represents the
+ // current state of the system, use that instead of creating another
+ // offer.
+ this_weak_ptr->DoCreateOffer(RTCOfferAnswerOptions(),
+ create_sdp_observer);
+ break;
+ case PeerConnectionInterface::kHaveLocalPrAnswer:
+ case PeerConnectionInterface::kHaveRemoteOffer:
+ // TODO(hbos): If [LastCreatedAnswer] exists and still represents
+ // the current state of the system, use that instead of creating
+ // another answer.
+ this_weak_ptr->DoCreateAnswer(RTCOfferAnswerOptions(),
+ create_sdp_observer);
+ break;
+ case PeerConnectionInterface::kClosed:
+ create_sdp_observer->OnFailure(RTCError(
+ RTCErrorType::INVALID_STATE,
+ "SetLocalDescription called when PeerConnection is closed."));
+ break;
+ }
+ });
+}
+
void PeerConnection::DoSetLocalDescription(
std::unique_ptr<SessionDescriptionInterface> desc,
rtc::scoped_refptr<SetSessionDescriptionObserver> observer) {
@@ -2807,11 +2939,9 @@
// the |observer|'s callbacks are invoked asynchronously in a post to
// OnMessage().
// For backwards-compatability reasons, we declare the operation as
- // completed here (rather than in OnMessage()). This ensures that:
- // - This operation is not keeping the PeerConnection alive past this
- // point.
- // - Subsequent offer/answer operations can start immediately (without
- // waiting for OnMessage()).
+ // completed here (rather than in OnMessage()). This ensures that
+ // subsequent offer/answer operations can start immediately (without
+ // waiting for OnMessage()).
operations_chain_callback();
});
}
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 7a576f3..dea05ac 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -210,6 +210,7 @@
const RTCOfferAnswerOptions& options) override;
void SetLocalDescription(SetSessionDescriptionObserver* observer,
SessionDescriptionInterface* desc) override;
+ void SetLocalDescription(SetSessionDescriptionObserver* observer) override;
void SetRemoteDescription(SetSessionDescriptionObserver* observer,
SessionDescriptionInterface* desc) override;
void SetRemoteDescription(
@@ -314,6 +315,8 @@
~PeerConnection() override;
private:
+ class ImplicitCreateSessionDescriptionObserver;
+ friend class ImplicitCreateSessionDescriptionObserver;
class SetRemoteDescriptionObserverAdapter;
friend class SetRemoteDescriptionObserverAdapter;
// Represents the [[LocalIceCredentialsToReplace]] internal slot in the spec.
diff --git a/pc/peer_connection_signaling_unittest.cc b/pc/peer_connection_signaling_unittest.cc
index e79ee3d..30b11ce 100644
--- a/pc/peer_connection_signaling_unittest.cc
+++ b/pc/peer_connection_signaling_unittest.cc
@@ -565,6 +565,32 @@
EXPECT_TRUE(observer->called());
}
+TEST_P(PeerConnectionSignalingTest, ImplicitCreateOfferAndShutdown) {
+ auto caller = CreatePeerConnection();
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->SetLocalDescription(observer);
+ caller.reset(nullptr);
+ EXPECT_FALSE(observer->called());
+}
+
+TEST_P(PeerConnectionSignalingTest, CloseBeforeImplicitCreateOfferAndShutdown) {
+ auto caller = CreatePeerConnection();
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->Close();
+ caller->pc()->SetLocalDescription(observer);
+ caller.reset(nullptr);
+ EXPECT_FALSE(observer->called());
+}
+
+TEST_P(PeerConnectionSignalingTest, CloseAfterImplicitCreateOfferAndShutdown) {
+ auto caller = CreatePeerConnection();
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->SetLocalDescription(observer);
+ caller->pc()->Close();
+ caller.reset(nullptr);
+ EXPECT_FALSE(observer->called());
+}
+
TEST_P(PeerConnectionSignalingTest, SetRemoteDescriptionExecutesImmediately) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnection();
@@ -608,6 +634,143 @@
EXPECT_EQ(2u, callee->pc()->GetReceivers().size());
}
+TEST_P(PeerConnectionSignalingTest,
+ ParameterlessSetLocalDescriptionCreatesOffer) {
+ auto caller = CreatePeerConnectionWithAudioVideo();
+
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->SetLocalDescription(observer);
+
+ // The offer is created asynchronously; message processing is needed for it to
+ // complete.
+ EXPECT_FALSE(observer->called());
+ EXPECT_FALSE(caller->pc()->pending_local_description());
+ EXPECT_EQ(PeerConnection::kStable, caller->signaling_state());
+
+ // Wait for messages to be processed.
+ EXPECT_TRUE_WAIT(observer->called(), kWaitTimeout);
+ EXPECT_TRUE(observer->result());
+ EXPECT_TRUE(caller->pc()->pending_local_description());
+ EXPECT_EQ(SdpType::kOffer,
+ caller->pc()->pending_local_description()->GetType());
+ EXPECT_EQ(PeerConnection::kHaveLocalOffer, caller->signaling_state());
+}
+
+TEST_P(PeerConnectionSignalingTest,
+ ParameterlessSetLocalDescriptionCreatesAnswer) {
+ auto caller = CreatePeerConnectionWithAudioVideo();
+ auto callee = CreatePeerConnectionWithAudioVideo();
+
+ callee->SetRemoteDescription(caller->CreateOffer());
+ EXPECT_EQ(PeerConnection::kHaveRemoteOffer, callee->signaling_state());
+
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ callee->pc()->SetLocalDescription(observer);
+
+ // The answer is created asynchronously; message processing is needed for it
+ // to complete.
+ EXPECT_FALSE(observer->called());
+ EXPECT_FALSE(callee->pc()->current_local_description());
+
+ // Wait for messages to be processed.
+ EXPECT_TRUE_WAIT(observer->called(), kWaitTimeout);
+ EXPECT_TRUE(observer->result());
+ EXPECT_TRUE(callee->pc()->current_local_description());
+ EXPECT_EQ(SdpType::kAnswer,
+ callee->pc()->current_local_description()->GetType());
+ EXPECT_EQ(PeerConnection::kStable, callee->signaling_state());
+}
+
+TEST_P(PeerConnectionSignalingTest,
+ ParameterlessSetLocalDescriptionFullExchange) {
+ auto caller = CreatePeerConnectionWithAudioVideo();
+ auto callee = CreatePeerConnectionWithAudioVideo();
+
+ // SetLocalDescription(), implicitly creating an offer.
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver>
+ caller_set_local_description_observer(
+ new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
+ caller->pc()->SetLocalDescription(caller_set_local_description_observer);
+ EXPECT_TRUE_WAIT(caller_set_local_description_observer->called(),
+ kWaitTimeout);
+ ASSERT_TRUE(caller->pc()->pending_local_description());
+
+ // SetRemoteDescription(offer)
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver>
+ callee_set_remote_description_observer(
+ new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
+ callee->pc()->SetRemoteDescription(
+ callee_set_remote_description_observer.get(),
+ CloneSessionDescription(caller->pc()->pending_local_description())
+ .release());
+
+ // SetLocalDescription(), implicitly creating an answer.
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver>
+ callee_set_local_description_observer(
+ new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
+ callee->pc()->SetLocalDescription(callee_set_local_description_observer);
+ EXPECT_TRUE_WAIT(callee_set_local_description_observer->called(),
+ kWaitTimeout);
+ // Chaining guarantees SetRemoteDescription() happened before
+ // SetLocalDescription().
+ EXPECT_TRUE(callee_set_remote_description_observer->called());
+ EXPECT_TRUE(callee->pc()->current_local_description());
+
+ // SetRemoteDescription(answer)
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver>
+ caller_set_remote_description_observer(
+ new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
+ caller->pc()->SetRemoteDescription(
+ caller_set_remote_description_observer,
+ CloneSessionDescription(callee->pc()->current_local_description())
+ .release());
+ EXPECT_TRUE_WAIT(caller_set_remote_description_observer->called(),
+ kWaitTimeout);
+
+ EXPECT_EQ(PeerConnection::kStable, caller->signaling_state());
+ EXPECT_EQ(PeerConnection::kStable, callee->signaling_state());
+}
+
+TEST_P(PeerConnectionSignalingTest,
+ ParameterlessSetLocalDescriptionCloseBeforeCreatingOffer) {
+ auto caller = CreatePeerConnectionWithAudioVideo();
+
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->Close();
+ caller->pc()->SetLocalDescription(observer);
+
+ // The operation should fail asynchronously.
+ EXPECT_FALSE(observer->called());
+ EXPECT_TRUE_WAIT(observer->called(), kWaitTimeout);
+ EXPECT_FALSE(observer->result());
+ // This did not affect the signaling state.
+ EXPECT_EQ(PeerConnection::kClosed, caller->pc()->signaling_state());
+ EXPECT_EQ(
+ "SetLocalDescription failed to create session description - "
+ "SetLocalDescription called when PeerConnection is closed.",
+ observer->error());
+}
+
+TEST_P(PeerConnectionSignalingTest,
+ ParameterlessSetLocalDescriptionCloseWhileCreatingOffer) {
+ auto caller = CreatePeerConnectionWithAudioVideo();
+
+ auto observer = MockSetSessionDescriptionObserver::Create();
+ caller->pc()->SetLocalDescription(observer);
+ caller->pc()->Close();
+
+ // The operation should fail asynchronously.
+ EXPECT_FALSE(observer->called());
+ EXPECT_TRUE_WAIT(observer->called(), kWaitTimeout);
+ EXPECT_FALSE(observer->result());
+ // This did not affect the signaling state.
+ EXPECT_EQ(PeerConnection::kClosed, caller->pc()->signaling_state());
+ EXPECT_EQ(
+ "SetLocalDescription failed to create session description - "
+ "CreateOffer failed because the session was shut down",
+ observer->error());
+}
+
INSTANTIATE_TEST_SUITE_P(PeerConnectionSignalingTest,
PeerConnectionSignalingTest,
Values(SdpSemantics::kPlanB,
diff --git a/pc/test/mock_peer_connection_observers.h b/pc/test/mock_peer_connection_observers.h
index 5a388bd..2017735 100644
--- a/pc/test/mock_peer_connection_observers.h
+++ b/pc/test/mock_peer_connection_observers.h
@@ -271,6 +271,10 @@
class MockSetSessionDescriptionObserver
: public webrtc::SetSessionDescriptionObserver {
public:
+ static rtc::scoped_refptr<MockSetSessionDescriptionObserver> Create() {
+ return new rtc::RefCountedObject<MockSetSessionDescriptionObserver>();
+ }
+
MockSetSessionDescriptionObserver()
: called_(false),
error_("MockSetSessionDescriptionObserver not called") {}